Re: [PATCH v4 5/5] math: Remove the error handling wrapper from fmod and fmodf

[Wilco Dijkstra <Wilco.Dijkstra@arm.com>]

Hi Adhemerval,

LGTM - one minor nit about the is_inf functions, are they actually needed?
I'm happy for this to go in either way.

Reviewed-by: Wilco Dijkstra  <Wilco.Dijkstra@arm.com>

Cheers,
Wilco


diff --git a/sysdeps/ieee754/dbl-64/e_fmod.c b/sysdeps/ieee754/dbl-64/e_fmod.c
index d20466fd5d..ff506c40b4 100644
--- a/sysdeps/ieee754/dbl-64/e_fmod.c
+++ b/sysdeps/ieee754/dbl-64/e_fmod.c
@@ -16,7 +16,9 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <libm-alias-double.h>
 #include <libm-alias-finite.h>
+#include <math-svid-compat.h>
 #include <math.h>
 #include "math_config.h"
 
@@ -44,7 +46,7 @@
      }  */
 
 double
-__ieee754_fmod (double x, double y)
+__fmod (double x, double y)
 {
   uint64_t hx = asuint64 (x);
   uint64_t hy = asuint64 (y);
@@ -56,11 +58,16 @@ __ieee754_fmod (double x, double y)
 
   /* Special cases:
      - If x or y is a Nan, NaN is returned.
-     - If x is an inifinity, a NaN is returned.
+     - If x is an inifinity, a NaN is returned and EDOM is set.
      - If y is zero, Nan is returned.
      - If x is +0/-0, and y is not zero, +0/-0 is returned.  */
-  if (__glibc_unlikely (hy == 0        || hx >= EXPONENT_MASK || hy > EXPONENT_MASK))
-    return (x * y) / (x * y);
+  if (__glibc_unlikely (hy == 0
+                       || hx >= EXPONENT_MASK || hy > EXPONENT_MASK))
+    {
+      if (is_nan (hx) || is_nan (hy))
+       return (x * y) / (x * y);
+      return __math_edom ((x * y) / (x * y));
+    }

OK

   if (__glibc_unlikely (hx <= hy))
     {
@@ -142,4 +149,11 @@ __ieee754_fmod (double x, double y)
 
   return make_double (mx, ey, sx);
 }
+strong_alias (__fmod, __ieee754_fmod)
 libm_alias_finite (__ieee754_fmod, __fmod)
+#if LIBM_SVID_COMPAT
+versioned_symbol (libm, __fmod, fmod, GLIBC_2_38);
+libm_alias_double_other (__fmod, fmod)
+#else
+libm_alias_double (__fmod, fmod)
+#endif
diff --git a/sysdeps/ieee754/dbl-64/math_config.h b/sysdeps/ieee754/dbl-64/math_config.h
index 2049cea3f7..5173b99e4a 100644
--- a/sysdeps/ieee754/dbl-64/math_config.h
+++ b/sysdeps/ieee754/dbl-64/math_config.h
@@ -121,6 +121,12 @@ is_nan (uint64_t x)
   return (x & EXP_MANT_MASK) > EXPONENT_MASK;
 }
 
+static inline bool
+is_inf (uint64_t x)
+{
+  return (x & EXP_MANT_MASK) == EXPONENT_MASK;
+}

Is this used?

 static inline uint64_t
 get_mantissa (uint64_t x)
 {
@@ -170,6 +176,9 @@ attribute_hidden double __math_invalid (double);
 
 /* Error handling using output checking, only for errno setting.  */
 
+/* Check if the result generated a demain error.  */
+attribute_hidden double __math_edom (double x);
+
 /* Check if the result overflowed to infinity.  */
 attribute_hidden double __math_check_oflow (double);
 /* Check if the result underflowed to 0.  */
diff --git a/sysdeps/ieee754/dbl-64/math_err.c b/sysdeps/ieee754/dbl-64/math_err.c
index 5f5f965d3e..377a4caad2 100644
--- a/sysdeps/ieee754/dbl-64/math_err.c
+++ b/sysdeps/ieee754/dbl-64/math_err.c
@@ -33,6 +33,12 @@ with_errno (double y, int e)
 #define with_errno(x, e) (x)
 #endif
 
+attribute_hidden double
+__math_edom (double y)
+{
+  return with_errno (y, EDOM);
+}
+
 /* NOINLINE reduces code size.  */
 NOINLINE static double
 xflow (uint32_t sign, double y)
diff --git a/sysdeps/ieee754/dbl-64/w_fmod.c b/sysdeps/ieee754/dbl-64/w_fmod.c
new file mode 100644
index 0000000000..1cc8931700
--- /dev/null
+++ b/sysdeps/ieee754/dbl-64/w_fmod.c
@@ -0,0 +1 @@
+/* Not needed.  */
diff --git a/sysdeps/ieee754/flt-32/e_fmodf.c b/sysdeps/ieee754/flt-32/e_fmodf.c
index 53db32cbe2..c0b5dccc23 100644
--- a/sysdeps/ieee754/flt-32/e_fmodf.c
+++ b/sysdeps/ieee754/flt-32/e_fmodf.c
@@ -17,6 +17,8 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <libm-alias-finite.h>
+#include <libm-alias-float.h>
+#include <math-svid-compat.h>
 #include <math.h>
 #include "math_config.h"
 
@@ -44,7 +46,7 @@
      }  */
 
 float
-__ieee754_fmodf (float x, float y)
+__fmodf (float x, float y)
 {
   uint32_t hx = asuint (x);
   uint32_t hy = asuint (y);
@@ -59,8 +61,13 @@ __ieee754_fmodf (float x, float y)
      - If x is an inifinity, a NaN is returned.
      - If y is zero, Nan is returned.
      - If x is +0/-0, and y is not zero, +0/-0 is returned.  */
-  if (__glibc_unlikely (hy == 0        || hx >= EXPONENT_MASK || hy > EXPONENT_MASK))
-    return (x * y) / (x * y);
+  if (__glibc_unlikely (hy == 0
+                       || hx >= EXPONENT_MASK || hy > EXPONENT_MASK))
+    {
+      if (is_nan (hx) || is_nan (hy))
+       return (x * y) / (x * y);
+      return __math_edomf ((x * y) / (x * y));
+    }

OK

   if (__glibc_unlikely (hx <= hy))
     {
@@ -141,4 +148,11 @@ __ieee754_fmodf (float x, float y)
 
   return make_float (mx, ey, sx);
 }
+strong_alias (__fmodf, __ieee754_fmodf)
+#if LIBM_SVID_COMPAT
+versioned_symbol (libm, __fmodf, fmodf, GLIBC_2_38);
+libm_alias_float_other (__fmod, fmod)
+#else
+libm_alias_float (__fmod, fmod)
+#endif
 libm_alias_finite (__ieee754_fmodf, __fmodf)
diff --git a/sysdeps/ieee754/flt-32/math_config.h b/sysdeps/ieee754/flt-32/math_config.h
index 829430ea28..f97cd39df1 100644
--- a/sysdeps/ieee754/flt-32/math_config.h
+++ b/sysdeps/ieee754/flt-32/math_config.h
@@ -125,6 +125,12 @@ is_nan (uint32_t x)
   return (x & EXP_MANT_MASK) > EXPONENT_MASK;
 }
 
+static inline bool
+is_inf (uint32_t x)
+{
+  return (x & EXP_MANT_MASK) == EXPONENT_MASK;
+}

Used?