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]
