This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [PATCH v3 2/9] Use gcc attribute ifunc in libc_ifunc macro instead of inline assembly due to false debuginfo.

From: "Paul E. Murphy" <murphyp at linux dot vnet dot ibm dot com>
To: Stefan Liebler <stli at linux dot vnet dot ibm dot com>, libc-alpha at sourceware dot org
Cc: fweimer at redhat dot com, schwab at linux-m68k dot org, joseph_myers at mentor dot com
Date: Mon, 29 Aug 2016 15:16:07 -0500
Subject: Re: [PATCH v3 2/9] Use gcc attribute ifunc in libc_ifunc macro instead of inline assembly due to false debuginfo.
Authentication-results: sourceware.org; auth=none
References: <1472047472-30307-1-git-send-email-stli@linux.vnet.ibm.com> <1472047472-30307-2-git-send-email-stli@linux.vnet.ibm.com>


On 08/24/2016 09:04 AM, Stefan Liebler wrote:

> 
> This way there is no trouble with the internal __GI_* symbols.
> Glibc builds fine with this construct and the debuginfo is "correct".
> For functions without a __GI_* symbol like memccpy this redirection is not needed.

Should memccpy read mempcpy?  Also, is the reason why you are able to remove the
libc_hidden_def usage later on in this patch?

> --- a/sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_finite.c
> +++ b/sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_finite.c
> @@ -16,6 +16,9 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
> 
> +#define __finite __redirect___finite
> +#define __finitef __redirect___finitef
> +#define __finitel __redirect___finitel
finitef and finitel don't seem to be needed here.

>  #include <math.h>
>  #include <math_ldbl_opt.h>
>  #include <shlib-compat.h>
> @@ -23,11 +26,14 @@
> 
>  extern __typeof (__finite) __finite_ppc32 attribute_hidden;
>  extern __typeof (__finite) __finite_power7 attribute_hidden;
> -
> -libc_ifunc (__finite,
> -	    (hwcap & PPC_FEATURE_ARCH_2_06)
> -	    ? __finite_power7
> -            : __finite_ppc32);
> +#undef __finite
> +#undef __finitef
> +#undef __finitel
Likewise.


> diff --git a/sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_isinf.c b/sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_isinf.c
> index 506c111..fe6c912 100644
> --- a/sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_isinf.c
> +++ b/sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_isinf.c
> @@ -16,6 +16,9 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
> 
> +#define __isinf __redirect___isinf
> +#define __isinff __redirect___isinff
> +#define __isinfl __redirect___isinfl
Similarly, __isinff and __isinfl don't seem to need indirect.

>  #include <math.h>
>  #include <math_ldbl_opt.h>
>  #include <shlib-compat.h>
> @@ -23,11 +26,14 @@
> 
>  extern __typeof (__isinf) __isinf_ppc32 attribute_hidden;
>  extern __typeof (__isinf) __isinf_power7 attribute_hidden;
> -
> -libc_ifunc (__isinf,
> -	    (hwcap & PPC_FEATURE_ARCH_2_06)
> -	    ? __isinf_power7
> -            : __isinf_ppc32);
> +#undef __isinf
> +#undef __isinff
> +#undef __isinfl
Likewise.


> diff --git a/sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_isnan.c b/sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_isnan.c
> index 8f848d7..3655b81 100644
> --- a/sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_isnan.c
> +++ b/sysdeps/powerpc/powerpc32/power4/fpu/multiarch/s_isnan.c
> @@ -16,6 +16,9 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
> 
> +#define __isnan __redirect___isnan
> +#define __isnanf __redirect___isnanf
> +#define __isnanl __redirect___isnanl
__isnanf and __isnanl don't appear to need overrides too. 

>  #include <math.h>
>  #include <math_ldbl_opt.h>
>  #include <shlib-compat.h>
> @@ -25,15 +28,18 @@ extern __typeof (__isnan) __isnan_ppc32 attribute_hidden;
>  extern __typeof (__isnan) __isnan_power5 attribute_hidden;
>  extern __typeof (__isnan) __isnan_power6 attribute_hidden;
>  extern __typeof (__isnan) __isnan_power7 attribute_hidden;
> -
> -libc_ifunc (__isnan,
> -	    (hwcap & PPC_FEATURE_ARCH_2_06)
> -	    ? __isnan_power7 :
> -	      (hwcap & PPC_FEATURE_ARCH_2_05)
> -	      ? __isnan_power6 :
> -		(hwcap & PPC_FEATURE_POWER5)
> -		? __isnan_power5
> -            : __isnan_ppc32);
> +#undef __isnan
> +#undef __isnanf
> +#undef __isnanl
Likewise.

> diff --git a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c
> index 067edc2..c7d67f1 100644
> --- a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c
> +++ b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_finite.c
> @@ -16,6 +16,9 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
> 
> +#define __finite __redirect___finite
> +#define __finitef __redirect___finitef
> +#define __finitel __redirect___finitel
__finitef and __finitel redirection doesn't seem necessary here.

