Re: About GSOC.

[Joseph Myers <joseph@codesourcery.com>]

On Wed, 8 May 2019, Tejas Joshi wrote:

> Hello.
> As per my understanding, 3.5 would be represented in GCC as follows :
> r->uexp  = 2
> and
> r->sig[2] = 1110000....00 in binary 64 bit. (first 2 bits being 3 and
> following 1000....0 being 0.5, which later only happens for halfway cases)
> So, if we clear out the significand part and the immediate bit to the right
> which represent 0.5, the entire significand would become 0 due to bit-wise
> ANDing.
> 
> > +  tempsig[w] &= (((unsigned long)1 << ((n % HOST_BITS_PER_LONG) - 1)) -
> > 1);
> >
> 
> That is what the following line intend to do. The clearing part would
> change the significand, that's why significand was copied to a temporary

That much is fine.  My issues are two other things:

* The function would wrongly return true for 3, not just for 3.5, because 
it never checks the bit representing 0.5.  (If you don't care what it 
returns for 3, see my previous point about every function needing a 
comment defining its semantics.  Without such a comment, I have to guess, 
and my guess here is that the function should return true for 3.5 but 
false for 3 and for 3.5000...0001.)

* The function would wrongly return true for 3.5000...0001, if there are 
enough 0s that all those low bits in sig[2] are 0, but some low bits in 
sig[1] or sig[0] are not 0.

And also:

* You should never need to modify parts of (a copy of) the significand in 
place.  Compare low parts of the significand (masked as needed) with 0.  
If not 0, just return false.  Likewise for comparing the 0.5 bit with 1.  
It's not that copying and modifying in place results in incorrect logic, 
it's simply excessively convoluted compared to things like:

  if ((something & mask) != 0)
    return false

(the function is probably twice as long as necessary because of that 
copying).

> array for checking. This logic is inspired by the clear_significand_below
> function. Or isn't this the way it was meant to be implemented? Also, why
> unsigned long sig[SIGSZ] has to be an array?

What would it be other than an array?  It can't be a single scalar because 
floating-point significands may be longer than any supported integer type 
on the host (remember the IEEE binary128 case).  And if you made it a 
sequence of individually named fields, a load of loops would need to be 
manually unrolled, which would be much more error prone and hard to read.

-- 
Joseph S. Myers
joseph@codesourcery.com