| Message ID | 20240327164527.3717523-12-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix some libm static issues | expand |
On Wed, 27 Mar 2024, Adhemerval Zanella wrote: > The template is used by some ABsI for static build, and it fails set > the expected floating exceptions if the argument is outside of the > range (on x86_64 this triggers an overflow calculation in > __ieee754_acos). Patches 11 through 15 all seem incorrect; it's the responsibility of the __ieee754_* functions to raise the correct exceptions, not of the wrappers. Please make sure you don't have a compiler with a bug like https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95115 miscompiling those __ieee754_* functions.
On 27/03/24 14:14, Joseph Myers wrote: > On Wed, 27 Mar 2024, Adhemerval Zanella wrote: > >> The template is used by some ABsI for static build, and it fails set >> the expected floating exceptions if the argument is outside of the >> range (on x86_64 this triggers an overflow calculation in >> __ieee754_acos). > > Patches 11 through 15 all seem incorrect; it's the responsibility of the > __ieee754_* functions to raise the correct exceptions, not of the > wrappers. Please make sure you don't have a compiler with a bug like > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95115 miscompiling those > __ieee754_* functions. > The failures are both: math/test-float64x-acos-static math/test-ldouble-acos-static $ cat math/test-ldouble-acos-static.out testing long double (without inline functions) Failure: acos (max_value): Exception "Overflow" set Failure: acos (-max_value): Exception "Overflow" set Failure: acos_downward (max_value): Exception "Overflow" set Failure: acos_downward (-max_value): Exception "Overflow" set Failure: acos_towardzero (max_value): Exception "Overflow" set Failure: acos_towardzero (-max_value): Exception "Overflow" set Failure: acos_upward (max_value): Exception "Overflow" set Failure: acos_upward (-max_value): Exception "Overflow" set Test suite completed: 452 test cases plus 448 tests for exception flags and 448 tests for errno executed. 8 errors occurred. And I think it is unrelated to gcc PR95115 because x86_64/i686 will use and specific sysdeps/i386/fpu/e_acosl.c that explicit does not handle this input case for overflow exceptions. For shared build this case is handle by w_acosl_compat.c: if (__builtin_expect (isgreater (fabsl (x), 1.0L), 0) && _LIB_VERSION != _IEEE_) { /* acos(|x|>1) */ feraiseexcept (FE_INVALID); return __kernel_standard_l (x, x, 201); } And that's why I though following the same logic on template would be better. But I think maybe we should fix on x86_64 implementation instead.
On Wed, 27 Mar 2024, Adhemerval Zanella Netto wrote: > And I think it is unrelated to gcc PR95115 because x86_64/i686 will use > and specific sysdeps/i386/fpu/e_acosl.c that explicit does not handle this > input case for overflow exceptions. For shared build this case is > handle by w_acosl_compat.c: > > if (__builtin_expect (isgreater (fabsl (x), 1.0L), 0) > && _LIB_VERSION != _IEEE_) > { > /* acos(|x|>1) */ > feraiseexcept (FE_INVALID); > return __kernel_standard_l (x, x, 201); > } The compat code is dealing with the possibility of SVID exceptions, which isn't relevant here. > And that's why I though following the same logic on template would be > better. But I think maybe we should fix on x86_64 implementation instead. Yes, we should fix the x86_64 implementation. Such issues in dbl-64, flt-32 or ldbl-128 sources would largely have been fixed (modulo compiler bugs) when we started adding new architectures after new architectures stopped using the compat wrappers - but any issues for ldbl-128ibm, ldbl-96 or architecture-specific sources wouldn't have been detected then.
On 27/03/24 16:04, Joseph Myers wrote: > On Wed, 27 Mar 2024, Adhemerval Zanella Netto wrote: > >> And I think it is unrelated to gcc PR95115 because x86_64/i686 will use >> and specific sysdeps/i386/fpu/e_acosl.c that explicit does not handle this >> input case for overflow exceptions. For shared build this case is >> handle by w_acosl_compat.c: >> >> if (__builtin_expect (isgreater (fabsl (x), 1.0L), 0) >> && _LIB_VERSION != _IEEE_) >> { >> /* acos(|x|>1) */ >> feraiseexcept (FE_INVALID); >> return __kernel_standard_l (x, x, 201); >> } > > The compat code is dealing with the possibility of SVID exceptions, which > isn't relevant here. I meant that __ieee754_acosl in being called in both code paths, but the compat code returns early without calling __ieee754_acosl for the possible problematic code path. > >> And that's why I though following the same logic on template would be >> better. But I think maybe we should fix on x86_64 implementation instead. > > Yes, we should fix the x86_64 implementation. > > Such issues in dbl-64, flt-32 or ldbl-128 sources would largely have been > fixed (modulo compiler bugs) when we started adding new architectures > after new architectures stopped using the compat wrappers - but any issues > for ldbl-128ibm, ldbl-96 or architecture-specific sources wouldn't have > been detected then. > I will drop this change and rather focus on the original issues on missing symbols. I will open some bug against the x86 implementation so we keep track of these issues with static linkage.
diff --git a/math/Makefile b/math/Makefile index af1909d0c7..2b2683a9fa 100644 --- a/math/Makefile +++ b/math/Makefile @@ -368,6 +368,7 @@ $(libm-test-c-narrow-obj): $(objpfx)libm-test%.c: libm-test%.inc \ libm-test-funcs-auto-static = \ + acos \ exp10 \ # libm-test-funcs-auto-static libm-test-funcs-noauto-static = \ diff --git a/math/w_acos_template.c b/math/w_acos_template.c index fe6d6c01db..f4b0e91ae1 100644 --- a/math/w_acos_template.c +++ b/math/w_acos_template.c @@ -30,8 +30,13 @@ FLOAT M_DECL_FUNC (__acos) (FLOAT x) { if (__glibc_unlikely (isgreater (M_FABS (x), M_LIT (1.0)))) - /* Domain error: acos(|x|>1). */ - __set_errno (EDOM); + { + /* Domain error: acos(|x|>1). */ + __feraiseexcept (FE_INVALID); + __set_errno (EDOM); + return __builtin_nanl (""); + } + return M_SUF (__ieee754_acos) (x); } declare_mgen_alias (__acos, acos)