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


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


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