From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf29.google.com (mail-qv1-xf29.google.com [IPv6:2607:f8b0:4864:20::f29]) by sourceware.org (Postfix) with ESMTPS id B09A13858C60 for ; Thu, 7 Oct 2021 11:37:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B09A13858C60 Received: by mail-qv1-xf29.google.com with SMTP id z15so3910268qvj.7 for ; Thu, 07 Oct 2021 04:37:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=f3qDXTejWAVA+SGHcAX+UrnPmYdT5YjY/IEUhVRjNZE=; b=7T5Y8OlEHKFbfpacIKiyl660OvMMG5yL2hE9Hg5giyybFI5be1RItoBkXpv6hdkhw9 wZO4jH1W3W0LZpsYN66i3WgMIt27lsA9xwltD3lpDxKPYy2dAMCpHmLjwNHKxK1UvfCQ jY9CF+r20xJE9vV2NR+dgRxbop4DXo4no9AN+eaT+hUQrczKOhkmJP287qI4DZgrI2b+ qkoRj8PFfsavuqr9OajkouVIPjw4cNTWcDV06Wd07K99RazTkOcRhI4YgwVBFH9dbVIj Z3zMErsUB0Sg/lrmloD5PZr8QEHygAoL8ACcal8i2dW7xDaUpN8WALa38PbReG1kUHq7 /92g== X-Gm-Message-State: AOAM5324KGTh2rQBAGhl3fLoPz1HRhPb0NCWSUhpNvq0G8GQ9H9fwLYd 1UGLdSxfVccitKqII70+J/k1/h5yJK1CBw== X-Google-Smtp-Source: ABdhPJxPNAVpZHJiPRcAT60J5irqCxakuElVt6PhCIJEvSgbjJXEOFZea4lp9SE/wuNhIvvDeMU6TA== X-Received: by 2002:a0c:f383:: with SMTP id i3mr3453446qvk.0.1633606634127; Thu, 07 Oct 2021 04:37:14 -0700 (PDT) Received: from ?IPv6:2804:431:c7cb:807a:2864:3aef:e68:8698? ([2804:431:c7cb:807a:2864:3aef:e68:8698]) by smtp.gmail.com with ESMTPSA id w1sm15005397qtv.71.2021.10.07.04.37.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 07 Oct 2021 04:37:13 -0700 (PDT) Subject: Re: [PATCH 3/9] math: Simplify hypotf implementation To: Paul Zimmermann Cc: libc-alpha@sourceware.org References: <20211006180557.933826-1-adhemerval.zanella@linaro.org> <20211006180557.933826-4-adhemerval.zanella@linaro.org> From: Adhemerval Zanella Message-ID: Date: Thu, 7 Oct 2021 08:37:12 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_ASCII_DIVIDERS, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Oct 2021 11:37:16 -0000 On 07/10/2021 06:44, Paul Zimmermann wrote: > Dear Adhemerval, > >> Use isnan()/isinf() instead of GET_FLOAT_WORD and interger operations. > > interger -> integer Ack. > >> There is also no need to check for 0.0. >> >> The file Copyright is also change to use GPL, the implementation was > > change -> changed Ack. > >> complete change by 7c10fd3515f to use double precision instead of > > complete change -> completely changed ? Ack. > >> scaling and this change removes all the GET_FLOAT_WORD usage. >> >> Checked on x86_64-linux-gnu. >> --- >> sysdeps/ieee754/flt-32/e_hypotf.c | 57 +++++++++++++------------------ >> 1 file changed, 23 insertions(+), 34 deletions(-) >> >> diff --git a/sysdeps/ieee754/flt-32/e_hypotf.c b/sysdeps/ieee754/flt-32/e_hypotf.c >> index e770947dc1..6495a91cd4 100644 >> --- a/sysdeps/ieee754/flt-32/e_hypotf.c >> +++ b/sysdeps/ieee754/flt-32/e_hypotf.c >> @@ -1,46 +1,35 @@ >> -/* e_hypotf.c -- float version of e_hypot.c. >> - */ >> +/* Euclidean distance function. Float/Binary32 version. >> + Copyright (C) 2012-2021 Free Software Foundation, Inc. >> + This file is part of the GNU C Library. >> >> -/* >> - * ==================================================== >> - * Copyright (C) 1993 by Sun Microsystems, Inc. All rights reserved. >> - * >> - * Developed at SunPro, a Sun Microsystems, Inc. business. >> - * Permission to use, copy, modify, and distribute this >> - * software is freely granted, provided that this notice >> - * is preserved. >> - * ==================================================== >> - */ >> + The GNU C Library is free software; you can redistribute it and/or >> + modify it under the terms of the GNU Lesser General Public >> + License as published by the Free Software Foundation; either >> + version 2.1 of the License, or (at your option) any later version. >> + >> + The GNU C Library is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + Lesser General Public License for more details. >> + >> + You should have received a copy of the GNU Lesser General Public >> + License along with the GNU C Library; if not, see >> + . */ >> >> #include >> #include >> #include >> >> float >> -__ieee754_hypotf(float x, float y) >> +__ieee754_hypotf (float x, float y) >> { >> - double d_x, d_y; >> - int32_t ha, hb; >> - >> - GET_FLOAT_WORD(ha,x); >> - ha &= 0x7fffffff; >> - GET_FLOAT_WORD(hb,y); >> - hb &= 0x7fffffff; >> - if (ha == 0x7f800000 && !issignaling (y)) >> - return fabsf(x); >> - else if (hb == 0x7f800000 && !issignaling (x)) >> - return fabsf(y); >> - else if (ha > 0x7f800000 || hb > 0x7f800000) >> - return fabsf(x) * fabsf(y); >> - else if (ha == 0) >> - return fabsf(y); >> - else if (hb == 0) >> - return fabsf(x); >> - >> - d_x = (double) x; >> - d_y = (double) y; >> + if ((isinf (x) || isinf (y)) >> + && !issignaling (x) && !issignaling (y)) >> + return INFINITY; >> + if (isnan (x) || isnan (y)) >> + return x + y; > > why not test NaN before +/-Inf, to avoid the issignaling test? Because NaN should be returned iff there is a signaling one, for instance hypot (qNaN, Inf) should return Inf. To test NaN before Inf we would need to: if ((isnan (x) || isnan (y)) && (issignaling (x) || issignaling (y))) return x + y; if (isinf (x) || isinf (y)) return INFINITY; Which is essentially the same. > >> - return (float) sqrt(d_x * d_x + d_y * d_y); >> + return sqrt ((double) x * (double) x + (double) y * (double) y); > > a double-rounding problem can happen here, but anyway the function is not > correctly rounded Yeah, but I think this is preexisting issue in the current approach. > >> } >> #ifndef __ieee754_hypotf >> libm_alias_finite (__ieee754_hypotf, __hypotf) >> -- >> 2.30.2 >> >> > > Paul >