public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] ppc64le: Make shared libm routines work again for static programs
@ 2021-03-31  5:14 Siddhesh Poyarekar
  2021-03-31  8:02 ` Andreas Schwab
  0 siblings, 1 reply; 5+ messages in thread
From: Siddhesh Poyarekar @ 2021-03-31  5:14 UTC (permalink / raw)
  To: libc-alpha

Routines shared between libc and libm were made shared-only for libm
(so that libm.a does not have those functions) with this commit:

commit 4898d9712bbd85e6fb576442f578d6f3c3e35898
Author: Siddhesh Poyarekar <siddhesh@sourceware.org>
Date:   Tue Mar 30 14:58:37 2021 +0530

    Avoid adding duplicated symbols into static libraries

    Some math functions (such as __isnan*) are built into both libm and
    libc because they are needed in libc.  The symbol gets exported from
    libc.so and not libm.so, because of which dynamic linking works fine;
    the symbols are always resolved from libc.so and libm.so uses its
    internal copy of the same function if needed.

    When linking statically though, the libm variants get used throughout
    because the symbols are exported in both archives and libm.a is
    searched first.

    This patch removes these duplicate objects from the libm.a archive so
    that programs always link to libc in both, the static and dynamic
    case.  The difference this will cause is that libm uses of these
    functions will start using the libc versions in the !SHARED case.
    This is harmless at the moment because the objects are identical
    except for their names.

    Some of these duplicates could be removed from libm.so too, but I
    avoided that in the interest of retaining an internal reference if at
    all those functions get used within libm in future.

This broke ppc64le, which implements some of these shared routines as
ifuncs in libm.  To allow the ppc64le case to continue to work, use a
new make variable exclude-libm-shared-only-routines to list the
routines that should be excluded from the shared-only list so that
those routines continue to be built in both libc.a and libm.a on
ppc64le.  I have verified on ppc64le that the libm tests now run to
completion.  I also verified on x86_64 that the shared functions
continue to appear only in libc.a and not in libm.a.

Perhaps a long term fix is for ppc64le to move those ifuncs into libc.
As things stand now, libc use of the shared functions will use the
non-ifunc versions in libc.so, which may not be desirable.

Maybe an even better longer term fi would be to merge libm into libc.
---
 math/Makefile                                       | 6 ++++--
 sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile | 6 +++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/math/Makefile b/math/Makefile
index ceb1eb2085..e37bc594d7 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -194,10 +194,12 @@ calls = s_isinfF s_isnanF s_finiteF s_copysignF s_modfF s_scalbnF s_frexpF \
 	s_signbitF $(gen-calls)
 gen-calls = s_ldexpF
 generated += $(foreach s,.c .S,$(call type-foreach, $(calls:s_%=m_%$(s))))
-routines = $(call type-foreach, $(calls))
 # The $(calls) that are shared between libm and libc are not included in static
 # libm so the symbols end up in exactly one place.
-libm-shared-only-routines = $(call type-foreach, $(calls:s_%=m_%))
+shared-only = $(filter-out \
+		$(exclude-libm-shared-only-routines),$(calls:s_%=m_%))
+libm-shared-only-routines = $(call type-foreach, $(shared-only))
+routines = $(libm-shared-only-routines:m_%=s_%)
 
 ifeq ($(build-mathvec),yes)
 # We need to install libm.so and libm.a as linker scripts
diff --git a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
index 767805b510..3c0603fa02 100644
--- a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
+++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
@@ -85,6 +85,9 @@ endif
 #
 ifeq ($(do_f128_multiarch),yes)
 
+exclude-libm-shared-only-routines = s_modfF s_scalbnF s_frexpF \
+				    s_ldexpF
+
 gen-libm-f128-ifunc-routines = \
 	e_acosf128 e_acoshf128 e_asinf128 e_atan2f128 e_atanhf128 e_coshf128 \
 	e_expf128 e_fmodf128 e_hypotf128 e_j0f128 e_j1f128 e_jnf128 \
@@ -96,7 +99,8 @@ gen-libm-f128-ifunc-routines = \
 	s_tanhf128 s_truncf128 s_remquof128 e_log2f128 \
 	s_roundf128 s_nearbyintf128 s_sincosf128 s_fmaf128 s_lrintf128 \
 	s_llrintf128 s_lroundf128 s_llroundf128 e_exp10f128 \
-	m_modff128 m_scalbnf128 m_frexpf128 m_ldexpf128 x2y2m1f128 \
+	$(patsubst s_%F,m_%f128,$(exclude-libm-shared-only-routines)) \
+	x2y2m1f128 \
 	gamma_productf128 lgamma_negf128 lgamma_productf128 s_roundevenf128 \
 	cargf128 conjf128 cimagf128 crealf128 cabsf128 e_scalbf128 s_cacosf128 \
 	s_cacoshf128 s_ccosf128 s_ccoshf128 s_casinf128 s_csinf128 \
-- 
2.29.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ppc64le: Make shared libm routines work again for static programs
  2021-03-31  5:14 [PATCH] ppc64le: Make shared libm routines work again for static programs Siddhesh Poyarekar
@ 2021-03-31  8:02 ` Andreas Schwab
  2021-03-31  8:17   ` Siddhesh Poyarekar
  0 siblings, 1 reply; 5+ messages in thread
From: Andreas Schwab @ 2021-03-31  8:02 UTC (permalink / raw)
  To: Siddhesh Poyarekar via Libc-alpha; +Cc: Siddhesh Poyarekar

On Mär 31 2021, Siddhesh Poyarekar via Libc-alpha wrote:

> Perhaps a long term fix is for ppc64le to move those ifuncs into libc.

What prevents them to be moved now?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ppc64le: Make shared libm routines work again for static programs
  2021-03-31  8:02 ` Andreas Schwab
