libstdc++-v3: do not duplicate some math functions when using newlib

libstdc++-v3: do not duplicate some math functions when using newlib

[Alexandre Oliva]

Contributing a patch by Joel Brobecker <brobecker@adacore.com>.
Regstrapped on x86_64-linux-gnu just to be sure, also tested with
aarch64-rtems6.  I'm going to put this in later this week if there
aren't any objections.


When running the libstdc++ testsuite on AArch64 RTEMS, we noticed
that about 25 tests are failing during the link, due to the "sqrtl"
function being defined twice:
  - once inside RTEMS' libm;
  - once inside our libstdc++.

One test that fails, for instance, would be 26_numerics/complex/13450.cc.

In comparing libm and libstdc++, we found that libstc++ also
duplicates "hypotf", and "hypotl".

For "sqrtl" and "hypotl", the symbosl come a unit called
from math_stubs_long_double.cc, while "hypotf" comes from
the equivalent unit for the float version, called math_stubs_float.cc.
Those units are always compiled in libstdc++ and provide our own
version of various math routines when those are missing from
the target system. The definition of those symbols is predicated
on the existance of various macros provided by c++config.h, which
themselves are predicated by the corresponding HAVE_xxx macros
in config.h.

One key element behind what's happening, here, is that the target
uses newlib, and therefore GCC was configured --with-newlib.
The section of libstdc++v3's configure script that handles which math
functions are available has a newlib-specific section, and that
section provides a hardcoded list of symbols.

For "hypotf", this commit fixes the issue by doing the same
as for the other routines already declared in that section.
I verified by inspection in the newlib code that this function
should always be present, so hardcoding it in our configure
script should not be an issue.

For the math routines handling doubles ("sqrtl" and "hypotl"),
however, I do not believe we can assume that newlib's libm
will always provide them. Therefore, this commit fixes that
part of the issue by ading a compile-check for "sqrtl" and "hypotl".
And while at it, we also include checks for all the other math
functions that math_stubs_long_double.cc re-implements, allowing
us to be resilient to future newlib enhancements adding support
for more functions.

libstdc++-v3/ChangeLog:

        * configure.ac ["x${with_newlib}" = "xyes"]: Define
        HAVE_HYPOTF.  Add compile-checks for various long double
        math functions as well.
        * configure: Regenerate.
---
 libstdc++-v3/configure    | 1179 +++++++++++++++++++++++++++++++++++++++++++++
 libstdc++-v3/configure.ac |    9 
 2 files changed, 1188 insertions(+)

diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index 354c566b0055c..bda8053ecc279 100755
[omitted]
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index 0abe54e7b9a21..9770c1787679f 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -349,6 +349,7 @@ else
     AC_DEFINE(HAVE_FLOORF)
     AC_DEFINE(HAVE_FMODF)
     AC_DEFINE(HAVE_FREXPF)
+    AC_DEFINE(HAVE_HYPOTF)
     AC_DEFINE(HAVE_LDEXPF)
     AC_DEFINE(HAVE_LOG10F)
     AC_DEFINE(HAVE_LOGF)
@@ -360,6 +361,14 @@ else
     AC_DEFINE(HAVE_TANF)
     AC_DEFINE(HAVE_TANHF)
 
+dnl # Support for the long version of some math libraries depends on
+dnl # architecture and newlib version.  So test for their availability
+dnl # rather than hardcoding that information.
+    GLIBCXX_CHECK_MATH_DECLS([
+      acosl asinl atan2l atanl ceill coshl cosl expl fabsl floorl fmodl
+      frexpl hypotl ldexpl log10l logl modfl powl sinhl sinl sqrtl
+      tanhl tanl])
+
     AC_DEFINE(HAVE_ICONV)
     AC_DEFINE(HAVE_MEMALIGN)
 

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: libstdc++-v3: do not duplicate some math functions when using newlib

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 4299 bytes --]

On Thu, 15 Jun 2023, 01:46 Alexandre Oliva via Libstdc++, <
libstdc++@gcc.gnu.org> wrote:

>
> Contributing a patch by Joel Brobecker <brobecker@adacore.com>.
> Regstrapped on x86_64-linux-gnu just to be sure, also tested with
> aarch64-rtems6.  I'm going to put this in later this week if there
> aren't any objections.
>
>
> When running the libstdc++ testsuite on AArch64 RTEMS, we noticed
> that about 25 tests are failing during the link, due to the "sqrtl"
> function being defined twice:
>   - once inside RTEMS' libm;
>   - once inside our libstdc++.
>
> One test that fails, for instance, would be 26_numerics/complex/13450.cc.
>
> In comparing libm and libstdc++, we found that libstc++ also
> duplicates "hypotf", and "hypotl".
>
> For "sqrtl" and "hypotl", the symbosl come a unit called
> from math_stubs_long_double.cc, while "hypotf" comes from
> the equivalent unit for the float version, called math_stubs_float.cc.
> Those units are always compiled in libstdc++ and provide our own
> version of various math routines when those are missing from
> the target system. The definition of those symbols is predicated
> on the existance of various macros provided by c++config.h, which
> themselves are predicated by the corresponding HAVE_xxx macros
> in config.h.
>
> One key element behind what's happening, here, is that the target
> uses newlib, and therefore GCC was configured --with-newlib.
> The section of libstdc++v3's configure script that handles which math
> functions are available has a newlib-specific section, and that
> section provides a hardcoded list of symbols.
>
> For "hypotf", this commit fixes the issue by doing the same
> as for the other routines already declared in that section.
> I verified by inspection in the newlib code that this function
> should always be present, so hardcoding it in our configure
> script should not be an issue.
>
> For the math routines handling doubles ("sqrtl" and "hypotl"),
> however, I do not believe we can assume that newlib's libm
> will always provide them. Therefore, this commit fixes that
> part of the issue by ading a compile-check for "sqrtl" and "hypotl".
> And while at it, we also include checks for all the other math
> functions that math_stubs_long_double.cc re-implements, allowing
> us to be resilient to future newlib enhancements adding support
> for more functions.
>

