[PATCH] powerpc: Fix TLE build for SPE (BZ #22926)

[PATCH] powerpc: Fix TLE build for SPE (BZ #22926)

[Adhemerval Zanella]

Some SPE opcodes clashes with some recent PowerISA opcodes and
until recently gas did not complain about it.  However binutils
recently changed it and now VLE configured gas does not support to
assembler some instruction that might class with VLE (HTM for
instance).  It also does not help that glibc build hardware lock
elision support as default (regardless of assembler support).

Although runtime will not actually enables TLE on SPE hardware
(since kernel will not advertise it), I see little advantage on
adding HTM support on SPE built glibc.  SPE uses an incompatible
ABI which does not allow share the same build with default
powerpc and HTM code slows down SPE without any benefict.

This patch fixes it by only building HTM when SPE configuration
is not used.

Checked with a powerpc-linux-gnuspe build. I also did some sniff
tests on a e500 hardware without any issue.

	[BZ #22926]
	* sysdeps/powerpc/powerpc32/sysdep.h (ABORT_TRANSACTION_IMPL): Define
	empty for __SPE__.
	* sysdeps/powerpc/sysdep.h (ABORT_TRANSACTION): Likewise.
	* sysdeps/unix/sysv/linux/powerpc/elision-lock.c (__lll_lock_elision):
	Do not build hardware transactional code for __SPE__.
	* sysdeps/unix/sysv/linux/powerpc/elision-trylock.c
	(__lll_trylock_elision): Likewise.
	* sysdeps/unix/sysv/linux/powerpc/elision-unlock.c
	(__lll_unlock_elision): Likewise.
---
 ChangeLog                                         | 13 +++++++++++++
 sysdeps/powerpc/powerpc32/sysdep.h                |  2 +-
 sysdeps/powerpc/sysdep.h                          |  2 +-
 sysdeps/unix/sysv/linux/powerpc/elision-lock.c    |  2 ++
 sysdeps/unix/sysv/linux/powerpc/elision-trylock.c |  2 ++
 sysdeps/unix/sysv/linux/powerpc/elision-unlock.c  |  4 ++++
 6 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/sysdeps/powerpc/powerpc32/sysdep.h b/sysdeps/powerpc/powerpc32/sysdep.h
index 8e32a2a..5f1294e 100644
--- a/sysdeps/powerpc/powerpc32/sysdep.h
+++ b/sysdeps/powerpc/powerpc32/sysdep.h
@@ -90,7 +90,7 @@ GOT_LABEL:			;					      \
   cfi_endproc;								      \
   ASM_SIZE_DIRECTIVE(name)
 
-#if ! IS_IN(rtld)
+#if !IS_IN(rtld) && !defined(__SPE__)
 # define ABORT_TRANSACTION_IMPL \
     cmpwi    2,0;		\
     beq      1f;		\
diff --git a/sysdeps/powerpc/sysdep.h b/sysdeps/powerpc/sysdep.h
index 03db75f..8a6d236 100644
--- a/sysdeps/powerpc/sysdep.h
+++ b/sysdeps/powerpc/sysdep.h
@@ -174,7 +174,7 @@
    we abort transaction just before syscalls.
 
    [1] Documentation/powerpc/transactional_memory.txt [Syscalls]  */
-#if !IS_IN(rtld)
+#if !IS_IN(rtld) && !defined(__SPE__)
 # define ABORT_TRANSACTION \
   ({ 						\
     if (THREAD_GET_TM_CAPABLE ())		\
diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-lock.c b/sysdeps/unix/sysv/linux/powerpc/elision-lock.c
index b7093fe..98a23f0 100644
--- a/sysdeps/unix/sysv/linux/powerpc/elision-lock.c
+++ b/sysdeps/unix/sysv/linux/powerpc/elision-lock.c
@@ -45,6 +45,7 @@
 int
 __lll_lock_elision (int *lock, short *adapt_count, EXTRAARG int pshared)
 {
+#ifndef __SPE__
   /* adapt_count is accessed concurrently but is just a hint.  Thus,
      use atomic accesses but relaxed MO is sufficient.  */
   if (atomic_load_relaxed (adapt_count) > 0)
@@ -82,5 +83,6 @@ __lll_lock_elision (int *lock, short *adapt_count, EXTRAARG int pshared)
 			  aconf.skip_lock_out_of_tbegin_retries);
 
 use_lock:
+#endif
   return LLL_LOCK ((*lock), pshared);
 }
diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c b/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c
index b74a810..fabb03b 100644
--- a/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c
+++ b/sysdeps/unix/sysv/linux/powerpc/elision-trylock.c
@@ -30,6 +30,7 @@
 int
 __lll_trylock_elision (int *futex, short *adapt_count)
 {
+#ifndef __SPE__
   /* Implement POSIX semantics by forbiding nesting elided trylocks.  */
   __libc_tabort (_ABORT_NESTED_TRYLOCK);
 
@@ -65,5 +66,6 @@ __lll_trylock_elision (int *futex, short *adapt_count)
     }
 
 use_lock:
+#endif
   return lll_trylock (*futex);
 }
diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-unlock.c b/sysdeps/unix/sysv/linux/powerpc/elision-unlock.c
index dcfab19..14e0680 100644
--- a/sysdeps/unix/sysv/linux/powerpc/elision-unlock.c
+++ b/sysdeps/unix/sysv/linux/powerpc/elision-unlock.c
@@ -23,6 +23,7 @@
 int
 __lll_unlock_elision (int *lock, short *adapt_count, int pshared)
 {
+#ifndef __SPE__
   /* When the lock was free we're in a transaction.  */
   if (*lock == 0)
     __libc_tend (0);
@@ -39,5 +40,8 @@ __lll_unlock_elision (int *lock, short *adapt_count, int pshared)
 
       lll_unlock ((*lock), pshared);
     }
+#else
+  lll_unlock ((*lock), pshared);
+#endif
   return 0;
 }
-- 
2.7.4

Re: [PATCH] powerpc: Fix TLE build for SPE (BZ #22926)

[Adhemerval Zanella]

On 05/03/2018 15:17, Adhemerval Zanella wrote:
> Some SPE opcodes clashes with some recent PowerISA opcodes and
> until recently gas did not complain about it.  However binutils
> recently changed it and now VLE configured gas does not support to
> assembler some instruction that might class with VLE (HTM for
> instance).  It also does not help that glibc build hardware lock
> elision support as default (regardless of assembler support).
> 
> Although runtime will not actually enables TLE on SPE hardware
> (since kernel will not advertise it), I see little advantage on
> adding HTM support on SPE built glibc.  SPE uses an incompatible
> ABI which does not allow share the same build with default
> powerpc and HTM code slows down SPE without any benefict.
> 
> This patch fixes it by only building HTM when SPE configuration
> is not used.
> 
> Checked with a powerpc-linux-gnuspe build. I also did some sniff
> tests on a e500 hardware without any issue.

I also saw no regression compared to PowerPC (32-bit soft-float) report
for 2.27 [1] on a e500 hardware.

[1] https://sourceware.org/glibc/wiki/Release/2.27#PowerPC_.2832-bit_soft-float.29

Re: [PATCH] powerpc: Fix TLE build for SPE (BZ #22926)

[Adhemerval Zanella]

On 06/03/2018 15:37, Adhemerval Zanella wrote:
> 
> 
> On 05/03/2018 15:17, Adhemerval Zanella wrote:
>> Some SPE opcodes clashes with some recent PowerISA opcodes and
>> until recently gas did not complain about it.  However binutils
>> recently changed it and now VLE configured gas does not support to
>> assembler some instruction that might class with VLE (HTM for
>> instance).  It also does not help that glibc build hardware lock
>> elision support as default (regardless of assembler support).
>>
>> Although runtime will not actually enables TLE on SPE hardware
>> (since kernel will not advertise it), I see little advantage on
>> adding HTM support on SPE built glibc.  SPE uses an incompatible
>> ABI which does not allow share the same build with default
>> powerpc and HTM code slows down SPE without any benefict.
>>
>> This patch fixes it by only building HTM when SPE configuration
>> is not used.
>>
>> Checked with a powerpc-linux-gnuspe build. I also did some sniff
>> tests on a e500 hardware without any issue.
> 
> I also saw no regression compared to PowerPC (32-bit soft-float) report
> for 2.27 [1] on a e500 hardware.
> 
> [1] https://sourceware.org/glibc/wiki/Release/2.27#PowerPC_.2832-bit_soft-float.29
> 

If no one opposes I will commit it this shortly.

Re: [PATCH] powerpc: Fix TLE build for SPE (BZ #22926)

[James Clarke]

On Wed, Mar 07, 2018 at 01:50:27PM -0300, Adhemerval Zanella wrote:
>
>
> On 06/03/2018 15:37, Adhemerval Zanella wrote:
> >
> >
> > On 05/03/2018 15:17, Adhemerval Zanella wrote:
> >> Some SPE opcodes clashes with some recent PowerISA opcodes and
> >> until recently gas did not complain about it.  However binutils
> >> recently changed it and now VLE configured gas does not support to
> >> assembler some instruction that might class with VLE (HTM for
> >> instance).  It also does not help that glibc build hardware lock
> >> elision support as default (regardless of assembler support).
> >>
> >> Although runtime will not actually enables TLE on SPE hardware
> >> (since kernel will not advertise it), I see little advantage on
> >> adding HTM support on SPE built glibc.  SPE uses an incompatible
> >> ABI which does not allow share the same build with default
> >> powerpc and HTM code slows down SPE without any benefict.
> >>
> >> This patch fixes it by only building HTM when SPE configuration
> >> is not used.
> >>
> >> Checked with a powerpc-linux-gnuspe build. I also did some sniff
> >> tests on a e500 hardware without any issue.
> >
> > I also saw no regression compared to PowerPC (32-bit soft-float) report
> > for 2.27 [1] on a e500 hardware.
> >
> > [1] https://sourceware.org/glibc/wiki/Release/2.27#PowerPC_.2832-bit_soft-float.29
> >
>
> If no one opposes I will commit it this shortly.

Hi Adhemerval,
I just tested this myself (by dropping it into the currently-failing
Debian package) and it built successfully, so please go ahead.

Thanks,
James