From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-4.0 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 7DFBA1F453 for ; Thu, 25 Apr 2019 21:59:04 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:cc:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=IO3C3C0HzvLaPz/Q 5w4jS0v85n6dvkXxBvRVuX740d6dRgC5iSw22ESx/vWig1U1oCHXXX2dB5xryx0G HrU+UF3PHkSVWO/eGsovMgxJ+/m1SVGQfrZkrvEp0ZQLRShN8z8KspZ3+r74nk+s s/er+RKuK1iocWiXDeA3ZSSvxdA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:cc:references:from:subject:message-id:date :mime-version:in-reply-to:content-type :content-transfer-encoding; s=default; bh=2YsdiWIA4iRu+0RssgURTd 2wRj0=; b=YTtlein0rdejw9BSDrut7PqeKQt/COTjQReeyCDnf2Kl9a3dNMkK3C QY6SF+csfya/21/d7DJQXdadi9wPCXOdmit1JI8Tqnv8xe35DqDXm2rptsa71vni 576fvD/zYuz+tOlCV+0HV/vVDcvzBn+aAKPL5K7gJwdiCFAbrWUQ0= Received: (qmail 105742 invoked by alias); 25 Apr 2019 21:59:02 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 105733 invoked by uid 89); 25 Apr 2019 21:59:01 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-vs1-f67.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=to:cc:references:from:openpgp:autocrypt:subject:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=RjdKWiF+2DitnUk//ijjqQYnWCFKPuD//uZ/s1ziuxA=; b=zo4ASkxO774EhuuT+DrYNFkubUGgq8HMRtc5z3ASlG3Zrly29UyJqoEoytDJARWjvS W+RpwaM3lHbboFyy9RegM74jX2Zi5vCiv5FoBUih8q/c2iovj6mYH/51AFPEZAIM9R3z aQlfdMknZkPPy58wmq6AxYCLFuGqZ+8J8VbgjOoVmNZtakRrqTOrMalgcHtcR4oWF8nx TG2v/v/6IbTDwWq39ChCt6q7FRt1sqZ/bG4qj4aYiA4mtM5YIVmn3irXHwg5dkYhFM/R IXl5gtoV6z/GBUR1NVnlUcLq+YsAL9ze1AtMn8K2ABjh3n+fVL52z0TaoDDkHbp52eMc YV4Q== To: "Gabriel F. T. Gomes" Cc: libc-alpha@sourceware.org References: <20190329133529.22523-1-adhemerval.zanella@linaro.org> <20190329133529.22523-5-adhemerval.zanella@linaro.org> <20190425015655.c2wckfwowa4xboc4@tereshkova> From: Adhemerval Zanella Openpgp: preference=signencrypt Subject: Re: [PATCH 04/28] powerpc: ceil/ceilf refactor Message-ID: <83c7cdd1-0042-fa33-36f4-93d732fedba0@linaro.org> Date: Thu, 25 Apr 2019 18:58:56 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190425015655.c2wckfwowa4xboc4@tereshkova> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 24/04/2019 22:56, Gabriel F. T. Gomes wrote: > On Fri, Mar 29 2019, Adhemerval Zanella wrote: >> >> The IFUNC organization for powerpc64 is also change to be enabled only >> for powerpc64 and not for powerpc64le. > > OK. > >> * sysdeps/powerpc/fpu/fenv_libc.h (__fesetround_inline_nocheck): New >> macro. > > I really wish we can get rid of the burden of writing changelogs, but > for now shouldn't this be function, instead of macro? > >> +/* Same as __fesetround_inline, however without runtime check to use DFP >> + mtfsfi syntax or if round is valid. */ > > What would be a runtime check to use DFP syntax? Are you referring to > the optional third parameter (W) to mtfsfi? I don't see such mechanism > in __fesetround_inline, so I'm not sure I understand this > comment/comparison. The comment is not clear indeed, to use mtfsfi correctly one should check hwcap for PPC_FEATURE_HAS_DFP as fesetenv_register do. I changed comment to: /* Same as __fesetround_inline, however without runtime check to use DFP mtfsfi syntax (as relax_fenv_state) or if round value is valid. */ > >> +static inline void >> +__fesetround_inline_nocheck (const int round) >> +{ >> + asm volatile ("mtfsfi 7,%0" : : "i" (round)); >> +} >> + > > The implementation looks OK. > >> +enum round_mode >> +{ >> + CEIL, >> + FLOOR, >> + ROUND, >> + TRUNC, >> + NEARBYINT, >> +}; > > OK. To be used in following patches. In fact I think it would be better to add each round_mode on each patch it implements. I have changed it locally. > >> +static inline void >> +set_fenv_mode (enum round_mode mode) >> +{ >> + int rmode; >> + switch (mode) >> + { >> + case CEIL: rmode = FE_UPWARD; break; >> + default: rmode = FE_TONEAREST; break; >> + } >> + __fesetround_inline_nocheck (rmode); >> +} > > OK. > >> +static inline float >> +round_to_integer_float (enum round_mode mode, float x) >> +{ >> + /* Ensure sNaN input is converted to qNaN. */ >> + if (__glibc_unlikely (isnan (x))) >> + return x + x; >> + >> + if (fabs (x) > 0x1p+23) >> + return x; >> + >> + float r = x; >> + >> + /* Save current FPU rounding mode and inexact state. */ >> + fenv_t fe = fegetenv_register (); >> + set_fenv_mode (mode); >> + if (x > 0.0) >> + { >> + r += 0x1p+23; >> + r -= 0x1p+23; >> + r = fabs (r); >> + } >> + else if (x < 0.0) >> + { >> + r -= 0x1p+23; >> + r += 0x1p+23; >> + r = -fabs (r); >> + } >> + __builtin_mtfsf (0xff, fe); >> + >> + return r; >> +} > > Ok. > >> +static inline double >> +round_to_integer_double (enum round_mode mode, double x) >> +{ >> + /* Ensure sNaN input is converted to qNaN. */ >> + if (__glibc_unlikely (isnan (x))) >> + return x + x; >> + >> + if (fabs (x) > 0x1p+52) >> + return x; >> + >> + double r = x; >> + >> + /* Save current FPU rounding mode and inexact state. */ >> + fenv_t fe = fegetenv_register (); >> + set_fenv_mode (mode); >> + if (x > 0.0) >> + { >> + r += 0x1p+52; >> + r -= 0x1p+52; >> + r = fabs (r); >> + } >> + else if (x < 0.0) >> + { >> + r -= 0x1p+52; >> + r += 0x1p+52; >> + r = -fabs (r); >> + } >> + __builtin_mtfsf (0xff, fe); >> + >> + return r; >> +} > > OK. > >> +double >> +__ceil (double x) >> +{ >> +#ifdef _ARCH_PWR5X >> + return __builtin_ceil (x); >> +#else >> + return round_to_integer_double (CEIL, x); >> +#endif > > The arch check looks correct. >