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: Rical Jasan <ricaljasan at pacific dot net>
- To: Rajalakshmi Srinivasaraghavan <raji at linux dot vnet dot ibm dot com>
- Cc: "libc-alpha at sourceware dot org >> libc-alpha" <libc-alpha at sourceware dot org>, Joseph Myers <joseph at codesourcery dot com>
- Date: Tue, 14 Jun 2016 01:41:33 -0700
- 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 06/13/2016 09:58 PM, Rajalakshmi Srinivasaraghavan wrote:
> On 06/13/2016 04:24 AM, Rical Jasan wrote:
>> A few additional things from my attempt to review the code. Mostly
>> coding style/formatting.
>>
>> Rical
>>
> Addressed comments and formatting changes.
>
> --
> Thanks
> Rajalakshmi S
Brilliant! Only a few things jumped out at me for v5:
> diff --git a/manual/arith.texi b/manual/arith.texi
> index 72682f0..598f5a1 100644
> --- a/manual/arith.texi
> +++ b/manual/arith.texi
> @@ -1702,6 +1702,46 @@ These functions are identical to the corresponding versions of
> double}.
> @end deftypefun
>
> +@comment math.h
> +@comment GNU
> +@deftypefun double nextup (double @var{x})
> +@comment math.h
> +@comment GNU
> +@deftypefunx float nextupf (float @var{x})
> +@comment math.h
> +@comment GNU
> +@deftypefunx {long double} nextupl (long double @var{x})
> +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> +The @code{nextup} function returns the next representable neighbor of @var{x}
> +in the direction of positive infinity. If @var{x} is the smallest negative
> +subnormal number in the type of @var{x} the function returns @code{-0}. If
> +@math{@var{x} = @code{0}} the function returns the smallest positive subnormal
> +number in the type of @var{x}. If @var{x} is @code{NaN}, @code{NaN} is
I wonder how Joseph feels about @code{NaN} vs. NaN, given
https://sourceware.org/ml/libc-alpha/2016-06/msg00208.html, since you're
referring to it as an argument and return value here.
> +returned. If @var{x} is @math{+@infinity{}}, @math{+@infinity{}} is returned.
> +@code{nextup} is based on TS 18661 and currently enabled as a GNU extension.
> +@code{nextup} never raises an exception except for signaling NaNs.
> +@end deftypefun
> +
> +@comment math.h
> +@comment GNU
> +@deftypefun double nextdown (double @var{x})
> +@comment math.h
> +@comment GNU
> +@deftypefunx float nextdownf (float @var{x})
> +@comment math.h
> +@comment GNU
> +@deftypefunx {long double} nextdownl (long double @var{x})
> +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> +The @code{nextdown} function returns the next representable neighbor of @var{x}
> +in the direction of negative infinity. If @var{x} is the smallest positive
> +subnormal number in the type of @var{x} the function returns @code{+0}. If
> +@math{@var{x} = @code{0}} the function returns the smallest negative subnormal
> +number in the type of @var{x}. If @var{x} is @code{NaN}, @code{NaN} is
same
> +returned. If @var{x} is @math{-@infinity{}}, @math{-@infinity{}} is returned.
> +@code{nextdown} is based on TS 18661 and currently enabled as a GNU extension.
> +@code{nextdown} never raises an exception except for signaling NaNs.
> +@end deftypefun
> +
> @cindex NaN
> @comment math.h
> @comment ISO
...
> diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-nextdown.c b/sysdeps/ieee754/ldbl-opt/nldbl-nextdown.c
> new file mode 100644
> index 0000000..846deb2
> --- /dev/null
> +++ b/sysdeps/ieee754/ldbl-opt/nldbl-nextdown.c
> @@ -0,0 +1,24 @@
> +/* Copyright (C) 2016 Free Software Foundation, Inc.
Missing a description on the first line.
> + 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"
> +
Missing a comment/description of the function.
> +double attribute_hidden
I need someone else to weigh in, but I think attribute_hidden should be
on its own line between the return type and function name.
> +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..d358105
> --- /dev/null
> +++ b/sysdeps/ieee754/ldbl-opt/nldbl-nextup.c
> @@ -0,0 +1,24 @@
> +/* Copyright (C) 2016 Free Software Foundation, Inc.
Need one-liner.
> + 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"
> +
Function description.
> +double attribute_hidden
Attributes on separate line?
> +nextupl (double x)
> +{
> + return nextup (x);
> +}
Rical