[PATCHv2] powerpc64: Implement TLS using PC-relative addressing

Message ID 20210122012931.1354527-1-tuliom@linux.ibm.com
State Under Review, archived
Delegated to: Adhemerval Zanella Netto
Headers
Series [PATCHv2] powerpc64: Implement TLS using PC-relative addressing |

Commit Message

Tulio Magno Quites Machado Filho Jan. 22, 2021, 1:29 a.m. UTC
  Changes since v1:
 - Dropped the workaround for early GCC 10 versions.  We can't get this
   code to work without having the macro __PCREL__.  The previous code
   was failing if -mcpu=power10 -mno-pcrel was used.
 - Fixed coding style issues (added space after #defined and removed
   unnecessary parenthesis).

---8<---

As the TOC pointer becomes optional on POWER, TLS access has to be
modified in order to use PC-relative addressing instead of depending on
the TOC pointer availability.

This patch also changes old TLS code to use the new syntax.  It's a
purely aesthetical change: the old code still works and the new syntax
helps to move the argument setup instruction away from the call, which
is not used in an inline assembly.

Suggested-by: Alan Modra <amodra@gmail.com>
---
 sysdeps/powerpc/mod-tlsopt-powerpc.c   |  7 ++++
 sysdeps/powerpc/powerpc64/tls-macros.h | 54 ++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 8 deletions(-)
  

Comments

Tulio Magno Quites Machado Filho Jan. 25, 2021, 3:20 p.m. UTC | #1
Tulio Magno Quites Machado Filho via Libc-alpha <libc-alpha@sourceware.org> writes:

> Changes since v1:
>  - Dropped the workaround for early GCC 10 versions.  We can't get this
>    code to work without having the macro __PCREL__.  The previous code
>    was failing if -mcpu=power10 -mno-pcrel was used.
>  - Fixed coding style issues (added space after #defined and removed
>    unnecessary parenthesis).
>
> ---8<---
>
> As the TOC pointer becomes optional on POWER, TLS access has to be
> modified in order to use PC-relative addressing instead of depending on
> the TOC pointer availability.
>
> This patch also changes old TLS code to use the new syntax.  It's a
> purely aesthetical change: the old code still works and the new syntax
> helps to move the argument setup instruction away from the call, which
> is not used in an inline assembly.
>
> Suggested-by: Alan Modra <amodra@gmail.com>

Adhemerval,

I've just realized I forgot to Cc you in this patch.
It's fixing test failures when building glibc using --with-cpu=power10.

May we merge it for 2.33?
  
