Re: About GSOC.

[Tejas Joshi <tejasjoshi9673@gmail.com>]

Hello.

> NaN, and you should make sure it behaves accordingly.  (If it should never
> be called for them, a gcc_assert would be appropriate.)

I can't find any documentation about how and when to use gcc_assert.
But I used it looking at the comment at its definition and locations
it is used, is this appropriate? Or is it supposed to be used before
calling the function? :

+bool
+is_even (REAL_VALUE_TYPE *r)
+{
+  /* The function is not supposed to use for Inf and NaN. */
+  gcc_assert (r->cl != rvc_inf);
+  gcc_assert (r->cl != rvc_nan);

> So n is the bit position, and w is the word position, of the bit with
> value 1; n-1 is the position of the bit with value 0.5.
> If n is a multiple of HOST_BITS_PER_LONG (that is, the bits with values
> 0.5 and 1 are in different words), this will incorrectly return false when
> the 0.5 bit is set.

I did not understand this. What is the bit with value 1?
But when n is a multiple of HOST_BITS_PER_LONG, the function was
computing value of w wrong (e.g. for number 2^63 + 0.5). At such time,
would the following improvisation be acceptable in is_halfway_below?

+bool
+is_halfway_below (const REAL_VALUE_TYPE *r)
+{
+  /* The function is not supposed to use for Inf and NaN. */
+  gcc_assert (r->cl != rvc_inf);
+  gcc_assert (r->cl != rvc_nan);
+  int i;
+
+  /* For numbers (-0.5,0) and (0,0.5). */
+  if (REAL_EXP (r) < 0)
+    return false;
+
+  else if (REAL_EXP (r) <= SIGNIFICAND_BITS)
+  {
+    unsigned int n = SIGNIFICAND_BITS - REAL_EXP (r);
+    int w = n / HOST_BITS_PER_LONG;
+
+    if (n % HOST_BITS_PER_LONG == 0)
+    {
+      if (w > 0)
+        w = w - 1;
+      else
+        w = 0;
+    }
+
+    for (i = 0; i < w; ++i)
+    {
+      if (r->sig[i] != 0)
+        return false;
+    }

Thanks.


On Mon, 3 Jun 2019 at 22:08, Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Fri, 31 May 2019, Tejas Joshi wrote:
>
> > +/* Return true if integer part of R is even, else return false. */
> > +
> > +bool
> > +is_even (REAL_VALUE_TYPE *r)
> > +{
> > +  if (REAL_EXP (r) <= 0)
> > +    return false;
>
> But the integer part (truncation towards 0) of something in the interval
> (-1, 1) is of course even.
>
> > +  else if (REAL_EXP (r) < SIGNIFICAND_BITS)
> > +  {
> > +    unsigned int n = SIGNIFICAND_BITS - REAL_EXP (r);
> > +    int w = n / HOST_BITS_PER_LONG;
> > +
> > +    unsigned long num = ((unsigned long)1 << (n % HOST_BITS_PER_LONG));
> > +
> > +    if ((r->sig[w] & num) == 0)
> > +      return true;
> > +  }
> > +  return false;
> > +}
>
> Suppose REAL_EXP (r) == SIGNIFICAND_BITS.  Then you still need to check
> the low bit in that case.
>
> Suppose REAL_EXP (r) > SIGNIFICAND_BITS.  Then the number is definitely
> even, so you should return true, not false.
>
> The comment on this function needs to define what it does for infinity /
> NaN, and you should make sure it behaves accordingly.  (If it should never
> be called for them, a gcc_assert would be appropriate.)
>
> What does this function do for zero?  It should, of course, return that it
> is even.
>
> > +/* Return true if R is halfway between two integers, else return false. */
>
> Again, define what this does for infinity / NaN and make sure it behaves
> accordingly.
>
> > +bool
> > +is_halfway_below (const REAL_VALUE_TYPE *r)
> > +{
> > +  if (REAL_EXP (r) < 0)
> > +    return false;
> > +
> > +  if (REAL_EXP (r) == 0)
> > +  {
> > +    unsigned long temp = ((unsigned long)1 << 63);
>
> unsigned long might be 32-bit; you can't assume it's 64-bit.  You may mean
> (HOST_BITS_PER_LONG - 1) instead of 63.
>
> > +    if (((r->sig[SIGSZ-1] & temp) != 0) && ((r->sig[SIGSZ-1] & (temp-1)) == 0))
> > +      return true;
> > +    else
> > +      return false;
>
> This appears only to be checking the high word, not lower bits.
>
> > +  else if (REAL_EXP (r) < SIGNIFICAND_BITS)
> > +  {
> > +    unsigned int n = SIGNIFICAND_BITS - REAL_EXP (r);
> > +    int i, w = n / HOST_BITS_PER_LONG;
>
> So n is the bit position, and w is the word position, of the bit with
> value 1; n-1 is the position of the bit with value 0.5.
>
> > +    for (i = 0; i < w; ++i)
> > +    {
> > +      if (r->sig[i] != 0)
> > +        return false;
> > +    }
>
> If n is a multiple of HOST_BITS_PER_LONG (that is, the bits with values
> 0.5 and 1 are in different words), this will incorrectly return false when
> the 0.5 bit is set.
>
> > +    unsigned long num = ((unsigned long)1 << ((n - 1) % HOST_BITS_PER_LONG));
> > +
> > +    if (((r->sig[w] & num) != 0) && ((r->sig[w] & (num-1)) == 0))
> > +      return true;
>
> And this also needs updating to handle the case where 0.5 and 1 are in
> different words correctly; currently it's checking bits that are all one
> word too high.  It's possible that for both issues, you want w to be the
> word with the 0.5 bit, not the word with the 1 bit.
>
> For all the above, please add appropriate tests in the testsuite (for
> example, where 0.5 and 1 are in different words and the above would have
> produced incorrect results).
>
> --
> Joseph S. Myers
> joseph@codesourcery.com