@ 2021-03-31  8:17   ` Siddhesh Poyarekar
  2021-03-31 13:47     ` Andreas Schwab
  0 siblings, 1 reply; 5+ messages in thread
From: Siddhesh Poyarekar @ 2021-03-31  8:17 UTC (permalink / raw)
  To: Andreas Schwab, Siddhesh Poyarekar via Libc-alpha

On 3/31/21 1:32 PM, Andreas Schwab wrote:
> On Mär 31 2021, Siddhesh Poyarekar via Libc-alpha wrote:
> 
>> Perhaps a long term fix is for ppc64le to move those ifuncs into libc.
> 
> What prevents them to be moved now?

Time, really; I wrote up this quick fix to unbreak things until I can 
come back with some more time to take a closer look.  If there's 
consensus to not have this intermediate fix in place then I'll just 
revert 4898d9712bbd85e6fb576442f578d6f3c3e35898 and maybe revisit it later.

Siddhesh

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ppc64le: Make shared libm routines work again for static programs
  2021-03-31  8:17   ` Siddhesh Poyarekar
@ 2021-03-31 13:47     ` Andreas Schwab
  2021-03-31 15:54       ` Paul E Murphy
  0 siblings, 1 reply; 5+ messages in thread
From: Andreas Schwab @ 2021-03-31 13:47 UTC (permalink / raw)
  To: Siddhesh Poyarekar; +Cc: Siddhesh Poyarekar via Libc-alpha

This appears to work.

Andreas.

From ea7654a1bf82b4bf3cd2fb6b375a043746427232 Mon Sep 17 00:00:00 2001
From: Andreas Schwab <schwab@linux-m68k.org>
Date: Wed, 31 Mar 2021 14:17:24 +0200
Subject: [PATCH] powerpc64le: Use ifunc for _Float128 functions also in libc

---
 .../powerpc/powerpc64/le/fpu/multiarch/Makefile    | 14 +++++++++++---
 .../powerpc64/le/fpu/multiarch/float128_private.h  |  4 ++--
 .../le/fpu/multiarch/math-type-macros-float128.h   |  4 ++--
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
index 767805b510..fa3425fbc1 100644
--- a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
+++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
@@ -85,6 +85,7 @@ endif
 #
 ifeq ($(do_f128_multiarch),yes)
 
+f128-ifunc-calls = s_modff128 s_scalbnf128 s_frexpf128 s_ldexpf128
 gen-libm-f128-ifunc-routines = \
 	e_acosf128 e_acoshf128 e_asinf128 e_atan2f128 e_atanhf128 e_coshf128 \
 	e_expf128 e_fmodf128 e_hypotf128 e_j0f128 e_j1f128 e_jnf128 \
@@ -96,7 +97,7 @@ gen-libm-f128-ifunc-routines = \
 	s_tanhf128 s_truncf128 s_remquof128 e_log2f128 \
 	s_roundf128 s_nearbyintf128 s_sincosf128 s_fmaf128 s_lrintf128 \
 	s_llrintf128 s_lroundf128 s_llroundf128 e_exp10f128 \
-	m_modff128 m_scalbnf128 m_frexpf128 m_ldexpf128 x2y2m1f128 \
+	$(f128-ifunc-calls) $(f128-ifunc-calls:s_%=m_%) x2y2m1f128 \
 	gamma_productf128 lgamma_negf128 lgamma_productf128 s_roundevenf128 \
 	cargf128 conjf128 cimagf128 crealf128 cabsf128 e_scalbf128 s_cacosf128 \
 	s_cacoshf128 s_ccosf128 s_ccoshf128 s_casinf128 s_csinf128 \
@@ -118,19 +119,26 @@ f128-march-routines-ifunc = $(addsuffix -ifunc,$(gen-libm-f128-ifunc-routines))
 f128-march-routines = $(f128-march-routines-p9) $(f128-march-routines-ifunc)
 f128-march-cpus = power9
 
-libm-routines += $(f128-march-routines)
+f128-march-calls-p9 = $(addsuffix -power9,$(f128-ifunc-calls))
+f128-march-calls-ifunc = $(addsuffix -ifunc,$(f128-ifunc-calls))
+f128-march-calls = $(f128-march-calls-p9) $(f128-march-calls-ifunc)
+
+calls += $(f128-march-calls)
+libm-routines += $(filter-out $(f128-march-calls), $(f128-march-routines))
 generated += $(f128-march-routines)
 
 CFLAGS-float128-ifunc.c += $(type-float128-CFLAGS) $(no-gnu-attribute-CFLAGS)
 
 # Copy special CFLAGS for some functions
+CFLAGS-s_modff128-power9.c += -fsignaling-nans
 CFLAGS-m_modff128-power9.c += -fsignaling-nans
 
 # Generate ifunc wrapper files and target specific wrappers around
 # each routine above.  Note, m_%.c files are fixed up to include
 # s_%.c files.  This is an artifact of the makefile rules which allow
 # some files to be compiled for libc and libm.
-$(objpfx)gen-float128-ifuncs.stmp: Makefile
+$(objpfx)gen-float128-ifuncs.stmp: \
+  Makefile $(..)sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
 	$(make-target-directory)
 	for gcall in $(gen-libm-f128-ifunc-routines); do \
 	  ifile="$${gcall}";                             \
diff --git a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/float128_private.h b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/float128_private.h
index 08530df9a1..c613d4abae 100644
--- a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/float128_private.h
+++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/float128_private.h
@@ -19,7 +19,7 @@
 #ifndef _FLOAT128_PRIVATE_PPC64LE
 #define _FLOAT128_PRIVATE_PPC64LE 1
 
-#if IS_IN(libc) || !defined(_F128_ENABLE_IFUNC)
+#ifndef _F128_ENABLE_IFUNC
 /* multiarch is not supported.  Do nothing and pass through.  */
 #include_next <float128_private.h>
 #else
@@ -94,6 +94,6 @@ F128_REDIR (__lgamma_productf128)
 #include <float128-ifunc-redirects-mp.h>
 #include <float128-ifunc-redirects.h>
 
-#endif /* !(IS_IN(libc) && defined(_F128_ENABLE_IFUNC) */
+#endif /* _F128_ENABLE_IFUNC */
 
 #endif /* _FLOAT128_PRIVATE_PPC64LE */
diff --git a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/math-type-macros-float128.h b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/math-type-macros-float128.h
index 59cfbb87d3..422ce211be 100644
--- a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/math-type-macros-float128.h
+++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/math-type-macros-float128.h
@@ -21,7 +21,7 @@
 
 #include_next <math-type-macros-float128.h>
 
-#if !IS_IN(libc) && defined(_F128_ENABLE_IFUNC)
+#ifdef _F128_ENABLE_IFUNC
 
 /* Include fenv.h now before turning off PLT bypass.  At
    minimum fereaiseexcept is used today.  */
@@ -115,6 +115,6 @@ F128_REDIR (__w_log1pf128);
 /* Include the redirects shared with math_private.h users.  */
 #include <float128-ifunc-redirects.h>
 
-#endif /* !IS_IN(libc) && defined(_F128_ENABLE_IFUNC) */
+#endif /* _F128_ENABLE_IFUNC */
 
 #endif /*_MATH_TYPE_MACROS_FLOAT128_PPC64_MULTI */
-- 
2.31.1

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ppc64le: Make shared libm routines work again for static programs
  2021-03-31 13:47     ` Andreas Schwab
