[PATCH] powerpc: Use global register variable in <thread_pointer.h>

[Mathieu Desnoyers]

----- On Dec 15, 2021, at 10:49 AM, Florian Weimer fweimer@redhat.com wrote:

> * Mathieu Desnoyers:
> 
>> ----- On Dec 15, 2021, at 10:34 AM, Florian Weimer fweimer@redhat.com wrote:
>>
>>> * Mathieu Desnoyers:
>>> 
>>>> For instance, a library/application already defining its own global variable:
>>>>
>>>> register void *myvar asm ("r2");
>>>>
>>>> would not be able to include a header also declaring a global definition
>>>> for register "r2".
>>> 
>>> This may be a good thing, though, if the uses are actually incompatible.
>>> Even local register variables (correctly used) could clash, see below.
>>
>> Two headers independently trying to read the content of the ABI-defined
>> r2 register appear to be perfectly legitimate use-cases.
> 
> Then why does GCC cause these two declarations (definitions) to clash?

Not sure why, but with:

register void *abc asm ("r2");
register void *def asm ("r2");

building in a ppc32 environment:

In file included from ../include/rseq/rseq-thread-pointer.h:14,
                 from ../include/rseq/rseq.h:55,
                 from basic_percpu_ops_test.c:14:
../include/rseq/rseq-ppc-thread-pointer.h:16:16: warning: register of ‘def’ used for multiple global register variables
   16 | register void *def asm ("r2");
      |                ^~~
../include/rseq/rseq-ppc-thread-pointer.h:15:16: note: conflicts with ‘abc’
   15 | register void *abc asm ("r2");
      |      

gcc 11.2.0 emits a warning. It's not an error, but nevertheless this is
something we'd like to avoid, because it would break builds with -Werror.

> 
>>>> Do you see anything wrong with the following implementation ?
>>>>
>>>> static inline void *rseq_thread_pointer(void)
>>>> {
>>>> #ifdef __powerpc64__
>>>> 	register void *__result asm ("r13");
>>>> #else
>>>> 	register void *__result asm ("r2");
>>>> #endif
>>>> 	asm ("" : "=r" (__result));
>>>> 	return __result;
>>>> }
>>> 
>>> If there is a global register variable in scope for r13/r2, it is
>>> unclear whether the compiler is expected to spill and restore it around
>>> the inline assembly.  It's just a performance issue in this case, I
>>> think.  It would still be annoying if it happened.
>>
>> True. However, if both glibc and rseq agree on using local register
>> variables with the asm output operand rather than the global register
>> variable in public headers, we would not have to worry about this.
> 
> Fair point.  GCC should simply support __builtin_thread_pointer on
> POWER, though. 8-)

That would be even better! :) OK so for now, I'll use the local register
variable + asm trick in librseq public headers.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com