>  #include <math.h>
>  #include <math_ldbl_opt.h>
>  #include <shlib-compat.h>
> @@ -24,13 +27,16 @@
>  extern __typeof (__finite) __finite_ppc64 attribute_hidden;
>  extern __typeof (__finite) __finite_power7 attribute_hidden;
>  extern __typeof (__finite) __finite_power8 attribute_hidden;
> -
> -libc_ifunc (__finite,
> -	    (hwcap2 & PPC_FEATURE2_ARCH_2_07)
> -	    ? __finite_power8 :
> -	      (hwcap & PPC_FEATURE_ARCH_2_06)
> -	      ? __finite_power7
> -            : __finite_ppc64);
> +#undef __finite
> +#undef __finitef
> +#undef __finitel
Likewise.


> diff --git a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isinf.c b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isinf.c
> index 07e159d..a13ec27 100644
> --- a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isinf.c
> +++ b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isinf.c
> @@ -16,6 +16,9 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
> 
> +#define __isinf __redirect___isinf
> +#define __isinff __redirect___isinff
> +#define __isinfl __redirect___isinfl
__isinff __isinfl don't seem to need redirection here.

>  #include <math.h>
>  #include <math_ldbl_opt.h>
>  #include <shlib-compat.h>
> @@ -24,13 +27,16 @@
>  extern __typeof (__isinf) __isinf_ppc64 attribute_hidden;
>  extern __typeof (__isinf) __isinf_power7 attribute_hidden;
>  extern __typeof (__isinf) __isinf_power8 attribute_hidden;
> -
> -libc_ifunc (__isinf,
> -	    (hwcap2 & PPC_FEATURE2_ARCH_2_07)
> -	    ? __isinf_power8 :
> -	      (hwcap & PPC_FEATURE_ARCH_2_06)
> -	      ? __isinf_power7
> -            : __isinf_ppc64);
> +#undef __isinf
> +#undef __isinff
> +#undef __isinfl
Likewise.

> diff --git a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isnan.c b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isnan.c
> index a614f25..fce3c9d 100644
> --- a/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isnan.c
> +++ b/sysdeps/powerpc/powerpc64/fpu/multiarch/s_isnan.c
> @@ -16,6 +16,9 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
> 
> +#define __isnan __redirect___isnan
> +#define __isnanf __redirect___isnanf
> +#define __isnanl __redirect___isnanl
Is __isnanf or __isnanl redirection necessary here?

>  #include <math.h>
>  #include <math_ldbl_opt.h>
>  #include <shlib-compat.h>
> @@ -27,19 +30,22 @@ extern __typeof (__isnan) __isnan_power6 attribute_hidden;
>  extern __typeof (__isnan) __isnan_power6x attribute_hidden;
>  extern __typeof (__isnan) __isnan_power7 attribute_hidden;
>  extern __typeof (__isnan) __isnan_power8 attribute_hidden;
> -
> -libc_ifunc (__isnan,
> -	    (hwcap2 & PPC_FEATURE2_ARCH_2_07)
> -	    ? __isnan_power8 :
> -	      (hwcap & PPC_FEATURE_ARCH_2_06)
> -	      ? __isnan_power7 :
> -		(hwcap & PPC_FEATURE_POWER6_EXT)
> -		? __isnan_power6x :
> -		  (hwcap & PPC_FEATURE_ARCH_2_05)
> -		    ? __isnan_power6 :
> -		    (hwcap & PPC_FEATURE_POWER5)
> -		      ? __isnan_power5
> -            : __isnan_ppc64);
> +#undef __isnan
> +#undef __isnanf
> +#undef __isnanl
Likewise.

> diff --git a/sysdeps/powerpc/powerpc64/multiarch/mempcpy.c b/sysdeps/powerpc/powerpc64/multiarch/mempcpy.c
> index 3c77b5f..36ec954 100644
> --- a/sysdeps/powerpc/powerpc64/multiarch/mempcpy.c
> +++ b/sysdeps/powerpc/powerpc64/multiarch/mempcpy.c

> -libc_hidden_def (mempcpy)
These removals are allowed because of the behavior you've mentioned in changelog?


Otherwise, the ppc bits of this patch are OK with the removal of the extra redirections.

Follow-Ups:
- Re: [PATCH v3 2/9] Use gcc attribute ifunc in libc_ifunc macro instead of inline assembly due to false debuginfo.
  - From: Stefan Liebler

References:
- [PATCH v3 1/9] Add configure check to test if gcc supports attribute ifunc.
  - From: Stefan Liebler
- [PATCH v3 2/9] Use gcc attribute ifunc in libc_ifunc macro instead of inline assembly due to false debuginfo.
  - From: Stefan Liebler

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]