[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191018142327.GY16318@brightrain.aerifal.cx>
Date: Fri, 18 Oct 2019 10:23:27 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Use fabsl instead of fabs on long double operand
in floatscan.c
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 <sunfish@...illa.com>
> 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
>
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
