Message ID | 1520273865-15009-1-git-send-email-adhemerval.zanella@linaro.org |
---|---|
State | Accepted |
Commit | e921c89e01389161c036ec09112da6e18aeaa688 |
Headers | show |
Series | powerpc: Fix TLE build for SPE (BZ #22926) | expand |
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
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.
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
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; }