Re: [PATCH] Use fabsl instead of fabs on long double operand in floatscan.c

[Rich Felker <dalias@libc.org>]

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@mozilla.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
>