Mail Archives: djgpp-workers/2002/03/02/15:20:32
> 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.
- Raw text -