@ 2021-03-31 15:54       ` Paul E Murphy
  0 siblings, 0 replies; 5+ messages in thread
From: Paul E Murphy @ 2021-03-31 15:54 UTC (permalink / raw)
  To: Andreas Schwab, Siddhesh Poyarekar; +Cc: Siddhesh Poyarekar via Libc-alpha



On 3/31/21 8:47 AM, Andreas Schwab wrote:
> This appears to work.
> 
> Andreas.
> 
>  From ea7654a1bf82b4bf3cd2fb6b375a043746427232 Mon Sep 17 00:00:00 2001
> From: Andreas Schwab <schwab@linux-m68k.org>
> Date: Wed, 31 Mar 2021 14:17:24 +0200
> Subject: [PATCH] powerpc64le: Use ifunc for _Float128 functions also in libc
> 
> ---
>   .../powerpc/powerpc64/le/fpu/multiarch/Makefile    | 14 +++++++++++---
>   .../powerpc64/le/fpu/multiarch/float128_private.h  |  4 ++--
>   .../le/fpu/multiarch/math-type-macros-float128.h   |  4 ++--
>   3 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
> index 767805b510..fa3425fbc1 100644
> --- a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
> +++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
> @@ -85,6 +85,7 @@ endif
>   #
>   ifeq ($(do_f128_multiarch),yes)
> 
> +f128-ifunc-calls = s_modff128 s_scalbnf128 s_frexpf128 s_ldexpf128
>   gen-libm-f128-ifunc-routines = \
>   	e_acosf128 e_acoshf128 e_asinf128 e_atan2f128 e_atanhf128 e_coshf128 \
>   	e_expf128 e_fmodf128 e_hypotf128 e_j0f128 e_j1f128 e_jnf128 \
> @@ -96,7 +97,7 @@ gen-libm-f128-ifunc-routines = \
>   	s_tanhf128 s_truncf128 s_remquof128 e_log2f128 \
>   	s_roundf128 s_nearbyintf128 s_sincosf128 s_fmaf128 s_lrintf128 \
>   	s_llrintf128 s_lroundf128 s_llroundf128 e_exp10f128 \
> -	m_modff128 m_scalbnf128 m_frexpf128 m_ldexpf128 x2y2m1f128 \
> +	$(f128-ifunc-calls) $(f128-ifunc-calls:s_%=m_%) x2y2m1f128 \
>   	gamma_productf128 lgamma_negf128 lgamma_productf128 s_roundevenf128 \
>   	cargf128 conjf128 cimagf128 crealf128 cabsf128 e_scalbf128 s_cacosf128 \
>   	s_cacoshf128 s_ccosf128 s_ccoshf128 s_casinf128 s_csinf128 \
> @@ -118,19 +119,26 @@ f128-march-routines-ifunc = $(addsuffix -ifunc,$(gen-libm-f128-ifunc-routines))
>   f128-march-routines = $(f128-march-routines-p9) $(f128-march-routines-ifunc)
>   f128-march-cpus = power9
> 
> -libm-routines += $(f128-march-routines)
> +f128-march-calls-p9 = $(addsuffix -power9,$(f128-ifunc-calls))
> +f128-march-calls-ifunc = $(addsuffix -ifunc,$(f128-ifunc-calls))
> +f128-march-calls = $(f128-march-calls-p9) $(f128-march-calls-ifunc)
> +
> +calls += $(f128-march-calls)
> +libm-routines += $(filter-out $(f128-march-calls), $(f128-march-routines))
>   generated += $(f128-march-routines)
> 
>   CFLAGS-float128-ifunc.c += $(type-float128-CFLAGS) $(no-gnu-attribute-CFLAGS)
> 
>   # Copy special CFLAGS for some functions
> +CFLAGS-s_modff128-power9.c += -fsignaling-nans
>   CFLAGS-m_modff128-power9.c += -fsignaling-nans
> 
>   # Generate ifunc wrapper files and target specific wrappers around
>   # each routine above.  Note, m_%.c files are fixed up to include
>   # s_%.c files.  This is an artifact of the makefile rules which allow
>   # some files to be compiled for libc and libm.
> -$(objpfx)gen-float128-ifuncs.stmp: Makefile
> +$(objpfx)gen-float128-ifuncs.stmp: \
> +  Makefile $(..)sysdeps/powerpc/powerpc64/le/fpu/multiarch/Makefile
>   	$(make-target-directory)
>   	for gcall in $(gen-libm-f128-ifunc-routines); do \
>   	  ifile="$${gcall}";                             \
> diff --git a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/float128_private.h b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/float128_private.h
> index 08530df9a1..c613d4abae 100644
> --- a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/float128_private.h
> +++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/float128_private.h
> @@ -19,7 +19,7 @@
>   #ifndef _FLOAT128_PRIVATE_PPC64LE
>   #define _FLOAT128_PRIVATE_PPC64LE 1
> 
> -#if IS_IN(libc) || !defined(_F128_ENABLE_IFUNC)
> +#ifndef _F128_ENABLE_IFUNC
>   /* multiarch is not supported.  Do nothing and pass through.  */
>   #include_next <float128_private.h>
>   #else
> @@ -94,6 +94,6 @@ F128_REDIR (__lgamma_productf128)
>   #include <float128-ifunc-redirects-mp.h>
>   #include <float128-ifunc-redirects.h>
> 
> -#endif /* !(IS_IN(libc) && defined(_F128_ENABLE_IFUNC) */
> +#endif /* _F128_ENABLE_IFUNC */
> 
>   #endif /* _FLOAT128_PRIVATE_PPC64LE */
> diff --git a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/math-type-macros-float128.h b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/math-type-macros-float128.h
> index 59cfbb87d3..422ce211be 100644
> --- a/sysdeps/powerpc/powerpc64/le/fpu/multiarch/math-type-macros-float128.h
> +++ b/sysdeps/powerpc/powerpc64/le/fpu/multiarch/math-type-macros-float128.h
> @@ -21,7 +21,7 @@
> 
>   #include_next <math-type-macros-float128.h>
> 
> -#if !IS_IN(libc) && defined(_F128_ENABLE_IFUNC)
> +#ifdef _F128_ENABLE_IFUNC
> 
>   /* Include fenv.h now before turning off PLT bypass.  At
>      minimum fereaiseexcept is used today.  */
> @@ -115,6 +115,6 @@ F128_REDIR (__w_log1pf128);
>   /* Include the redirects shared with math_private.h users.  */
>   #include <float128-ifunc-redirects.h>
> 
> -#endif /* !IS_IN(libc) && defined(_F128_ENABLE_IFUNC) */
> +#endif /* _F128_ENABLE_IFUNC */
> 
>   #endif /*_MATH_TYPE_MACROS_FLOAT128_PPC64_MULTI */
> 

Thank you for fixing this.  This looks like the right way to resolve 
this.  LGTM.  Verified with no regressions on a multiarch build on P9.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-31 15:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-31  5:14 [PATCH] ppc64le: Make shared libm routines work again for static programs Siddhesh Poyarekar
2021-03-31  8:02 ` Andreas Schwab
2021-03-31  8:17   ` Siddhesh Poyarekar
2021-03-31 13:47     ` Andreas Schwab
2021-03-31 15:54       ` Paul E Murphy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).