| Message ID | 20211203000103.737833-9-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Improve hypot | expand |
Hi Adhemerval, math/s_fmax_template.c | 27 ++++++++++++++++++++++++ sysdeps/generic/math-use-builtins-fmax.h | 4 ++++ sysdeps/generic/math-use-builtins.h | 1 + 3 files changed, 32 insertions(+) create mode 100644 sysdeps/generic/math-use-builtins-fmax.h Looks good. Wilco
On Thu, 2 Dec 2021, Adhemerval Zanella via Libc-alpha wrote: > +#if __HAVE_FLOAT128 > +# define USE_BUILTIN_F128 , _Float128 : USE_FMAXF128_BUILTIN > +# define BUILTIN_F128 , _Float128 :__builtin_fmaxf128 > +#else > +# define USE_BUILTIN_F128 > +# define BUILTIN_F128 > +#endif > + > +#define USE_BUILTIN(X, Y) \ > + _Generic((X), \ > + float : USE_FMAXF_BUILTIN, \ > + double : USE_FMAX_BUILTIN, \ > + long double : USE_FMAXL_BUILTIN \ > + USE_BUILTIN_F128) > + > +#define BUILTIN(X, Y) \ > + _Generic((X), \ > + float : __builtin_fmaxf, \ > + double : __builtin_fmax, \ > + long double : __builtin_fmaxl \ > + BUILTIN_F128) \ > + (X, Y) You should not hardcode cases for different types in type-generic templates. This would fail if we add support for _Float16 functions in future, for example. Use M_SUF to call built-in functions (possibly via defining M_FMAX in math-type-macros.h, similar to the other M_* macros there. Do something similar for USE_*_BUILTIN (I suggest defining M_USE_BUILTIN in the per-type headers used by math-type-macros.h, so you can then call M_USE_BUILTIN (FMAX). You'll probably need to use #if with the M_USE_BUILTIN call (which should be OK after the above changes), because GCC only added __builtin_fmaxfN functions in commit ee5fd23a481f510528e00f4c988ed0e6a71218c2 (GCC 8 and later) but older GCC versions are supported for building for some architectures with _Float128 support, and if __builtin_fmaxf128 gets referenced inside if (0) with older GCC it would be an undeclared identifier.
On 06/12/2021 18:11, Joseph Myers wrote: > On Thu, 2 Dec 2021, Adhemerval Zanella via Libc-alpha wrote: > >> +#if __HAVE_FLOAT128 >> +# define USE_BUILTIN_F128 , _Float128 : USE_FMAXF128_BUILTIN >> +# define BUILTIN_F128 , _Float128 :__builtin_fmaxf128 >> +#else >> +# define USE_BUILTIN_F128 >> +# define BUILTIN_F128 >> +#endif >> + >> +#define USE_BUILTIN(X, Y) \ >> + _Generic((X), \ >> + float : USE_FMAXF_BUILTIN, \ >> + double : USE_FMAX_BUILTIN, \ >> + long double : USE_FMAXL_BUILTIN \ >> + USE_BUILTIN_F128) >> + >> +#define BUILTIN(X, Y) \ >> + _Generic((X), \ >> + float : __builtin_fmaxf, \ >> + double : __builtin_fmax, \ >> + long double : __builtin_fmaxl \ >> + BUILTIN_F128) \ >> + (X, Y) > > You should not hardcode cases for different types in type-generic > templates. This would fail if we add support for _Float16 functions in > future, for example. > > Use M_SUF to call built-in functions (possibly via defining M_FMAX in > math-type-macros.h, similar to the other M_* macros there. > > Do something similar for USE_*_BUILTIN (I suggest defining M_USE_BUILTIN > in the per-type headers used by math-type-macros.h, so you can then call > M_USE_BUILTIN (FMAX). Indeed it is cleaner, I don't see the need of just M_FMAX. > > You'll probably need to use #if with the M_USE_BUILTIN call (which should > be OK after the above changes), because GCC only added __builtin_fmaxfN > functions in commit ee5fd23a481f510528e00f4c988ed0e6a71218c2 (GCC 8 and > later) but older GCC versions are supported for building for some > architectures with _Float128 support, and if __builtin_fmaxf128 gets > referenced inside if (0) with older GCC it would be an undeclared > identifier. > Right, but currently we use declare_mgen_alias for fmax/fmin, so I am not sure it we really need to take care if __builtin_fmaxfN support. For _Float128 I will need to check if this does incur in any issue, I am currently building some older gcc toolchain to check it.
diff --git a/math/s_fmax_template.c b/math/s_fmax_template.c index d817406f04..4417fdb283 100644 --- a/math/s_fmax_template.c +++ b/math/s_fmax_template.c @@ -17,10 +17,37 @@ <https://www.gnu.org/licenses/>. */ #include <math.h> +#include <math-use-builtins.h> + +#if __HAVE_FLOAT128 +# define USE_BUILTIN_F128 , _Float128 : USE_FMAXF128_BUILTIN +# define BUILTIN_F128 , _Float128 :__builtin_fmaxf128 +#else +# define USE_BUILTIN_F128 +# define BUILTIN_F128 +#endif + +#define USE_BUILTIN(X, Y) \ + _Generic((X), \ + float : USE_FMAXF_BUILTIN, \ + double : USE_FMAX_BUILTIN, \ + long double : USE_FMAXL_BUILTIN \ + USE_BUILTIN_F128) + +#define BUILTIN(X, Y) \ + _Generic((X), \ + float : __builtin_fmaxf, \ + double : __builtin_fmax, \ + long double : __builtin_fmaxl \ + BUILTIN_F128) \ + (X, Y) FLOAT M_DECL_FUNC (__fmax) (FLOAT x, FLOAT y) { + if (USE_BUILTIN (x, y)) + return BUILTIN (x, y); + if (isgreaterequal (x, y)) return x; else if (isless (x, y)) diff --git a/sysdeps/generic/math-use-builtins-fmax.h b/sysdeps/generic/math-use-builtins-fmax.h new file mode 100644 index 0000000000..8fc4efca6a --- /dev/null +++ b/sysdeps/generic/math-use-builtins-fmax.h @@ -0,0 +1,4 @@ +#define USE_FMAX_BUILTIN 0 +#define USE_FMAXF_BUILTIN 0 +#define USE_FMAXL_BUILTIN 0 +#define USE_FMAXF128_BUILTIN 0 diff --git a/sysdeps/generic/math-use-builtins.h b/sysdeps/generic/math-use-builtins.h index 19d2d1cf3c..e07bba242f 100644 --- a/sysdeps/generic/math-use-builtins.h +++ b/sysdeps/generic/math-use-builtins.h @@ -34,5 +34,6 @@ #include <math-use-builtins-copysign.h> #include <math-use-builtins-sqrt.h> #include <math-use-builtins-fma.h> +#include <math-use-builtins-fmax.h> #endif /* MATH_USE_BUILTINS_H */