riscv: fmax/fmin sNaN fix
Commit Message
RISC-V's FPU follows the IEEE spec, not the POSIX spec. This patch
adds handling for sNaN cases where the two specs differ.
* sysdeps/riscv/rvd/s_fmax.c (__fmax): Handle sNaNs correctly.
* sysdeps/riscv/rvd/s_fmin.c (__fmin): Likewise.
* sysdeps/riscv/rvf/s_fmaxf.c (__fmaxf): Likewise.
* sysdeps/riscv/rvf/s_fminf.c (__fminf): Likewise.
Comments
On 20/02/18 02:57, DJ Delorie wrote:
>
> RISC-V's FPU follows the IEEE spec, not the POSIX spec. This patch
^^^^^^^^^
which one?
(the next ieee revision will have different min/max operations)
On 02/20/2018 02:26 AM, Szabolcs Nagy wrote:
> On 20/02/18 02:57, DJ Delorie wrote:
>>
>> RISC-V's FPU follows the IEEE spec, not the POSIX spec. This patch
> ^^^^^^^^^
> which one?
> (the next ieee revision will have different min/max operations)
The next (201x) one.
r~
On Tue, 20 Feb 2018, Szabolcs Nagy wrote:
> On 20/02/18 02:57, DJ Delorie wrote:
> >
> > RISC-V's FPU follows the IEEE spec, not the POSIX spec. This patch
> ^^^^^^^^^
> which one?
> (the next ieee revision will have different min/max operations)
The point (as per
<https://sourceware.org/ml/libc-alpha/2018-01/msg01084.html>) is that fmax
and fmin in TS 18661-1 bind to maxNum and minNum. Those operations are to
be removed in IEEE 754-2018 so the C functions will have semantics that no
longer have a corresponding IEEE operation (much like e.g. nextafter /
nexttoward, which correspond to the Nextafter operation recommended in
IEEE 754-1985 but removed in IEEE 754-2008). Instead, the new minimum,
minimumNumber, maximum and maximumNumber operations are proposed to have
new functions fminimum, fminimum_num, fmaximum, fmaximum_num (and likewise
*mag* functions) - but so far there is no public draft of those proposed
TS changes (which might in any case more likely be dealt with in the C2x
process rather than through a new revision of the TS being produced).
On Mon, 19 Feb 2018 18:57:49 PST (-0800), dj@redhat.com wrote:
>
> RISC-V's FPU follows the IEEE spec, not the POSIX spec. This patch
> adds handling for sNaN cases where the two specs differ.
>
> * sysdeps/riscv/rvd/s_fmax.c (__fmax): Handle sNaNs correctly.
> * sysdeps/riscv/rvd/s_fmin.c (__fmin): Likewise.
> * sysdeps/riscv/rvf/s_fmaxf.c (__fmaxf): Likewise.
> * sysdeps/riscv/rvf/s_fminf.c (__fminf): Likewise.
>
> diff --git a/sysdeps/riscv/rvd/s_fmax.c b/sysdeps/riscv/rvd/s_fmax.c
> index ef8f1344ce..22e91bfc4b 100644
> --- a/sysdeps/riscv/rvd/s_fmax.c
> +++ b/sysdeps/riscv/rvd/s_fmax.c
> @@ -17,12 +17,19 @@
> <http://www.gnu.org/licenses/>. */
>
> #include <math.h>
> +#include <math_private.h>
> #include <libm-alias-double.h>
>
> double
> __fmax (double x, double y)
> {
> - asm ("fmax.d %0, %1, %2" : "=f" (x) : "f" (x), "f" (y));
> - return x;
> + double res;
> +
> + if (__glibc_unlikely ((_FCLASS (x) | _FCLASS (y)) & _FCLASS_SNAN))
> + return x + y;
> + else
> + asm ("fmax.d %0, %1, %2" : "=f" (res) : "f" (x), "f" (y));
> +
> + return res;
> }
> libm_alias_double (__fmax, fmax)
> diff --git a/sysdeps/riscv/rvd/s_fmin.c b/sysdeps/riscv/rvd/s_fmin.c
> index c6ff24cefb..7b35230cac 100644
> --- a/sysdeps/riscv/rvd/s_fmin.c
> +++ b/sysdeps/riscv/rvd/s_fmin.c
> @@ -17,12 +17,19 @@
> <http://www.gnu.org/licenses/>. */
>
> #include <math.h>
> +#include <math_private.h>
> #include <libm-alias-double.h>
>
> double
> __fmin (double x, double y)
> {
> - asm ("fmin.d %0, %1, %2" : "=f" (x) : "f" (x), "f" (y));
> - return x;
> + double res;
> +
> + if (__glibc_unlikely ((_FCLASS (x) | _FCLASS (y)) & _FCLASS_SNAN))
> + return x + y;
> + else
> + asm ("fmin.d %0, %1, %2" : "=f" (res) : "f" (x), "f" (y));
> +
> + return res;
> }
> libm_alias_double (__fmin, fmin)
> diff --git a/sysdeps/riscv/rvf/s_fmaxf.c b/sysdeps/riscv/rvf/s_fmaxf.c
> index 3293f2f41c..63f7e3d664 100644
> --- a/sysdeps/riscv/rvf/s_fmaxf.c
> +++ b/sysdeps/riscv/rvf/s_fmaxf.c
> @@ -17,12 +17,19 @@
> <http://www.gnu.org/licenses/>. */
>
> #include <math.h>
> +#include <math_private.h>
> #include <libm-alias-float.h>
>
> float
> __fmaxf (float x, float y)
> {
> - asm ("fmax.s %0, %1, %2" : "=f" (x) : "f" (x), "f" (y));
> - return x;
> + float res;
> +
> + if (__glibc_unlikely ((_FCLASS (x) | _FCLASS (y)) & _FCLASS_SNAN))
> + return x + y;
> + else
> + asm ("fmax.s %0, %1, %2" : "=f" (res) : "f" (x), "f" (y));
> +
> + return res;
> }
> libm_alias_float (__fmax, fmax)
> diff --git a/sysdeps/riscv/rvf/s_fminf.c b/sysdeps/riscv/rvf/s_fminf.c
> index e4411f04b2..82cca4e37d 100644
> --- a/sysdeps/riscv/rvf/s_fminf.c
> +++ b/sysdeps/riscv/rvf/s_fminf.c
> @@ -17,12 +17,19 @@
> <http://www.gnu.org/licenses/>. */
>
> #include <math.h>
> +#include <math_private.h>
> #include <libm-alias-float.h>
>
> float
> __fminf (float x, float y)
> {
> - asm ("fmin.s %0, %1, %2" : "=f" (x) : "f" (x), "f" (y));
> - return x;
> + float res;
> +
> + if (__glibc_unlikely ((_FCLASS (x) | _FCLASS (y)) & _FCLASS_SNAN))
> + return x + y;
> + else
> + asm ("fmin.s %0, %1, %2" : "=f" (res) : "f" (x), "f" (y));
> +
> + return res;
> }
> libm_alias_float (__fmin, fmin)
Also looks good, modulo the commit message and ChangeLog. Thanks!
@@ -17,12 +17,19 @@
<http://www.gnu.org/licenses/>. */
#include <math.h>
+#include <math_private.h>
#include <libm-alias-double.h>
double
__fmax (double x, double y)
{
- asm ("fmax.d %0, %1, %2" : "=f" (x) : "f" (x), "f" (y));
- return x;
+ double res;
+
+ if (__glibc_unlikely ((_FCLASS (x) | _FCLASS (y)) & _FCLASS_SNAN))
+ return x + y;
+ else
+ asm ("fmax.d %0, %1, %2" : "=f" (res) : "f" (x), "f" (y));
+
+ return res;
}
libm_alias_double (__fmax, fmax)
@@ -17,12 +17,19 @@
<http://www.gnu.org/licenses/>. */
#include <math.h>
+#include <math_private.h>
#include <libm-alias-double.h>
double
__fmin (double x, double y)
{
- asm ("fmin.d %0, %1, %2" : "=f" (x) : "f" (x), "f" (y));
- return x;
+ double res;
+
+ if (__glibc_unlikely ((_FCLASS (x) | _FCLASS (y)) & _FCLASS_SNAN))
+ return x + y;
+ else
+ asm ("fmin.d %0, %1, %2" : "=f" (res) : "f" (x), "f" (y));
+
+ return res;
}
libm_alias_double (__fmin, fmin)
@@ -17,12 +17,19 @@
<http://www.gnu.org/licenses/>. */
#include <math.h>
+#include <math_private.h>
#include <libm-alias-float.h>
float
__fmaxf (float x, float y)
{
- asm ("fmax.s %0, %1, %2" : "=f" (x) : "f" (x), "f" (y));
- return x;
+ float res;
+
+ if (__glibc_unlikely ((_FCLASS (x) | _FCLASS (y)) & _FCLASS_SNAN))
+ return x + y;
+ else
+ asm ("fmax.s %0, %1, %2" : "=f" (res) : "f" (x), "f" (y));
+
+ return res;
}
libm_alias_float (__fmax, fmax)
@@ -17,12 +17,19 @@
<http://www.gnu.org/licenses/>. */
#include <math.h>
+#include <math_private.h>
#include <libm-alias-float.h>
float
__fminf (float x, float y)
{
- asm ("fmin.s %0, %1, %2" : "=f" (x) : "f" (x), "f" (y));
- return x;
+ float res;
+
+ if (__glibc_unlikely ((_FCLASS (x) | _FCLASS (y)) & _FCLASS_SNAN))
+ return x + y;
+ else
+ asm ("fmin.s %0, %1, %2" : "=f" (res) : "f" (x), "f" (y));
+
+ return res;
}
libm_alias_float (__fmin, fmin)