Adhemerval Zanella Netto Jan. 25, 2021, 5:13 p.m. UTC | #2
On 21/01/2021 22:29, Tulio Magno Quites Machado Filho via Libc-alpha wrote:
> Changes since v1:
>  - Dropped the workaround for early GCC 10 versions.  We can't get this
>    code to work without having the macro __PCREL__.  The previous code
>    was failing if -mcpu=power10 -mno-pcrel was used.
>  - Fixed coding style issues (added space after #defined and removed
>    unnecessary parenthesis).
> 
> ---8<---
> 
> As the TOC pointer becomes optional on POWER, TLS access has to be
> modified in order to use PC-relative addressing instead of depending on
> the TOC pointer availability.
> 
> This patch also changes old TLS code to use the new syntax.  It's a
> purely aesthetical change: the old code still works and the new syntax
> helps to move the argument setup instruction away from the call, which
> is not used in an inline assembly.
> 
> Suggested-by: Alan Modra <amodra@gmail.com>
> ---
>  sysdeps/powerpc/mod-tlsopt-powerpc.c   |  7 ++++
>  sysdeps/powerpc/powerpc64/tls-macros.h | 54 ++++++++++++++++++++++----
>  2 files changed, 53 insertions(+), 8 deletions(-)
> 
> diff --git a/sysdeps/powerpc/mod-tlsopt-powerpc.c b/sysdeps/powerpc/mod-tlsopt-powerpc.c
> index ee0db12a73..ecfa11cb95 100644
> --- a/sysdeps/powerpc/mod-tlsopt-powerpc.c
> +++ b/sysdeps/powerpc/mod-tlsopt-powerpc.c
> @@ -24,7 +24,14 @@ tls_get_addr_opt_test (void)
>    tls_index *tls_arg;
>  #ifdef __powerpc64__
>    register unsigned long thread_pointer __asm__ ("r13");
> +  /* PC-relative addressing is available when compiling with -mpcrel.
> +     The first releases of GCC 10 do support -mpcrel, but do not export
> +     __PCREL__.  */
> +# if defined __PCREL__
> +  asm ("pla %0,foo@got@tlsgd@pcrel" : "=r" (tls_arg));
> +# else
>    asm ("addi %0,2,foo@got@tlsgd" : "=r" (tls_arg));
> +# endif
>  #else
>    register unsigned long thread_pointer __asm__ ("r2");
>    asm ("bcl 20,31,1f\n1:\t"

The 'pla/pld' instruction support was added in binutils 2.33 version
(PowerPC D-form prefixed loads and stores, commit 8acf14351c8),
however the current idea is to set the minimal version for powerpc 
to 2.29 [1].  

The tls-macros.h is used solely for testing and if I understood
correctly -mpcrel should not require extra glibc support (so there
is no compiling reason to increase the current proposal of minimal
binutils to 2.33).

So, what I recommend you to do is:

  1. Change the documentation patch [1] and update the powerpc 
     sysdeps/powerpc/powerpc64/le/configure.ac to check 'as' version
     instead of 'objcopy'.

  2. Add a configure check for the new TLS using PC-relative and
     set a new config.h flag (SUPPORT_PPC_TLS_PCREL).

  3. Use it instead of __PCREL__ on internal testing.

[1] https://sourceware.org/pipermail/libc-alpha/2021-January/121919.html

> diff --git a/sysdeps/powerpc/powerpc64/tls-macros.h b/sysdeps/powerpc/powerpc64/tls-macros.h
> index 79a0b2579c..a0e5c4a832 100644
> --- a/sysdeps/powerpc/powerpc64/tls-macros.h
> +++ b/sysdeps/powerpc/powerpc64/tls-macros.h
> @@ -9,20 +9,56 @@
>  	  : "=b" (__result) );						      \
>       __result;								      \
>    })
> -/* PowerPC64 Initial Exec TLS access.  */
> -#define TLS_IE(x)							      \
> +
> +/* PC-relative addressing is available when compiling with -mpcrel.
> +   The first releases of GCC 10 do support -mpcrel, but do not export
> +   __PCREL__.  */
> +#if defined __PCREL__
> +/* PowerPC64 Initial Exec TLS access, PC-relative addressing.  */
> +# define TLS_IE(x)							      \
>    ({ int * __result;							      \
> -     asm ("ld  %0," #x "@got@tprel(2)\n\t"				      \
> -	  "add %0,%0," #x "@tls"					      \
> +     asm ("pld  %0," #x "@got@tprel@pcrel\n\t"				      \
> +	  "add %0,%0," #x "@tls@pcrel"					      \
>  	  : "=r" (__result) );						      \
>       __result;								      \
>    })
>  
> +/* PowerPC64 Local Dynamic TLS access, PC-relative addressing.  */
> +# define TLS_LD(x)							      \
> +  ({ int * __result;							      \
> +     asm ("pla   3, " #x "@got@tlsld@pcrel\n\t"				      \
> +	  "bl    __tls_get_addr@notoc(" #x "@tlsld)\n\t"		      \
> +	  "addis %0,3," #x "@dtprel@ha\n\t"				      \
> +	  "addi  %0,%0," #x "@dtprel@l"					      \
> +	  : "=b" (__result) :						      \
> +	  : "3", __TLS_CALL_CLOBBERS);					      \
> +     __result;								      \
> +  })
> +
> +/* PowerPC64 General Dynamic TLS access, PC-relative addressing.  */
> +# define TLS_GD(x)							      \
> +  ({ register int *__result __asm__ ("r3");				      \
> +    asm ("pla   3," #x "@got@tlsgd@pcrel\n\t"				      \
> +	 "bl    __tls_get_addr@notoc(" #x "@tlsgd)"			      \
> +	 : "=r" (__result) :						      \
> +	 : __TLS_CALL_CLOBBERS);					      \
> +    __result;								      \
> +  })
> +#else
> +/* PowerPC64 Initial Exec TLS access.  */
> +# define TLS_IE(x)							      \
> +  ({ int * __result;							      \
> +    asm ("ld  %0," #x "@got@tprel(2)\n\t"				      \
> +	 "add %0,%0," #x "@tls"						      \
> +	 : "=r" (__result) );						      \
> +    __result;								      \
> +  })
> +
>  /* PowerPC64 Local Dynamic TLS access.  */
> -#define TLS_LD(x)							      \
> +# define TLS_LD(x)							      \
>    ({ int * __result;							      \
>       asm ("addi  3,2," #x "@got@tlsld\n\t"				      \
> -	  "bl    __tls_get_addr\n\t"					      \
> +	  "bl    __tls_get_addr(" #x "@tlsld)\n\t"			      \
>  	  "nop   \n\t"							      \
>  	  "addis %0,3," #x "@dtprel@ha\n\t"				      \
>  	  "addi  %0,%0," #x "@dtprel@l"					      \
> @@ -30,13 +66,15 @@
>  	  : "3", __TLS_CALL_CLOBBERS);					      \
>       __result;								      \
>    })
> +
>  /* PowerPC64 General Dynamic TLS access.  */
> -#define TLS_GD(x)							      \
> +# define TLS_GD(x)							      \
>    ({ register int *__result __asm__ ("r3");				      \
>       asm ("addi  3,2," #x "@got@tlsgd\n\t"				      \
> -	  "bl    __tls_get_addr\n\t"					      \
> +	  "bl    __tls_get_addr(" #x "@tlsgd)\n\t"			      \
>  	  "nop   "							      \
>  	  : "=r" (__result) :						      \
>  	  : __TLS_CALL_CLOBBERS);					      \
>       __result;								      \
>    })
> +#endif
>
  
Tulio Magno Quites Machado Filho Jan. 25, 2021, 8:22 p.m. UTC | #3
Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> writes:

> On 21/01/2021 22:29, Tulio Magno Quites Machado Filho via Libc-alpha wrote:

> So, what I recommend you to do is:
>
>   1. Change the documentation patch [1] and update the powerpc 
>      sysdeps/powerpc/powerpc64/le/configure.ac to check 'as' version
>      instead of 'objcopy'.

This has already been changed in the following patch:
https://sourceware.org/pipermail/libc-alpha/2021-January/121919.html

It's still missing the configure check, though.  I'm working on that.

>   2. Add a configure check for the new TLS using PC-relative and
>      set a new config.h flag (SUPPORT_PPC_TLS_PCREL).

Ack. I'm working on it.

Thanks!
  
Alan Modra Jan. 27, 2021, 11:16 a.m. UTC | #4
On Mon, Jan 25, 2021 at 05:22:02PM -0300, Tulio Magno Quites Machado Filho wrote:
> Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> writes:
> 
> > On 21/01/2021 22:29, Tulio Magno Quites Machado Filho via Libc-alpha wrote:
> 
> > So, what I recommend you to do is:
> >
> >   1. Change the documentation patch [1] and update the powerpc 
> >      sysdeps/powerpc/powerpc64/le/configure.ac to check 'as' version
> >      instead of 'objcopy'.
> 
> This has already been changed in the following patch:
> https://sourceware.org/pipermail/libc-alpha/2021-January/121919.html
> 
> It's still missing the configure check, though.  I'm working on that.
> 
> >   2. Add a configure check for the new TLS using PC-relative and
> >      set a new config.h flag (SUPPORT_PPC_TLS_PCREL).
> 
> Ack. I'm working on it.

mod-tlsopt-powerpc.c and tls-macros.h are used only by the
testsuite, so don't fuss too much.  Hopefully one day those files will
disappear.
  

Patch

diff --git a/sysdeps/powerpc/mod-tlsopt-powerpc.c b/sysdeps/powerpc/mod-tlsopt-powerpc.c
index ee0db12a73..ecfa11cb95 100644
--- a/sysdeps/powerpc/mod-tlsopt-powerpc.c
+++ b/sysdeps/powerpc/mod-tlsopt-powerpc.c
@@ -24,7 +24,14 @@  tls_get_addr_opt_test (void)
   tls_index *tls_arg;
 #ifdef __powerpc64__
   register unsigned long thread_pointer __asm__ ("r13");
+  /* PC-relative addressing is available when compiling with -mpcrel.
+     The first releases of GCC 10 do support -mpcrel, but do not export
+     __PCREL__.  */
+# if defined __PCREL__
+  asm ("pla %0,foo@got@tlsgd@pcrel" : "=r" (tls_arg));
+# else
   asm ("addi %0,2,foo@got@tlsgd" : "=r" (tls_arg));
+# endif
 #else
   register unsigned long thread_pointer __asm__ ("r2");
   asm ("bcl 20,31,1f\n1:\t"
diff --git a/sysdeps/powerpc/powerpc64/tls-macros.h b/sysdeps/powerpc/powerpc64/tls-macros.h
index 79a0b2579c..a0e5c4a832 100644
--- a/sysdeps/powerpc/powerpc64/tls-macros.h
+++ b/sysdeps/powerpc/powerpc64/tls-macros.h
@@ -9,20 +9,56 @@ 
 	  : "=b" (__result) );						      \
      __result;								      \
   })
