powerpc: Revert to default atomic ops in elision code

Message ID 55CD0A41.6000002@linux.vnet.ibm.com
State Committed
Delegated to: Tulio Magno Quites Machado Filho
Headers

Commit Message

Paul E. Murphy Aug. 13, 2015, 9:21 p.m. UTC
  Power ISA 2.07B section B.5.5 relaxed the barrier requirement around a
TLE enabled lock.  It is now identical to a traditional lock.

2015-08-07  Paul E. Murphy  <murphyp@linux.vnet.ibm.com>

	* sysdeps/unix/sysv/linux/powerpc/elision-lock.c
	(__arch_compare_and_exchange_val_32_acq): Remove and use common
	definition.  ISA 2.07B no longer requires full sync.
---
 sysdeps/unix/sysv/linux/powerpc/elision-lock.c |   21 ---------------------
 1 files changed, 0 insertions(+), 21 deletions(-)
  

Comments

Adhemerval Zanella Aug. 13, 2015, 10:05 p.m. UTC | #1
Good to hear it does not need to require different synchronization for TLE
implementation.  Does all ISA 2.07 implementations (currently only POWER8
afaik) implement this?

On 13-08-2015 18:21, Paul E. Murphy wrote:
> Power ISA 2.07B section B.5.5 relaxed the barrier requirement around a
> TLE enabled lock.  It is now identical to a traditional lock.
> 
> 2015-08-07  Paul E. Murphy  <murphyp@linux.vnet.ibm.com>
> 
> 	* sysdeps/unix/sysv/linux/powerpc/elision-lock.c
> 	(__arch_compare_and_exchange_val_32_acq): Remove and use common
> 	definition.  ISA 2.07B no longer requires full sync.
> ---
>  sysdeps/unix/sysv/linux/powerpc/elision-lock.c |   21 ---------------------
>  1 files changed, 0 insertions(+), 21 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-lock.c b/sysdeps/unix/sysv/linux/powerpc/elision-lock.c
> index 7f9bcc2..26d272e 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/elision-lock.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/elision-lock.c
> @@ -23,27 +23,6 @@
>  #include <elision-conf.h>
>  #include "htm.h"
>  
> -/* PowerISA 2.0.7 Section B.5.5 defines isync to be insufficient as a
> -   barrier in acquire mechanism for HTM operations, a strong 'sync' is
> -   required.  */
> -#undef __arch_compare_and_exchange_val_32_acq
> -#define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval)           \
> -  ({                                                                          \
> -      __typeof (*(mem)) __tmp;                                                \
> -      __typeof (mem)  __memp = (mem);                                         \
> -      __asm __volatile (                                                      \
> -                        "1:     lwarx   %0,0,%1" MUTEX_HINT_ACQ "\n"          \
> -                        "       cmpw    %0,%2\n"                              \
> -                        "       bne     2f\n"                                 \
> -                        "       stwcx.  %3,0,%1\n"                            \
> -                        "       bne-    1b\n"                                 \
> -                        "2:     sync"                                         \
> -                        : "=&r" (__tmp)                                       \
> -                        : "b" (__memp), "r" (oldval), "r" (newval)            \
> -                        : "cr0", "memory");                                   \
> -      __tmp;                                                                  \
> -  })
> -
>  #if !defined(LLL_LOCK) && !defined(EXTRAARG)
>  /* Make sure the configuration code is always linked in for static
>     libraries.  */
>
  
Steven Munroe Aug. 13, 2015, 10:18 p.m. UTC | #2
On Thu, 2015-08-13 at 19:05 -0300, Adhemerval Zanella wrote:
> Good to hear it does not need to require different synchronization for TLE
> implementation.  Does all ISA 2.07 implementations (currently only POWER8
> afaik) implement this?
> 
Any production hardware (outside the lab) should conform to 2.07B
  
Gabriel F T Gomes Aug. 14, 2015, 11:43 a.m. UTC | #3
On Thu, Aug 13, 2015 at 04:21:05PM -0500, Paul E. Murphy wrote:
>Power ISA 2.07B section B.5.5 relaxed the barrier requirement around a
>TLE enabled lock.  It is now identical to a traditional lock.

For the record, this is in Book II.
The patch looks good to me.
  
Tulio Magno Quites Machado Filho Aug. 26, 2015, 5:54 p.m. UTC | #4
"Paul E. Murphy" <murphyp@linux.vnet.ibm.com> writes:

> Power ISA 2.07B section B.5.5 relaxed the barrier requirement around a
> TLE enabled lock.  It is now identical to a traditional lock.
>
> 2015-08-07  Paul E. Murphy  <murphyp@linux.vnet.ibm.com>
>
> 	* sysdeps/unix/sysv/linux/powerpc/elision-lock.c
> 	(__arch_compare_and_exchange_val_32_acq): Remove and use common
> 	definition.  ISA 2.07B no longer requires full sync.

LGTM.

I'm pushing it.

Thanks!
  

Patch

diff --git a/sysdeps/unix/sysv/linux/powerpc/elision-lock.c b/sysdeps/unix/sysv/linux/powerpc/elision-lock.c
index 7f9bcc2..26d272e 100644
--- a/sysdeps/unix/sysv/linux/powerpc/elision-lock.c
+++ b/sysdeps/unix/sysv/linux/powerpc/elision-lock.c
@@ -23,27 +23,6 @@ 
 #include <elision-conf.h>
 #include "htm.h"
 
-/* PowerISA 2.0.7 Section B.5.5 defines isync to be insufficient as a
-   barrier in acquire mechanism for HTM operations, a strong 'sync' is
-   required.  */
-#undef __arch_compare_and_exchange_val_32_acq
-#define __arch_compare_and_exchange_val_32_acq(mem, newval, oldval)           \
-  ({                                                                          \
-      __typeof (*(mem)) __tmp;                                                \
-      __typeof (mem)  __memp = (mem);                                         \
-      __asm __volatile (                                                      \
-                        "1:     lwarx   %0,0,%1" MUTEX_HINT_ACQ "\n"          \
-                        "       cmpw    %0,%2\n"                              \
-                        "       bne     2f\n"                                 \
-                        "       stwcx.  %3,0,%1\n"                            \
-                        "       bne-    1b\n"                                 \
-                        "2:     sync"                                         \
-                        : "=&r" (__tmp)                                       \
-                        : "b" (__memp), "r" (oldval), "r" (newval)            \
-                        : "cr0", "memory");                                   \
-      __tmp;                                                                  \
-  })
-
 #if !defined(LLL_LOCK) && !defined(EXTRAARG)
 /* Make sure the configuration code is always linked in for static
    libraries.  */