From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/14827 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] Use fabsl instead of fabs on long double operand in floatscan.c Date: Fri, 18 Oct 2019 10:23:27 -0400 Message-ID: <20191018142327.GY16318@brightrain.aerifal.cx> References: Reply-To: musl@lists.openwall.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="15408"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-14843-gllmg-musl=m.gmane.org@lists.openwall.com Fri Oct 18 16:23:44 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.89) (envelope-from ) id 1iLTAN-0003uK-Ec for gllmg-musl@m.gmane.org; Fri, 18 Oct 2019 16:23:43 +0200 Original-Received: (qmail 7240 invoked by uid 550); 18 Oct 2019 14:23:41 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 7222 invoked from network); 18 Oct 2019 14:23:40 -0000 Content-Disposition: inline In-Reply-To: Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:14827 Archived-At: On Fri, Oct 18, 2019 at 07:02:11AM -0700, Dan Gohman wrote: > This fixes a compiler warning with clang: > > floatscan.c:304:13: warning: absolute value function 'fabs' given an > argument of type 'long double' but has parameter of type 'double' which may > cause truncation of value [-Wabsolute-value]. > > This does change the behavior of the expression because the value is no > longer rounded to double, however from my reading of the code, the rounding > doesn't seem intended. However, if it is, I suggest introducing an explicit > cast, to document the intent. > > Dan > From 1fecc521dc43b25366cd4a3062964ff3abc7506e Mon Sep 17 00:00:00 2001 > From: Dan Gohman > Date: Fri, 18 Oct 2019 06:22:49 -0700 > Subject: [PATCH] Use `fabsl` instead of `fabs` on long double in floatscan.c > > This fixes a compiler warning: > > floatscan.c:304:13: warning: absolute value function 'fabs' given an argument > of type 'long double' but has parameter of type 'double' which may cause > truncation of value [-Wabsolute-value] If correct, the needed commit message here is not the warning but the behavioral fix the commit makes, if any. I haven't looked in detail yet, but I suspect there may be an issue with rounding and spurious raising of flags here, so I think the patch is correct and does matter. > --- > src/internal/floatscan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/internal/floatscan.c b/src/internal/floatscan.c > index 278bf250..99a1ec29 100644 > --- a/src/internal/floatscan.c > +++ b/src/internal/floatscan.c > @@ -301,7 +301,7 @@ static long double decfloat(FILE *f, int c, int bits, int emin, int sign, int po > y -= bias; > > if ((e2+LDBL_MANT_DIG & INT_MAX) > emax-5) { > - if (fabs(y) >= CONCAT(0x1p, LDBL_MANT_DIG)) { > + if (fabsl(y) >= CONCAT(CONCAT(0x1p, LDBL_MANT_DIG), l)) { The double CONCAT definitely isn't needed. Comparison of fabsl(y) against 0x1p[LDBL_MANT_DIG] is well-defined, and has the same result. > if (denormal && bits==LDBL_MANT_DIG+e2-emin) > denormal = 0; > y *= 0.5; > -- > 2.17.1 >