-/* PowerPC64 Initial Exec TLS access.  */
-#define TLS_IE(x)							      \
+
+/* PC-relative addressing is available when compiling with -mpcrel.
+   The first releases of GCC 10 do support -mpcrel, but do not export
+   __PCREL__.  */
+#if defined __PCREL__
+/* PowerPC64 Initial Exec TLS access, PC-relative addressing.  */
+# define TLS_IE(x)							      \
   ({ int * __result;							      \
-     asm ("ld  %0," #x "@got@tprel(2)\n\t"				      \
-	  "add %0,%0," #x "@tls"					      \
+     asm ("pld  %0," #x "@got@tprel@pcrel\n\t"				      \
+	  "add %0,%0," #x "@tls@pcrel"					      \
 	  : "=r" (__result) );						      \
      __result;								      \
   })
 
+/* PowerPC64 Local Dynamic TLS access, PC-relative addressing.  */
+# define TLS_LD(x)							      \
+  ({ int * __result;							      \
+     asm ("pla   3, " #x "@got@tlsld@pcrel\n\t"				      \
+	  "bl    __tls_get_addr@notoc(" #x "@tlsld)\n\t"		      \
+	  "addis %0,3," #x "@dtprel@ha\n\t"				      \
+	  "addi  %0,%0," #x "@dtprel@l"					      \
+	  : "=b" (__result) :						      \
+	  : "3", __TLS_CALL_CLOBBERS);					      \
+     __result;								      \
+  })
+
+/* PowerPC64 General Dynamic TLS access, PC-relative addressing.  */
+# define TLS_GD(x)							      \
+  ({ register int *__result __asm__ ("r3");				      \
+    asm ("pla   3," #x "@got@tlsgd@pcrel\n\t"				      \
+	 "bl    __tls_get_addr@notoc(" #x "@tlsgd)"			      \
+	 : "=r" (__result) :						      \
+	 : __TLS_CALL_CLOBBERS);					      \
+    __result;								      \
+  })
+#else
+/* PowerPC64 Initial Exec TLS access.  */
+# define TLS_IE(x)							      \
+  ({ int * __result;							      \
+    asm ("ld  %0," #x "@got@tprel(2)\n\t"				      \
+	 "add %0,%0," #x "@tls"						      \
+	 : "=r" (__result) );						      \
+    __result;								      \
+  })
+
 /* PowerPC64 Local Dynamic TLS access.  */
-#define TLS_LD(x)							      \
+# define TLS_LD(x)							      \
   ({ int * __result;							      \
      asm ("addi  3,2," #x "@got@tlsld\n\t"				      \
-	  "bl    __tls_get_addr\n\t"					      \
+	  "bl    __tls_get_addr(" #x "@tlsld)\n\t"			      \
 	  "nop   \n\t"							      \
 	  "addis %0,3," #x "@dtprel@ha\n\t"				      \
 	  "addi  %0,%0," #x "@dtprel@l"					      \
@@ -30,13 +66,15 @@ 
 	  : "3", __TLS_CALL_CLOBBERS);					      \
      __result;								      \
   })
+
 /* PowerPC64 General Dynamic TLS access.  */
-#define TLS_GD(x)							      \
+# define TLS_GD(x)							      \
   ({ register int *__result __asm__ ("r3");				      \
      asm ("addi  3,2," #x "@got@tlsgd\n\t"				      \
-	  "bl    __tls_get_addr\n\t"					      \
+	  "bl    __tls_get_addr(" #x "@tlsgd)\n\t"			      \
 	  "nop   "							      \
 	  : "=r" (__result) :						      \
 	  : __TLS_CALL_CLOBBERS);					      \
      __result;								      \
   })
+#endif