Excellent, I've been looking at this area of our configury and the math
stubs recently and this is a nice improvement.

OK for trunk, thanks.


> libstdc++-v3/ChangeLog:
>
>         * configure.ac ["x${with_newlib}" = "xyes"]: Define
>         HAVE_HYPOTF.  Add compile-checks for various long double
>         math functions as well.
>         * configure: Regenerate.
> ---
>  libstdc++-v3/configure    | 1179
> +++++++++++++++++++++++++++++++++++++++++++++
>  libstdc++-v3/configure.ac |    9
>  2 files changed, 1188 insertions(+)
>
> diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
> index 354c566b0055c..bda8053ecc279 100755
> [omitted]
> diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
> index 0abe54e7b9a21..9770c1787679f 100644
> --- a/libstdc++-v3/configure.ac
> +++ b/libstdc++-v3/configure.ac
> @@ -349,6 +349,7 @@ else
>      AC_DEFINE(HAVE_FLOORF)
>      AC_DEFINE(HAVE_FMODF)
>      AC_DEFINE(HAVE_FREXPF)
> +    AC_DEFINE(HAVE_HYPOTF)
>      AC_DEFINE(HAVE_LDEXPF)
>      AC_DEFINE(HAVE_LOG10F)
>      AC_DEFINE(HAVE_LOGF)
> @@ -360,6 +361,14 @@ else
>      AC_DEFINE(HAVE_TANF)
>      AC_DEFINE(HAVE_TANHF)
>
> +dnl # Support for the long version of some math libraries depends on
> +dnl # architecture and newlib version.  So test for their availability
> +dnl # rather than hardcoding that information.
> +    GLIBCXX_CHECK_MATH_DECLS([
> +      acosl asinl atan2l atanl ceill coshl cosl expl fabsl floorl fmodl
> +      frexpl hypotl ldexpl log10l logl modfl powl sinhl sinl sqrtl
> +      tanhl tanl])
> +
>      AC_DEFINE(HAVE_ICONV)
>      AC_DEFINE(HAVE_MEMALIGN)
>
>
> --
> Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
>    Free Software Activist                       GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about <https://stallmansupport.org>
>

Re: libstdc++-v3: do not duplicate some math functions when using newlib

[Andre Vieira (lists)]

Hey,

I have to admit I am not super familiar with long doubles, either than 
knowing they are 128-bit FP representations... but bisect has pointed me 
to this patch when investigating a regression on aarch64_be-none-elf for 
the libstdc++ testcase: 26_numerics/complex/13450.cc

After some reduction and investigation I found that __complex_pow<long 
double> now returns a different value (which I believe is a NaN) after 
this patch. Some debugging and it seems that the implementation of 
complex_pow itself has not changed, or at least I didn't spot a change 
but the underlying sqrtl is different, assuming one is from libstdc++ 
and the other from newlib? The one used prior to this patch when called 
with input 0x3fff0000...0000 which I believe to be the value 1.0, 
returns the same value. Whereas the new one returns 0x4101000...0000fdfa 
with the same input.

Can you just confirm this is 'wrong' and that the problem is likely to 
be with the big-endian implementation of sqrtl in I'm guessing newlib?


Kind regards,
Andre

Re: libstdc++-v3: do not duplicate some math functions when using newlib

[Alexandre Oliva]

On Oct 25, 2024, "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> wrote:

> I have to admit I am not super familiar with long doubles, either than
> knowing they are 128-bit FP representations... but bisect has pointed
> me to this patch when investigating a regression on
> aarch64_be-none-elf for the libstdc++ testcase:
> 26_numerics/complex/13450.cc

> Can you just confirm this is 'wrong' and that the problem is likely to
> be with the big-endian implementation of sqrtl in I'm guessing newlib?

Yeah, this looks like a bug in newlib.  sqrtl is always defined there,
it manipulates long doubles' internal representation, but unlike double
and float, its ieeefp.h doesn't distinguish between big and little
endian.  That should be easy to fix by defining another version of
struct ieee_ext on big endian systems.

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: libstdc++-v3: do not duplicate some math functions when using newlib

[Andre Vieira (lists)]

On 31/10/2024 08:23, Alexandre Oliva wrote:
> On Oct 25, 2024, "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> wrote:
> 
>> I have to admit I am not super familiar with long doubles, either than
>> knowing they are 128-bit FP representations... but bisect has pointed
>> me to this patch when investigating a regression on
>> aarch64_be-none-elf for the libstdc++ testcase:
>> 26_numerics/complex/13450.cc
> 
>> Can you just confirm this is 'wrong' and that the problem is likely to
>> be with the big-endian implementation of sqrtl in I'm guessing newlib?
> 
> Yeah, this looks like a bug in newlib.  sqrtl is always defined there,
> it manipulates long doubles' internal representation, but unlike double
> and float, its ieeefp.h doesn't distinguish between big and little
> endian.  That should be easy to fix by defining another version of
> struct ieee_ext on big endian systems.
> 

Thanks, reported it in https://sourceware.org/bugzilla/show_bug.cgi?id=32326