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 <[email protected]> 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
> [email protected]
