This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v4] Add nextup and nextdown math functions
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Rajalakshmi Srinivasaraghavan <raji at linux dot vnet dot ibm dot com>
- Cc: Rical Jasan <ricaljasan at pacific dot net>, <libc-alpha at sourceware dot org>
- Date: Tue, 14 Jun 2016 14:29:16 +0000
- Subject: Re: [PATCH v4] Add nextup and nextdown math functions
- Authentication-results: sourceware.org; auth=none
- References: <57599ABC dot 9070609 at linux dot vnet dot ibm dot com> <575A3EAF dot 4040209 at pacific dot net> <575D8A3C dot 6020908 at linux dot vnet dot ibm dot com> <575DE817 dot 8090101 at pacific dot net> <575F8EF0 dot 6040305 at linux dot vnet dot ibm dot com>
On Tue, 14 Jun 2016, Rajalakshmi Srinivasaraghavan wrote:
> +#if TEST_COND_m68k96
> + TEST_ff_f (nextafter, -0x0.fffffffep-16383L, 0.0L, -0x3.fffffff7fffffff0p-16385L, INEXACT_EXCEPTION|UNDERFLOW_EXCEPTION|ERRNO_ERANGE),
> #endif
I think the test you have there is wrong for m68k96. For intel96, the
smallest normal exponent is -16382; for m68k96, the smallest normal
exponent is -16383. This input has exponent -16384. So for m68k96 it's a
63-bit mantissa. The input has bits <31 1s><32 0s>. So the correct
output has bits <30 1s><0><32 1s>. But your output has bits <30 1s><0><31
1s>. That is, the final digit in your output should be 8, not 0.
In general, it seems very confusing in such tests to show the exponent
differently in the output from the input. Why do you show the input with
exponent -16383 but the output with exponent -16385? That applies to
other tests in this patch as well.
> + TEST_f_f (nextup, 0, min_subnorm_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> + TEST_f_f (nextup, plus_zero, min_subnorm_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
0 and plus_zero are the same thing. These are duplicate tests.
> + TEST_f_f (nextup, snan_value, qnan_value, INVALID_EXCEPTION),
> + TEST_f_f (nextup, -snan_value, qnan_value, INVALID_EXCEPTION),
As exactly defined functions, it should be
NO_INEXACT_EXCEPTION|INVALID_EXCEPTION for the exception expectations for
nextup and nextdown on sNaN input (whereas for functions that aren't
exactly defined, Annex F says nothing about "inexact" for sNaN inputs,
although no exceptions are allowed for qNaN inputs).
> +#if TEST_COND_m68k96
> + TEST_f_f (nextup, -0x0.fffffffep-16383L, -0x3.fffffff7fffffff0p-16385L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
Same issue as with the nextafter test.
> + TEST_f_f (nextdown, 0, -min_subnorm_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> + TEST_f_f (nextdown, plus_zero, -min_subnorm_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
Same point about duplicate tests.
> + TEST_f_f (nextdown, snan_value, qnan_value, INVALID_EXCEPTION),
> + TEST_f_f (nextdown, -snan_value, qnan_value, INVALID_EXCEPTION),
Same point about exception expectations.
> +#if TEST_COND_m68k96
> + TEST_f_f (nextdown, -0x3.fffffff7fffffff0p-16385L, -0x0.fffffffep-16383L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
Same point about correct m68k96 values.
> +#ifdef NO_LONG_DOUBLE
> + strong_alias (__nextdown, __nextdownl)
> + weak_alias (__nextdown, nextdownl)
> +#endif
These *_alias calls aren't normally indented.
> + if ((ix | lx) == 0)
> + {
> + INSERT_WORDS (x, 0, 1); /* Return minsubnormal. */
> + return x;
Would seem simpler just to do "return DBL_TRUE_MIN;", having included
<float.h> (and then no comment is needed, since the plain meaning of the
code says what it does).
> +#ifdef NO_LONG_DOUBLE
> + strong_alias (__nextup, __nextupl)
> + weak_alias (__nextup, nextupl)
> +#endif
Same comment about indentation.
> + if (ix == 0)
> + {
> + SET_FLOAT_WORD (x, 1); /* Return minsubnormal. */
> + return x;
Return FLT_TRUE_MIN.
> + if ((ix | lx) == 0)
> + {
> + /* Return minsubnormal. */
> + SET_LDOUBLE_WORDS64 (x, 0, 1);
Return LDBL_TRUE_MIN.
> + if (ihx == 0)
> + {
> + hx = 1;
> + INSERT_WORDS64 (xhi, hx);
> + x = xhi;
> + return x;
Likewise.
> + if ((ix | hx | lx) == 0)
> + {
> + SET_LDOUBLE_WORDS (x, 0, 0, 1); /* Return minsubnormal. */
> + return x;
> + }
Likewise.
--
Joseph S. Myers
joseph@codesourcery.com