Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 20 Jun 2013 08:06:31 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Eitan Adler <lists@eitanadler.com>
Cc:        freebsd-numerics@freebsd.org
Subject:   Re: operation precedence bug: lib/msun/src
Message-ID:  <20130620071226.L1067@besplex.bde.org>
In-Reply-To: <CAF6rxg=7AQ-5qnOu0_neZMG-=xQTL5wjM%2Bt_qopeVN4uxg8g5w@mail.gmail.com>
References:  <CAF6rxg=7AQ-5qnOu0_neZMG-=xQTL5wjM%2Bt_qopeVN4uxg8g5w@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 19 Jun 2013, Eitan Adler wrote:

> Does the following look correct?

No :-).

>    Fix operation precedence bug: != comes before ^ so add required parens

According to das, the code was correct, though unclear.  The change
adds unrequired parentheses.  Since these are in the wrong place, they
change the object code and probably break it.

> diff --git a/lib/msun/src/s_fma.c b/lib/msun/src/s_fma.c
> index 452bece..406ff47 100644
> --- a/lib/msun/src/s_fma.c
> +++ b/lib/msun/src/s_fma.c
> @@ -117,7 +117,7 @@ add_and_denormalize(double a, double b, int scale)
> 	if (sum.lo != 0) {
> 		EXTRACT_WORD64(hibits, sum.hi);
> 		bits_lost = -((int)(hibits >> 52) & 0x7ff) - scale + 1;
> -		if (bits_lost != 1 ^ (int)(hibits & 1)) {
> +		if (bits_lost != (1 ^ (int)(hibits & 1))) {
> 			/* hibits += (int)copysign(1.0, sum.hi * sum.lo) */
> 			EXTRACT_WORD64(lobits, sum.lo);
> 			hibits += 1 - (((hibits ^ lobits) >> 62) & 2);
> diff --git a/lib/msun/src/s_fmal.c b/lib/msun/src/s_fmal.c
> index 9271901..ec4cc4d 100644
> --- a/lib/msun/src/s_fmal.c
> +++ b/lib/msun/src/s_fmal.c
> @@ -113,7 +113,7 @@ add_and_denormalize(long double a, long double b, int scale)
> 	if (sum.lo != 0) {
> 		u.e = sum.hi;
> 		bits_lost = -u.bits.exp - scale + 1;
> -		if (bits_lost != 1 ^ (int)(u.bits.manl & 1))
> +		if (bits_lost != (1 ^ (int)(u.bits.manl & 1)))
> 			sum.hi = nextafterl(sum.hi, INFINITY * sum.lo);
> 	}
> 	return (ldexp(sum.hi, scale));

I use the following fix (put the parentheses in the right place), but
haven't actually tested it:

% diff -u2 s_fma.c~ s_fma.c
% --- s_fma.c~	Thu May 30 18:15:37 2013
% +++ s_fma.c	Thu May 30 18:15:38 2013
% @@ -118,5 +118,5 @@
%  		EXTRACT_WORD64(hibits, sum.hi);
%  		bits_lost = -((int)(hibits >> 52) & 0x7ff) - scale + 1;
% -		if (bits_lost != 1 ^ (int)(hibits & 1)) {
% +		if ((bits_lost != 1) ^ (int)(hibits & 1)) {
%  			/* hibits += (int)copysign(1.0, sum.hi * sum.lo) */
%  			EXTRACT_WORD64(lobits, sum.lo);
% diff -u2 s_fmal.c~ s_fmal.c
% --- s_fmal.c~	Thu May 30 18:16:08 2013
% +++ s_fmal.c	Thu May 30 18:16:09 2013
% @@ -114,5 +114,5 @@
%  		u.e = sum.hi;
%  		bits_lost = -u.bits.exp - scale + 1;
% -		if (bits_lost != 1 ^ (int)(u.bits.manl & 1))
% +		if ((bits_lost != 1) ^ (int)(u.bits.manl & 1))
%  			sum.hi = nextafterl(sum.hi, INFINITY * sum.lo);
%  	}

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130620071226.L1067>