This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v4] Add nextup and nextdown math functions


A few additional things from my attempt to review the code.  Mostly
coding style/formatting.

On 06/12/2016 09:13 AM, Rajalakshmi Srinivasaraghavan wrote:
> 
>>From e13a0b8b88b996be0364a374a32d60329a6b6552 Mon Sep 17 00:00:00 2001
> From: Rajalakshmi Srinivasaraghavan <raji@linux.vnet.ibm.com>
> Date: Sun, 12 Jun 2016 11:47:18 -0400
> Subject: [PATCH] Add nextup and nextdown math functions
> 
> TS 18661 adds nextup and nextdown functions alongside nextafter to provide
> support for float128 equivalent to it.  This patch adds nextupl, nextup,
> nextupf, nextdownl, nextdown and nextdownf to libm before float128 support.
> 
> The nextup functions return the next representable value in the direction of
> positive infinity and the nextdown functions return the next representable
> value in the direction of negative infinity.  These are currently enabled
> as GNU extensions.
> 
...

> diff --git a/manual/libm-err-tab.pl b/manual/libm-err-tab.pl
> index 3846afc..c2792c5 100755
> --- a/manual/libm-err-tab.pl
> +++ b/manual/libm-err-tab.pl
> @@ -73,9 +73,9 @@ use vars qw (%results @all_floats %suffices @all_functions);
>      "fmax", "fmin", "fmod", "frexp", "gamma", "hypot",
>      "ilogb", "j0", "j1", "jn", "lgamma", "lrint",
>      "llrint", "log", "log10", "log1p", "log2", "logb", "lround",
> -    "llround", "modf", "nearbyint", "nextafter", "nexttoward", "pow",
> -    "remainder", "remquo", "rint", "round", "scalb", "scalbn", "scalbln",
> -    "sin", "sincos", "sinh", "sqrt", "tan", "tanh", "tgamma",
> +    "llround", "modf", "nearbyint", "nextafter", "nextdown", "nexttoward",
> +    "nextup", "pow", "remainder", "remquo", "rint", "round", "scalb",
> +    "scalbn", "sin", "sincos", "sinh", "sqrt", "tan", "tanh", "tgamma",
>      "trunc", "y0", "y1", "yn" );
>  # fpclassify, isnormal, isfinite, isinf, isnan, issignaling, signbit,
>  # isgreater, isgreaterequal, isless, islessequal, islessgreater, isunordered
> diff --git a/math/Makefile b/math/Makefile
> index 6cd3cf1..4f14181 100644
> --- a/math/Makefile
> +++ b/math/Makefile
> @@ -63,7 +63,7 @@ libm-calls = e_acos e_acosh e_asin e_atan2 e_atanh e_cosh e_exp e_fmod	\
>  	     s_fma s_lrint s_llrint s_lround s_llround e_exp10 w_log2	\
>  	     s_issignaling $(calls:s_%=m_%) x2y2m1 k_casinh	\
>  	     gamma_product k_standard lgamma_neg lgamma_product		\
> -	     w_lgamma_compat
> +	     w_lgamma_compat s_nextup s_nextdown
>  
>  dbl-only-routines := branred doasin dosincos halfulp mpa mpatan2	\
>  		     mpatan mpexp mplog mpsqrt mptan sincos32 slowexp	\
> diff --git a/math/Versions b/math/Versions
> index e6a597c..467d7ed 100644
> --- a/math/Versions
> +++ b/math/Versions
> @@ -210,4 +210,8 @@ libm {
>      # dynamic symbol for signgam but not __signgam.
>      lgamma; lgammaf; lgammal; __signgam;
>    }
> +  GLIBC_2.24 {
> +    nextup; nextupf; nextupl;
> +    nextdown; nextdownf; nextdownl;
> +  }
>  }
> diff --git a/math/bits/mathcalls.h b/math/bits/mathcalls.h
> index 9a7b3f0..e2cf49f 100644
> --- a/math/bits/mathcalls.h
> +++ b/math/bits/mathcalls.h
> @@ -294,6 +294,13 @@ __MATHCALLX (nextafter,, (_Mdouble_ __x, _Mdouble_ __y), (__const__));
>  __MATHCALLX (nexttoward,, (_Mdouble_ __x, long double __y), (__const__));
>  # endif
>  
> +#ifdef __USE_GNU
> +/* Return X - epsilon.  */
> +__MATHCALL (nextdown,, (_Mdouble_ __x));
> +/* Return X + epsilon.  */
> +__MATHCALL (nextup,, (_Mdouble_ __x));
> +# endif
> +
>  /* Return the remainder of integer divison X / Y with infinite precision.  */
>  __MATHCALL (remainder,, (_Mdouble_ __x, _Mdouble_ __y));
>  
> diff --git a/math/libm-test.inc b/math/libm-test.inc
> index 3d901ae..56fe536 100644
> --- a/math/libm-test.inc
> +++ b/math/libm-test.inc
> @@ -9984,11 +9984,28 @@ static const struct test_ff_f_data nextafter_test_data[] =
>      TEST_ff_f (nextafter, -0x0.fffffffep-16382L, 0.0L, -0x0.fffffffdfffffffep-16382L, INEXACT_EXCEPTION|UNDERFLOW_EXCEPTION|ERRNO_ERANGE),
>  #endif
>  
> -#if MANT_DIG >= 64
> -    // XXX Enable once gcc is fixed.
> -    //TEST_ff_f (nextafter, 0x0.00000040000000000000p-16385L, -0.1L, 0x0.0000003ffffffff00000p-16385L),
> +#if TEST_COND_binary32
> +    TEST_ff_f (nextafter, 1.0, 2.0, 0x1.000002p0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, 1.0, 0.9, 0x0.ffffffp0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, -1.0, -2.0, -0x1.000002p0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, -1.0, 2.0, -0x0.ffffffp0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_binary64
> +    TEST_ff_f (nextafter, 1.0, 2.0, 0x1.0000000000001p+0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, 1.0, 0.9, 0x1.fffffffffffffp-1, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, -1.0, -2.0, -0x1.0000000000001p+0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, -1.0, 2.0, -0x1.fffffffffffffp-1, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_m68k96 || TEST_COND_intel96
> +    TEST_ff_f (nextafter, 1.0L, 2.0L, 0x8.0000000000000010p-3L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, 1.0L, -2.0L, 0xf.fffffffffffffff0p-4L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, -1.0L, -2.0L, -0x8.0000000000000010p-3L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, -1.0L, 2.0L, -0xf.fffffffffffffff0p-4L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_m68k96
> +    TEST_ff_f (nextafter, -0x0.fffffffep-16383L, 0.0L, -0x3.fffffff7fffffff0p-16385L, INEXACT_EXCEPTION|UNDERFLOW_EXCEPTION|ERRNO_ERANGE),
>  #endif
> -#if MANT_DIG == 106
> +#if TEST_COND_ibm128
>      TEST_ff_f (nextafter, 1.0L, -10.0L, 1.0L-0x1p-106L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
>      TEST_ff_f (nextafter, 1.0L, 10.0L, 1.0L+0x1p-105L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
>      TEST_ff_f (nextafter, 1.0L-0x1p-106L, 10.0L, 1.0L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> @@ -9996,6 +10013,12 @@ static const struct test_ff_f_data nextafter_test_data[] =
>      TEST_ff_f (nextafter, -1.0L, 10.0L, -1.0L+0x1p-106L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
>      TEST_ff_f (nextafter, -1.0L+0x1p-106L, -10.0L, -1.0L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
>  #endif
> +#if TEST_COND_binary128
> +    TEST_ff_f (nextafter, 1.0L, 10.0L, 0x1.0000000000000000000000000001p0L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, 1.0L, -10.0L, 0x1.ffffffffffffffffffffffffffffp-1L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, -1.0L, 10.0L, -0x1.ffffffffffffffffffffffffffffp-1L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_ff_f (nextafter, -1.0L, -10.0L, -0x1.0000000000000000000000000001p+0L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
>  
>      /* XXX We need the hexadecimal FP number representation here for further
>         tests.  */
> @@ -10006,6 +10029,102 @@ nextafter_test (void)
>  {
>    ALL_RM_TEST (nextafter, 1, nextafter_test_data, RUN_TEST_LOOP_ff_f, END);
>  }
> +static const struct test_f_f_data nextup_test_data[] =
> +  {
> +    TEST_f_f (nextup, minus_zero, min_subnorm_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    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),
> +    TEST_f_f (nextup, -min_subnorm_value, minus_zero, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, max_value, plus_infty, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, plus_infty, plus_infty, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, minus_infty, -max_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, qnan_value, qnan_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, -qnan_value, qnan_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, snan_value, qnan_value, INVALID_EXCEPTION),
> +    TEST_f_f (nextup, -snan_value, qnan_value, INVALID_EXCEPTION),
> +#if TEST_COND_binary32
> +    TEST_f_f (nextup, 1.0, 0x1.000002p0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, -1.0, -0x0.ffffffp0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_binary64
> +    TEST_f_f (nextup, 1.0, 0x1.0000000000001p+0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, -1.0, -0x1.fffffffffffffp-1, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_m68k96 || TEST_COND_intel96
> +    TEST_f_f (nextup, 1.0L, 0x8.0000000000000010p-3L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, -1.0L, -0xf.fffffffffffffff0p-4L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, 0xf.fffffffffffffff0p-4L, 1.0L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_intel96
> +    TEST_f_f (nextup, -0x0.fffffffep-16382L, -0x0.fffffffdfffffffep-16382L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_m68k96
> +    TEST_f_f (nextup, -0x0.fffffffep-16383L, -0x3.fffffff7fffffff0p-16385L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_ibm128
> +    TEST_f_f (nextup, 1.0L, 1.0L+0x1p-105L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, -1.0L-0x1p-105L, -1.0L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, -1.0L, -1.0L+0x1p-106L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_binary128
> +    TEST_f_f (nextup, 1.0L, 0x1.0000000000000000000000000001p0L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextup, -1.0L, -0x1.ffffffffffffffffffffffffffffp-1L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +  };
> +
> +static void
> +nextup_test (void)
> +{
> +  ALL_RM_TEST (nextup, 1, nextup_test_data, RUN_TEST_LOOP_f_f, END);
> +}
> +static const struct test_f_f_data nextdown_test_data[] =
> +  {
> +    TEST_f_f (nextdown, minus_zero, -min_subnorm_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    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),
> +    TEST_f_f (nextdown, min_subnorm_value, 0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, -max_value, minus_infty, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, plus_infty, max_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, minus_infty, minus_infty, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, qnan_value, qnan_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, -qnan_value, qnan_value, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, snan_value, qnan_value, INVALID_EXCEPTION),
> +    TEST_f_f (nextdown, -snan_value, qnan_value, INVALID_EXCEPTION),
> +#if TEST_COND_binary32
> +    TEST_f_f (nextdown, 1.0, 0x0.ffffffp0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, -1.0, -0x1.000002p0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_binary64
> +    TEST_f_f (nextdown, 1.0, 0x1.fffffffffffffp-1, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, -1.0, -0x1.0000000000001p+0, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_m68k96 || TEST_COND_intel96
> +    TEST_f_f (nextdown, -1.0L, -0x8.0000000000000010p-3L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, 1.0L, 0xf.fffffffffffffff0p-4L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, 0x8.0000000000000010p-3L, 1.0L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_intel96
> +    TEST_f_f (nextdown, -0x0.fffffffdfffffffep-16382L, -0x0.fffffffep-16382L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_m68k96
> +    TEST_f_f (nextdown, -0x3.fffffff7fffffff0p-16385L, -0x0.fffffffep-16383L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_ibm128
> +    TEST_f_f (nextdown, -1.0L, -1.0L-0x1p-105L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, 1.0L+0x1p-105L, 1.0L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, 1.0L, 1.0L-0x1p-106L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +#if TEST_COND_binary128
> +    TEST_f_f (nextdown, 1.0L, 0x1.ffffffffffffffffffffffffffffp-1L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +    TEST_f_f (nextdown, -1.0L, -0x1.0000000000000000000000000001p+0L, NO_INEXACT_EXCEPTION|ERRNO_UNCHANGED),
> +#endif
> +  };
> +
> +static void
> +nextdown_test (void)
> +{
> +  ALL_RM_TEST (nextdown, 1, nextdown_test_data, RUN_TEST_LOOP_f_f, END);
> +}
>  
>  
>  /* Note, the second argument is always typed as long double.  The j type
> @@ -12142,6 +12261,8 @@ main (int argc, char **argv)
>  
>    /* Manipulation functions:  */
>    copysign_test ();
> +  nextup_test();
> +  nextdown_test();
>    nextafter_test ();
>    nexttoward_test ();
>  

> diff --git a/math/s_nextdown.c b/math/s_nextdown.c
> new file mode 100644
> index 0000000..7b2e13f
> --- /dev/null
> +++ b/math/s_nextdown.c
> @@ -0,0 +1,35 @@
> +/* Return the next representable value less than x.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +/* nextdown(x) returns the least floating-point number in the
> +   format of x less than x.  */

I was originally trying to discern whether subnormal arguments were
always normalized, so this comment gave me pause.  By "format" do you
mean "type"?

This comment is present in a handful of the files, but not all.  If the
comment is important, it should probably be present in all of them.
Also, should it be moved right above the function?  The placement and
presence are inconsistent throughout.

> +
> +#include <math.h>
> +#include <math_private.h>
> +
> +double
> +__nextdown (double x)
> +{
> +  return -__nextup (-x);
> +}
> +
> +weak_alias (__nextdown, nextdown)
> +#ifdef NO_LONG_DOUBLE
> +  strong_alias (__nextdown, __nextdownl)
> +  weak_alias (__nextdown, nextdownl)
> +#endif
> diff --git a/math/s_nextdownf.c b/math/s_nextdownf.c
> new file mode 100644
> index 0000000..510472b
> --- /dev/null
> +++ b/math/s_nextdownf.c
> @@ -0,0 +1,31 @@
> +/* Return the next representable value less than x.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +/* nextdown(x) returns the least floating-point number in the
> +   format of x less than x.  */

same

> +
> +#include <math.h>
> +#include <math_private.h>
> +
> +float
> +__nextdownf (float x)
> +{
> +  return -__nextupf (-x);
> +}
> +
> +weak_alias (__nextdownf, nextdownf)
> diff --git a/math/s_nextdownl.c b/math/s_nextdownl.c
> new file mode 100644
> index 0000000..9f5b7f1
> --- /dev/null
> +++ b/math/s_nextdownl.c
> @@ -0,0 +1,31 @@
> +/* Return the next representable value less than x.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +/* nextdown(x) returns the least floating-point number in the
> +   format of x less than x.  */

same

> +
> +#include <math.h>
> +#include <math_private.h>
> +
> +long double
> +__nextdownl (long double x)
> +{
> +  return -__nextupl (-x);
> +}
> +
> +weak_alias (__nextdownl, nextdownl)
> diff --git a/math/test-tgmath.c b/math/test-tgmath.c
> index 0e978d1..70ab428 100644
> --- a/math/test-tgmath.c
> +++ b/math/test-tgmath.c
> @@ -50,7 +50,7 @@ int count_cdouble;
>  int count_cfloat;
>  int count_cldouble;
>  
> -#define NCALLS     115
> +#define NCALLS     119
>  #define NCALLS_INT 4
>  #define NCCALLS    47
>  
> @@ -274,7 +274,9 @@ F(compile_test) (void)
>    b = lgamma (lgamma (a));
>    a = rint (rint (x));
>    b = nextafter (nextafter (a, b), nextafter (c, x));
> -  a = nexttoward (nexttoward (x, a), c);
> +  a = nextdown (nextdown (a));
> +  b = nexttoward (nexttoward (x, a), c);
> +  a = nextup (nextup (a));
>    b = remainder (remainder (a, b), remainder (c, x));
>    a = scalb (scalb (x, a), (TYPE) (6));
>    k = scalbn (a, 7) + scalbln (c, 10l);
> @@ -777,6 +779,14 @@ TYPE
>  }
>  
>  TYPE
> +(F(nextdown)) (TYPE x)
> +{
> +  ++count;
> +  P ();
> +  return x;
> +}
> +
> +TYPE
>  (F(nexttoward)) (TYPE x, long double y)
>  {
>    ++count;
> @@ -785,6 +795,14 @@ TYPE
>  }
>  
>  TYPE
> +(F(nextup)) (TYPE x)
> +{
> +  ++count;
> +  P ();
> +  return x;
> +}
> +
> +TYPE
>  (F(remainder)) (TYPE x, TYPE y)
>  {
>    ++count;
> diff --git a/math/tgmath.h b/math/tgmath.h
> index ea2b611..42d7c2d 100644
> --- a/math/tgmath.h
> +++ b/math/tgmath.h
> @@ -391,6 +391,12 @@
>  /* Return the integer nearest X in the direction of the
>     prevailing rounding mode.  */
>  #define rint(Val) __TGMATH_UNARY_REAL_ONLY (Val, rint)

Should be a blank line here, I think.  Most of these are separated that way.

> +#ifdef __USE_GNU
> +/* Return X - epsilon. */

Two spaces after period.

> +#define nextdown(Val) __TGMATH_UNARY_REAL_ONLY (Val, nextdown)

# define, I believe (space).

> +/* Return X + epsilon. */

Two spaces.

> +#define nextup(Val) __TGMATH_UNARY_REAL_ONLY (Val, nextup)

# define

> +#endif
>  
>  /* Return X + epsilon if X < Y, X - epsilon if X > Y.  */
>  #define nextafter(Val1, Val2) __TGMATH_BINARY_REAL_ONLY (Val1, Val2, nextafter)
> diff --git a/sysdeps/ieee754/dbl-64/s_nextup.c b/sysdeps/ieee754/dbl-64/s_nextup.c
> new file mode 100644
> index 0000000..897e9cb
> --- /dev/null
> +++ b/sysdeps/ieee754/dbl-64/s_nextup.c
> @@ -0,0 +1,62 @@
> +/* Return the next representable value greater than x.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <math.h>
> +#include <math_private.h>
> +
> +/* nextup(x) returns the least floating-point number in the
> +   format of x that compares greater than x.  */

This seems most correct, wrt. formatting the beginning of the file and
commenting on the function.

> +double
> +__nextup (double x)
> +{
> +  int32_t hx, ix;
> +  u_int32_t lx;
> +
> +  EXTRACT_WORDS (hx, lx, x);
> +  ix = hx & 0x7fffffff;
> +
> +  if (((ix >= 0x7ff00000) && ((ix - 0x7ff00000) | lx) != 0))	/* x is nan */
> +    return x + x;
> +  if ((ix | lx) == 0)
> +    {
> +      INSERT_WORDS (x, 0, 1);	/* return +-minsubnormal */

I don't see anything that changes the sign (sign bit appears to be set
to 0).  Is +/- meant to refer to the fact nextdown returns -nextup?  I
don't think the "+-" is necessary.

Also, are comments like these allowable in GNU-style?  Looks like you
possibly inherited the style here from math/s_nextafter.c, which has a
copyright by Sun.  I see they made the "+-minsubnormal" comment as well.

All these comments (this file and others) are missing the double space
at the end.  Normally comments are sentences so the spaces follow a
period, but I'm not sure how everybody feels about comments like the one
below (x > 0), and whether that also requires 2 spaces.

> +      return x;
> +    }
> +  if (hx >= 0)
> +    {				/* x > 0 */
> +      if (isinf (x))
> +	return x;

I see a tab made the alignment off in my reply, but I can't find
anything about whether glibc is spaces-only or not.  Thought I'd point
out the whitespace issue, though.

> +      lx += 1;
> +      if (lx == 0)
> +	hx += 1;

Here too.

> +    }
> +  else
> +    {				/* x < 0 */
> +      if (lx == 0)
> +	hx -= 1;

and here.  Don't see it anywhere else in this file, although it happens
in others.

> +      lx -= 1;
> +    }
> +  INSERT_WORDS (x, hx, lx);
> +  return x;
> +}
> +
> +weak_alias (__nextup, nextup)
> +#ifdef NO_LONG_DOUBLE
> +  strong_alias (__nextup, __nextupl)
> +  weak_alias (__nextup, nextupl)
> +#endif
> diff --git a/sysdeps/ieee754/flt-32/s_nextupf.c b/sysdeps/ieee754/flt-32/s_nextupf.c
> new file mode 100644
> index 0000000..770182e
> --- /dev/null
> +++ b/sysdeps/ieee754/flt-32/s_nextupf.c
> @@ -0,0 +1,49 @@
> +/* Return the next representable value greater than x.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +#include <math.h>
> +#include <math_private.h>

We start losing the comments on the functions here, and there is no
longer a blank line between the copyright notice and the #includes,
which I think is the norm.

> +
> +float
> +__nextupf (float x)
> +{
> +  int32_t hx, ix;
> +
> +  GET_FLOAT_WORD (hx, x);
> +  ix = hx & 0x7fffffff;		/* |x| */
> +  if (ix == 0)
> +    {
> +      SET_FLOAT_WORD (x, 1);	/* return +-minsubnormal */
> +      return x;
> +    }
> +  if (ix > 0x7f800000)		/* x is nan */
> +    return x + x;
> +  if (hx >= 0)
> +    {				/* x > 0 */
> +      if (isinf (x))
> +	return x;
> +      hx += 1;
> +    }
> +  else
> +    {				/* x < 0 */
> +      hx -= 1;
> +    }

I believe braces are omitted for single-line statements, even if the
corresponding if had them.  (The GNU Coding Standards, Formatting Your
Source Code, has an example with an if with no braces and an else with
them.)

> +  SET_FLOAT_WORD (x, hx);
> +  return x;
> +}
> +
> +weak_alias (__nextupf, nextupf)
> diff --git a/sysdeps/ieee754/ldbl-128/s_nextupl.c b/sysdeps/ieee754/ldbl-128/s_nextupl.c
> new file mode 100644
> index 0000000..a97c054
> --- /dev/null
> +++ b/sysdeps/ieee754/ldbl-128/s_nextupl.c
> @@ -0,0 +1,58 @@
> +/* Return the next representable value greater than x.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +#include <math.h>
> +#include <math_private.h>
> +
> +long double
> +__nextupl (long double x)
> +{
> +  int64_t hx, ix;
> +  u_int64_t lx;
> +
> +  GET_LDOUBLE_WORDS64 (hx, lx, x);
> +  ix = hx & 0x7fffffffffffffffLL;	/* |x| */
> +
> +  /* x is nan */
> +  if (((ix >= 0x7fff000000000000LL)
> +       && ((ix - 0x7fff000000000000LL) | lx) != 0))
> +    return x + x;
> +  if ((ix | lx) == 0)
> +    {
> +      /* return +-minsubnormal */
> +      SET_LDOUBLE_WORDS64 (x, 0, 1);
> +      return x;
> +    }
> +  if (hx >= 0)
> +    {				/* x > 0 */
> +      if (isinf (x))
> +	return x;
> +      lx++;
> +      if (lx == 0)
> +	hx++;
> +    }
> +  else
> +    {				/* x < 0 */
> +      if (lx == 0)
> +	hx--;
> +      lx--;
> +    }
> +  SET_LDOUBLE_WORDS64 (x, hx, lx);
> +  return x;
> +}
> +
> +weak_alias (__nextupl, nextupl)
> diff --git a/sysdeps/ieee754/ldbl-128ibm/s_nextupl.c b/sysdeps/ieee754/ldbl-128ibm/s_nextupl.c
> new file mode 100644
> index 0000000..29457d5
> --- /dev/null
> +++ b/sysdeps/ieee754/ldbl-128ibm/s_nextupl.c
> @@ -0,0 +1,83 @@
> +/* Return the next representable value greater than x.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +#include <math.h>
> +#include <math_private.h>
> +#include <math_ldbl_opt.h>
> +
> +long double
> +__nextupl (long double x)
> +{
> +  int64_t hx, ihx, lx;
> +  double xhi, xlo, yhi;
> +
> +  ldbl_unpack (x, &xhi, &xlo);
> +  EXTRACT_WORDS64 (hx, xhi);
> +  EXTRACT_WORDS64 (lx, xlo);
> +  ihx = hx & 0x7fffffffffffffffLL;	/* |hx| */
> +
> +  if (ihx > 0x7ff0000000000000LL)	/* x is nan */
> +    return x + x;		/* signal the nan */
> +  if (ihx == 0)
> +    {				/* x == 0 */
> +      hx =  1;
> +      INSERT_WORDS64 (xhi, hx);
> +      x = xhi;
> +      return x;
> +    }
> +
> +  long double u;
> +  if ((hx == 0x7fefffffffffffffLL) && (lx == 0x7c8ffffffffffffeLL))
> +    {
> +      return INFINITY;
> +    }

No braces.

> +  if ((uint64_t) hx >= 0xfff0000000000000ULL)
> +    {
> +      u = -0x1.fffffffffffff7ffffffffffff8p+1023L;
> +      return u;
> +    }
> +  if (ihx <= 0x0360000000000000LL)
> +    {				/* x <= LDBL_MIN */
> +      x += LDBL_TRUE_MIN;
> +      if (x == 0.0L)		/* handle negative LDBL_TRUE_MIN case */
> +	x = -0.0L;
> +      return x;
> +    }
> +  /* If the high double is an exact power of two and the low
> +     double is the opposite sign, then 1ulp is one less than
> +     what we might determine from the high double.  Similarly
> +     if X is an exact power of two, and negative, because
> +     making it a little larger will result in the exponent
> +     decreasing by one and normalisation of the mantissa.   */
> +  if ((hx & 0x000fffffffffffffLL) == 0
> +      && ((lx != 0 && lx != 0x8000000000000000LL && (hx ^ lx) < 0)
> +	  || ((lx == 0 || lx == 0x8000000000000000LL) && hx < 0)))

Tab here too.

> +    ihx -= 1LL << 52;
> +  if (ihx < (106LL << 52))
> +    {				/* ulp will denormal */
> +      INSERT_WORDS64 (yhi, ihx & (0x7ffLL << 52));
> +      u = yhi * 0x1p-105;
> +    }
> +  else
> +    {
> +      INSERT_WORDS64 (yhi, (ihx & (0x7ffLL << 52)) - (105LL << 52));
> +      u = yhi;
> +    }
> +  return x + u;
> +}
> +
> +weak_alias (__nextupl, nextupl)
> diff --git a/sysdeps/ieee754/ldbl-96/s_nextupl.c b/sysdeps/ieee754/ldbl-96/s_nextupl.c
> new file mode 100644
> index 0000000..a7a7f96
> --- /dev/null
> +++ b/sysdeps/ieee754/ldbl-96/s_nextupl.c
> @@ -0,0 +1,85 @@
> +/* Return the next representable value greater than x.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +#include <math.h>
> +#include <math_private.h>
> +
> +long double
> +__nextupl (long double x)
> +{
> +  u_int32_t hx, ix;
> +  u_int32_t lx;
> +  int32_t esx;
> +
> +  GET_LDOUBLE_WORDS (esx, hx, lx, x);
> +  ix = esx & 0x7fff;		/* |x| */
> +
> +  if (((ix == 0x7fff) && (((hx & 0x7fffffff) | lx) != 0)))	/* x is nan */
> +    return x + x;
> +  if ((ix | hx | lx) == 0)
> +    {				/* x == 0 */
> +      SET_LDOUBLE_WORDS (x, 0, 0, 1);	/* return +-minsubnormal */
> +      return x;
> +    }
> +  if (esx >= 0)
> +    {				/* x > 0 */
> +      if (isinf (x))
> +	return x;
> +      lx += 1;
> +      if (lx == 0)
> +	{
> +	  hx += 1;
> +#if LDBL_MIN_EXP == -16381
> +	  if (hx == 0 || (esx == 0 && hx == 0x80000000))
> +#else
> +	  if (hx == 0)
> +#endif
> +	    {
> +	      esx += 1;
> +	      hx |= 0x80000000;
> +	    }
> +	}
> +    }
> +  else
> +    {				/* x < 0 */
> +      if (lx == 0)
> +	{
> +#if LDBL_MIN_EXP == -16381
> +	  if (hx <= 0x80000000 && esx != 0xffff8000)
> +	    {
> +	      esx -= 1;
> +	      hx = hx - 1;
> +	      if ((esx & 0x7fff) > 0)
> +		hx |= 0x80000000;
> +	    }
> +	  else
> +	    hx -= 1;
> +#else
> +	  if (ix != 0 && hx == 0x80000000)
> +	    hx = 0;
> +	  if (hx == 0)
> +	    esx -= 1;
> +	  hx -= 1;
> +#endif
> +	}
> +      lx -= 1;
> +    }
> +  SET_LDOUBLE_WORDS (x, esx, hx, lx);
> +  return x;
> +}
> +
> +weak_alias (__nextupl, nextupl)
> diff --git a/sysdeps/ieee754/ldbl-opt/Makefile b/sysdeps/ieee754/ldbl-opt/Makefile
> index 53091e4..af08209 100644
> --- a/sysdeps/ieee754/ldbl-opt/Makefile
> +++ b/sysdeps/ieee754/ldbl-opt/Makefile
> @@ -40,7 +40,8 @@ libnldbl-calls = asprintf dprintf fprintf fscanf fwprintf fwscanf iovfscanf \
>  		 isoc99_scanf isoc99_fscanf isoc99_sscanf \
>  		 isoc99_vscanf isoc99_vfscanf isoc99_vsscanf \
>  		 isoc99_wscanf isoc99_fwscanf isoc99_swscanf \
> -		 isoc99_vwscanf isoc99_vfwscanf isoc99_vswscanf
> +		 isoc99_vwscanf isoc99_vfwscanf isoc99_vswscanf \
> +		 nextup nextdown
>  libnldbl-routines = $(libnldbl-calls:%=nldbl-%)
>  libnldbl-inhibit-o = $(object-suffixes)
>  libnldbl-static-only-routines = $(libnldbl-routines)
> @@ -120,8 +121,10 @@ CFLAGS-nldbl-modf.c = -fno-builtin-modfl
>  CFLAGS-nldbl-nan.c = -fno-builtin-nanl
>  CFLAGS-nldbl-nearbyint.c = -fno-builtin-nearbyintl
>  CFLAGS-nldbl-nextafter.c = -fno-builtin-nextafterl
> +CFLAGS-nldbl-nextdown.c = -fno-builtin-nextdownl
>  CFLAGS-nldbl-nexttoward.c = -fno-builtin-nexttoward -fno-builtin-nexttowardl
>  CFLAGS-nldbl-nexttowardf.c = -fno-builtin-nexttowardf
> +CFLAGS-nldbl-nextup.c = -fno-builtin-nextupl
>  CFLAGS-nldbl-pow.c = -fno-builtin-powl
>  CFLAGS-nldbl-pow10.c = -fno-builtin-pow10l
>  CFLAGS-nldbl-remainder.c = -fno-builtin-remainderl -fno-builtin-dreml
> diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-nextdown.c b/sysdeps/ieee754/ldbl-opt/nldbl-nextdown.c
> new file mode 100644
> index 0000000..5d81395
> --- /dev/null
> +++ b/sysdeps/ieee754/ldbl-opt/nldbl-nextdown.c
> @@ -0,0 +1,23 @@
> +/* Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +#include "nldbl-compat.h"
> +
> +double attribute_hidden
> +nextdownl (double x)
> +{
> +  return nextdown (x);
> +}
> diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-nextup.c b/sysdeps/ieee754/ldbl-opt/nldbl-nextup.c
> new file mode 100644
> index 0000000..ad66d05
> --- /dev/null
> +++ b/sysdeps/ieee754/ldbl-opt/nldbl-nextup.c
> @@ -0,0 +1,23 @@
> +/* Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +#include "nldbl-compat.h"
> +
> +double attribute_hidden
> +nextupl (double x)
> +{
> +  return nextup (x);
> +}

Rical


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]