Message ID | xnk1v8jrki.fsf@greed.delorie.com |
---|---|
State | New |
Headers | show |
Series | riscv: fmax/fmin sNaN fix | expand |
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!
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)