X-Authentication-Warning: delorie.com: mailnull set sender to djgpp-workers-bounces using -f
Date: Sat, 02 Mar 2002 20:54:20 +0200
From: "Eli Zaretskii" <eliz AT is DOT elta DOT co DOT il>
Sender: halo1 AT zahav DOT net DOT il
To: djgpp-workers AT delorie DOT com
Message-Id: <8011-Sat02Mar2002205420+0200-eliz@is.elta.co.il>
X-Mailer: emacs 21.2.50 (via feedmail 8 I) and Blat ver 1.8.9
In-reply-to: <200203021531.QAA04573@father.ludd.luth.se> (message from Martin
	Str|mberg on Sat, 2 Mar 2002 16:31:16 +0100 (MET))
Subject: Re: libm signed/unsigned warnings
References:  <200203021531 DOT QAA04573 AT father DOT ludd DOT luth DOT se>
Reply-To: djgpp-workers AT delorie DOT com
Errors-To: nobody AT delorie DOT com
X-Mailing-List: djgpp-workers AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk

> From: Martin Str|mberg <ams AT ludd DOT luth DOT se>
> Date: Sat, 2 Mar 2002 16:31:16 +0100 (MET)
> 
> -                   if((j<<(52-k))==ly) yisint = 2-(j&1);
> +                   if((((__uint32_t)(j))<<(52-k))==ly) yisint = 2-(j&1);

This seems okay, but is mighty ugly.  Can you find a better way of
shutting up GCC in this case?

> -               if(((t1&sign)==sign)&&(s1&sign)==0) s0 += 1;
> +               if((t1&sign)&&(s1&sign)==0) s0 += 1;
>                 ix0 -= t;
>                 if (ix1 < t1) ix0 -= 1;
>                 ix1 -= t1;
> 
> __int32_t sign = (int)0x80000000U;
> __uint32_t t1,s1;
> 
> (t1&sign)==sign checks if the sign bit is set or not. The comparision
> with sign should be uneccessary.

I don't understand the original problem here: both t1 and sign are
int's, so why does GCC complain?  What is the exact language of the
warning?

> -               if(ix>0x80000000U) z = (invsqrtpi*cc)/__ieee754_sqrtf(x);
> +               if((__uint32_t)ix>0x80000000U) z = (invsqrtpi*cc)/__ieee754_sqrtf(x);

I tend to think that this is a bug in the original code, since ix is
computed a few lines above this like so:

	ix = hx&0x7fffffff;

So its high bit can never be set, and the comparison should always
fail.  Am I missing something?

Perhaps you could add a call to abort() into that if clause, and see
if it ever gets called when you run the cygnus test suite from djtst.

> -               if(ix>0x80000000U) z = (invsqrtpi*cc)/__ieee754_sqrtf(y);
> +               if((__uint32_t)ix>0x80000000U) z = (invsqrtpi*cc)/__ieee754_sqrtf(y);

Same here.

> -       for(i=1;i<n&&ib!=0xff800000U;i++){
> +       for(i=1;i<n&&(__uint32_t)ib!=0xff800000U;i++){

Can this be handled by making ib an unsigned int?

> Perhaps the right correction should be to make 0xff800000U
> 0xff800000?

That would just produce a different warning ("constant is so large
that it's unsigned"), and you are toast again.

> -       else if (j==0xc3160000U){                       /* z == -150 */
> +       else if ((__uint32_t)j==0xc3160000U){           /* z == -150 */

I'd prefer instead to cast 0xc3160000U to __int32_t.

> The author must have wanted j be unsigned here, otherwise he wouldn't
> have coded 0xc3160000U.

Actually, I think the original fdlibm code was without the U; U was
added because some previous version of GCC whined about a constant
that is too large for signed int.

> -           if(n<32&&(ix&0xffffff00U)!=npio2_hw[n-1]) {
> +           if(n<32&&((__int32_t)(ix&0xffffff00U))!=npio2_hw[n-1]) {

This is okay.  (I think the fact that GCC complains about == and !=
is a bug, btw.  Comparison between signed and unsigned can only yild
problems if they compare for less-than or greater-than.)

>                         j = i1 + (1<<(52-j_0));
> -                       if(j<i1) i0+=1; /* got a carry */
> +                       if( (0<=i1) && (j<(__uint32_t)i1) ) i0+=1;      /* got a carry */
>                         i1 = j;

Hmm... seems okay.

> -                       if(j<i1) i0 +=1 ;       /* got a carry */
> +                       if( (0<=i1) && (j<(__uint32_t)i1) ) i0 +=1 ;    /* got a carry */

Same here.

Please run the cygnus test suite after the changes, and compare the
results with tests/cygnus/standard.results.  Any changes should be
scrutinized.

Thanks for working on this.