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

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

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
>