diff mbox

[AArch64,1/5] Use atomic instructions for swap and fetch-update operations.

Message ID 55FFE4C0.6010106@foss.arm.com
State New
Headers show

Commit Message

Matthew Wahab Sept. 21, 2015, 11:06 a.m. UTC
On 18/09/15 08:58, James Greenhalgh wrote:
> On Thu, Sep 17, 2015 at 05:37:55PM +0100, Matthew Wahab wrote:

>> diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
>> index 65d2cc9..0e71002 100644
>> --- a/gcc/config/aarch64/atomics.md
>> +++ b/gcc/config/aarch64/atomics.md
>> @@ -27,6 +27,7 @@
>>       UNSPECV_ATOMIC_CMPSW		; Represent an atomic compare swap.
>>       UNSPECV_ATOMIC_EXCHG		; Represent an atomic exchange.
>>       UNSPECV_ATOMIC_CAS			; Represent an atomic CAS.
>> +    UNSPECV_ATOMIC_SWP			; Represent an atomic SWP.
>>       UNSPECV_ATOMIC_OP			; Represent an atomic operation.
>>   ])
>>
>> @@ -122,19 +123,19 @@
>>   )
>>
>>   (define_insn_and_split "aarch64_compare_and_swap<mode>_lse"
>> -  [(set (reg:CC CC_REGNUM)					;; bool out
>> +  [(set (reg:CC CC_REGNUM)
>>       (unspec_volatile:CC [(const_int 0)] UNSPECV_ATOMIC_CMPSW))
>> -   (set (match_operand:GPI 0 "register_operand" "=&r")		;; val out
>> -    (match_operand:GPI 1 "aarch64_sync_memory_operand" "+Q"))   ;; memory
>> +   (set (match_operand:GPI 0 "register_operand" "=&r")
>> +    (match_operand:GPI 1 "aarch64_sync_memory_operand" "+Q"))
>>      (set (match_dup 1)
>>       (unspec_volatile:GPI
>> -      [(match_operand:GPI 2 "aarch64_plus_operand" "rI")	;; expect
>> -       (match_operand:GPI 3 "register_operand" "r")		;; desired
>> -       (match_operand:SI 4 "const_int_operand")			;; is_weak
>> -       (match_operand:SI 5 "const_int_operand")			;; mod_s
>> -       (match_operand:SI 6 "const_int_operand")]		;; mod_f
>> +      [(match_operand:GPI 2 "aarch64_plus_operand" "rI")
>> +       (match_operand:GPI 3 "register_operand" "r")
>> +       (match_operand:SI 4 "const_int_operand")
>> +       (match_operand:SI 5 "const_int_operand")
>> +       (match_operand:SI 6 "const_int_operand")]
>
> I'm not sure I understand the change here, those comments still look helpful
> enough for understanding the pattern, what have a I missed?

That was part of an attempt to clean up some code. It's unnecessary and I've dropped 
the change.

Attached is the updated patch with some other changes:
- Simplified the atomic_exchange<mode> expander in line with reviews for
   other patches in the series.
- Removed the CC clobber from aarch64_atomic_exchange<mode>_lse, it was
   over-cautious.
- Added a missing entry to the change log (noting a whitespace fix).

Ok for trunk?
Matthew

gcc/
2015-09-21  Matthew Wahab  <matthew.wahab@arm.com>

	* config/aarch64/aarch64-protos.h (aarch64_gen_atomic_ldop):
	Declare.
	* config/aarch64/aarch64.c (aarch64_emit_atomic_swap): New.
	(aarch64_gen_atomic_ldop): New.
	(aarch64_split_atomic_op): Fix whitespace and add a comment.
	* config/aarch64/atomics.md (UNSPECV_ATOMIC_SWP): New.
	(aarch64_compare_and_swap<mode>_lse): Fix some whitespace.
	(atomic_exchange<mode>): Replace with an expander.
	(aarch64_atomic_exchange<mode>): New.
	(aarch64_atomic_exchange<mode>_lse): New.
	(aarch64_atomic_<atomic_optab><mode>): Fix some whitespace.
	(aarch64_atomic_swp<mode>): New.


gcc/testsuite/
2015-09-21  Matthew Wahab  <matthew.wahab@arm.com>

	* gcc.target/aarch64/atomic-inst-ops.inc: (TEST_MODEL): New.
	(TEST_ONE): New.
         * gcc.target/aarch64/atomic-inst-swap.c: New.

Comments

James Greenhalgh Sept. 21, 2015, 11:24 a.m. UTC | #1
On Mon, Sep 21, 2015 at 12:06:40PM +0100, Matthew Wahab wrote:
> On 18/09/15 08:58, James Greenhalgh wrote:
> > On Thu, Sep 17, 2015 at 05:37:55PM +0100, Matthew Wahab wrote:
> 
> >> diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
> >> index 65d2cc9..0e71002 100644
> >> --- a/gcc/config/aarch64/atomics.md
> >> +++ b/gcc/config/aarch64/atomics.md
> >> @@ -27,6 +27,7 @@
> >>       UNSPECV_ATOMIC_CMPSW		; Represent an atomic compare swap.
> >>       UNSPECV_ATOMIC_EXCHG		; Represent an atomic exchange.
> >>       UNSPECV_ATOMIC_CAS			; Represent an atomic CAS.
> >> +    UNSPECV_ATOMIC_SWP			; Represent an atomic SWP.
> >>       UNSPECV_ATOMIC_OP			; Represent an atomic operation.
> >>   ])
> >>
> >> @@ -122,19 +123,19 @@
> >>   )
> >>
> >>   (define_insn_and_split "aarch64_compare_and_swap<mode>_lse"
> >> -  [(set (reg:CC CC_REGNUM)					;; bool out
> >> +  [(set (reg:CC CC_REGNUM)
> >>       (unspec_volatile:CC [(const_int 0)] UNSPECV_ATOMIC_CMPSW))
> >> -   (set (match_operand:GPI 0 "register_operand" "=&r")		;; val out
> >> -    (match_operand:GPI 1 "aarch64_sync_memory_operand" "+Q"))   ;; memory
> >> +   (set (match_operand:GPI 0 "register_operand" "=&r")
> >> +    (match_operand:GPI 1 "aarch64_sync_memory_operand" "+Q"))
> >>      (set (match_dup 1)
> >>       (unspec_volatile:GPI
> >> -      [(match_operand:GPI 2 "aarch64_plus_operand" "rI")	;; expect
> >> -       (match_operand:GPI 3 "register_operand" "r")		;; desired
> >> -       (match_operand:SI 4 "const_int_operand")			;; is_weak
> >> -       (match_operand:SI 5 "const_int_operand")			;; mod_s
> >> -       (match_operand:SI 6 "const_int_operand")]		;; mod_f
> >> +      [(match_operand:GPI 2 "aarch64_plus_operand" "rI")
> >> +       (match_operand:GPI 3 "register_operand" "r")
> >> +       (match_operand:SI 4 "const_int_operand")
> >> +       (match_operand:SI 5 "const_int_operand")
> >> +       (match_operand:SI 6 "const_int_operand")]
> >
> > I'm not sure I understand the change here, those comments still look helpful
> > enough for understanding the pattern, what have a I missed?
> 
> That was part of an attempt to clean up some code. It's unnecessary and I've dropped 
> the change.
> 
> Attached is the updated patch with some other changes:
> - Simplified the atomic_exchange<mode> expander in line with reviews for
>    other patches in the series.
> - Removed the CC clobber from aarch64_atomic_exchange<mode>_lse, it was
>    over-cautious.
> - Added a missing entry to the change log (noting a whitespace fix).
> 
> Ok for trunk?
> Matthew

OK.

Thanks,
James

> gcc/
> 2015-09-21  Matthew Wahab  <matthew.wahab@arm.com>
> 
> 	* config/aarch64/aarch64-protos.h (aarch64_gen_atomic_ldop):
> 	Declare.
> 	* config/aarch64/aarch64.c (aarch64_emit_atomic_swap): New.
> 	(aarch64_gen_atomic_ldop): New.
> 	(aarch64_split_atomic_op): Fix whitespace and add a comment.
> 	* config/aarch64/atomics.md (UNSPECV_ATOMIC_SWP): New.
> 	(aarch64_compare_and_swap<mode>_lse): Fix some whitespace.
> 	(atomic_exchange<mode>): Replace with an expander.
> 	(aarch64_atomic_exchange<mode>): New.
> 	(aarch64_atomic_exchange<mode>_lse): New.
> 	(aarch64_atomic_<atomic_optab><mode>): Fix some whitespace.
> 	(aarch64_atomic_swp<mode>): New.
> 
> 
> gcc/testsuite/
> 2015-09-21  Matthew Wahab  <matthew.wahab@arm.com>
> 
> 	* gcc.target/aarch64/atomic-inst-ops.inc: (TEST_MODEL): New.
> 	(TEST_ONE): New.
>          * gcc.target/aarch64/atomic-inst-swap.c: New.
> 
>
diff mbox

Patch

From 31226dce8d36be98ca95d9165d4147a3bf84d180 Mon Sep 17 00:00:00 2001
From: Matthew Wahab <matthew.wahab@arm.com>
Date: Fri, 7 Aug 2015 17:18:37 +0100
Subject: [PATCH 1/5] Add atomic SWP instruction

Change-Id: I87bf48526cb11e65edd15691f5eab20446e418c9
---
 gcc/config/aarch64/aarch64-protos.h                |  1 +
 gcc/config/aarch64/aarch64.c                       | 46 +++++++++++++-
 gcc/config/aarch64/atomics.md                      | 71 ++++++++++++++++++++--
 .../gcc.target/aarch64/atomic-inst-ops.inc         | 13 ++++
 gcc/testsuite/gcc.target/aarch64/atomic-inst-swp.c | 44 ++++++++++++++
 5 files changed, 170 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/atomic-inst-swp.c

diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index ff19851..eba4c76 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -378,6 +378,7 @@  rtx aarch64_load_tp (rtx);
 void aarch64_expand_compare_and_swap (rtx op[]);
 void aarch64_split_compare_and_swap (rtx op[]);
 void aarch64_gen_atomic_cas (rtx, rtx, rtx, rtx, rtx);
+void aarch64_gen_atomic_ldop (enum rtx_code, rtx, rtx, rtx, rtx);
 void aarch64_split_atomic_op (enum rtx_code, rtx, rtx, rtx, rtx, rtx, rtx);
 
 bool aarch64_gen_adjusted_ldpstp (rtx *, bool, enum machine_mode, RTX_CODE);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 4d2126b..dc05c6e 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11185,11 +11185,54 @@  aarch64_split_compare_and_swap (rtx operands[])
     aarch64_emit_post_barrier (model);
 }
 
+/* Emit an atomic swap.  */
+
+static void
+aarch64_emit_atomic_swap (machine_mode mode, rtx dst, rtx value,
+			  rtx mem, rtx model)
+{
+  rtx (*gen) (rtx, rtx, rtx, rtx);
+
+  switch (mode)
+    {
+    case QImode: gen = gen_aarch64_atomic_swpqi; break;
+    case HImode: gen = gen_aarch64_atomic_swphi; break;
+    case SImode: gen = gen_aarch64_atomic_swpsi; break;
+    case DImode: gen = gen_aarch64_atomic_swpdi; break;
+    default:
+      gcc_unreachable ();
+    }
+
+  emit_insn (gen (dst, mem, value, model));
+}
+
+/* Emit an atomic operation where the architecture supports it.  */
+
+void
+aarch64_gen_atomic_ldop (enum rtx_code code, rtx out_data,
+			 rtx mem, rtx value, rtx model_rtx)
+{
+  machine_mode mode = GET_MODE (mem);
+
+  out_data = gen_lowpart (mode, out_data);
+
+  switch (code)
+    {
+    case SET:
+      aarch64_emit_atomic_swap (mode, out_data, value, mem, model_rtx);
+      return;
+
+    default:
+      /* The operation can't be done with atomic instructions.  */
+      gcc_unreachable ();
+    }
+}
+
 /* Split an atomic operation.  */
 
 void
 aarch64_split_atomic_op (enum rtx_code code, rtx old_out, rtx new_out, rtx mem,
-		     rtx value, rtx model_rtx, rtx cond)
+			 rtx value, rtx model_rtx, rtx cond)
 {
   machine_mode mode = GET_MODE (mem);
   machine_mode wmode = (mode == DImode ? DImode : SImode);
@@ -11198,6 +11241,7 @@  aarch64_split_atomic_op (enum rtx_code code, rtx old_out, rtx new_out, rtx mem,
   rtx_code_label *label;
   rtx x;
 
+  /* Split the atomic operation into a sequence.  */
   label = gen_label_rtx ();
   emit_label (label);
 
diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index 65d2cc9..cb80539 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -27,6 +27,7 @@ 
     UNSPECV_ATOMIC_CMPSW		; Represent an atomic compare swap.
     UNSPECV_ATOMIC_EXCHG		; Represent an atomic exchange.
     UNSPECV_ATOMIC_CAS			; Represent an atomic CAS.
+    UNSPECV_ATOMIC_SWP			; Represent an atomic SWP.
     UNSPECV_ATOMIC_OP			; Represent an atomic operation.
 ])
 
@@ -134,7 +135,7 @@ 
        (match_operand:SI 5 "const_int_operand")			;; mod_s
        (match_operand:SI 6 "const_int_operand")]		;; mod_f
       UNSPECV_ATOMIC_CMPSW))]
-  "TARGET_LSE "
+  "TARGET_LSE"
   "#"
   "&& reload_completed"
   [(const_int 0)]
@@ -146,7 +147,28 @@ 
   }
 )
 
-(define_insn_and_split "atomic_exchange<mode>"
+(define_expand "atomic_exchange<mode>"
+ [(match_operand:ALLI 0 "register_operand" "")
+  (match_operand:ALLI 1 "aarch64_sync_memory_operand" "")
+  (match_operand:ALLI 2 "register_operand" "")
+  (match_operand:SI 3 "const_int_operand" "")]
+  ""
+  {
+    rtx (*gen) (rtx, rtx, rtx, rtx);
+
+    /* Use an atomic SWP when available.  */
+    if (TARGET_LSE)
+      gen = gen_aarch64_atomic_exchange<mode>_lse;
+    else
+      gen = gen_aarch64_atomic_exchange<mode>;
+
+    emit_insn (gen (operands[0], operands[1], operands[2], operands[3]));
+
+    DONE;
+  }
+)
+
+(define_insn_and_split "aarch64_atomic_exchange<mode>"
   [(set (match_operand:ALLI 0 "register_operand" "=&r")		;; output
     (match_operand:ALLI 1 "aarch64_sync_memory_operand" "+Q")) ;; memory
    (set (match_dup 1)
@@ -162,7 +184,26 @@ 
   [(const_int 0)]
   {
     aarch64_split_atomic_op (SET, operands[0], NULL, operands[1],
-			    operands[2], operands[3], operands[4]);
+			     operands[2], operands[3], operands[4]);
+    DONE;
+  }
+)
+
+(define_insn_and_split "aarch64_atomic_exchange<mode>_lse"
+  [(set (match_operand:ALLI 0 "register_operand" "=&r")
+    (match_operand:ALLI 1 "aarch64_sync_memory_operand" "+Q"))
+   (set (match_dup 1)
+    (unspec_volatile:ALLI
+      [(match_operand:ALLI 2 "register_operand" "r")
+       (match_operand:SI 3 "const_int_operand" "")]
+      UNSPECV_ATOMIC_EXCHG))]
+  "TARGET_LSE"
+  "#"
+  "&& reload_completed"
+  [(const_int 0)]
+  {
+    aarch64_gen_atomic_ldop (SET, operands[0], operands[1],
+			     operands[2], operands[3]);
     DONE;
   }
 )
@@ -183,7 +224,7 @@ 
   [(const_int 0)]
   {
     aarch64_split_atomic_op (<CODE>, NULL, operands[3], operands[0],
-			    operands[1], operands[2], operands[4]);
+			     operands[1], operands[2], operands[4]);
     DONE;
   }
 )
@@ -425,6 +466,28 @@ 
 
 ;; ARMv8.1 LSE instructions.
 
+;; Atomic swap with memory.
+(define_insn "aarch64_atomic_swp<mode>"
+ [(set (match_operand:ALLI 0 "register_operand" "+&r")
+   (match_operand:ALLI 1 "aarch64_sync_memory_operand" "+Q"))
+  (set (match_dup 1)
+   (unspec_volatile:ALLI
+    [(match_operand:ALLI 2 "register_operand" "r")
+     (match_operand:SI 3 "const_int_operand" "")]
+    UNSPECV_ATOMIC_SWP))]
+  "TARGET_LSE && reload_completed"
+  {
+    enum memmodel model = memmodel_from_int (INTVAL (operands[3]));
+    if (is_mm_relaxed (model))
+      return "swp<atomic_sfx>\t%<w>2, %<w>0, %1";
+    else if (is_mm_acquire (model) || is_mm_consume (model))
+      return "swpa<atomic_sfx>\t%<w>2, %<w>0, %1";
+    else if (is_mm_release (model))
+      return "swpl<atomic_sfx>\t%<w>2, %<w>0, %1";
+    else
+      return "swpal<atomic_sfx>\t%<w>2, %<w>0, %1";
+  })
+
 ;; Atomic compare-and-swap: HI and smaller modes.
 
 (define_insn "aarch64_atomic_cas<mode>"
diff --git a/gcc/testsuite/gcc.target/aarch64/atomic-inst-ops.inc b/gcc/testsuite/gcc.target/aarch64/atomic-inst-ops.inc
index 72c7e5c..c2fdcba 100644
--- a/gcc/testsuite/gcc.target/aarch64/atomic-inst-ops.inc
+++ b/gcc/testsuite/gcc.target/aarch64/atomic-inst-ops.inc
@@ -32,6 +32,15 @@  typedef __uint128_t uint128;
   TEST_M##N (NAME, FN, int128, MODEL1, MODEL2)		\
   TEST_M##N (NAME, FN, uint128, MODEL1, MODEL2)
 
+/* Models to test.  */
+#define TEST_MODEL(NAME, FN, N)					\
+  TEST_TY (NAME##_relaxed, FN, N, __ATOMIC_RELAXED, DUMMY)	\
+  TEST_TY (NAME##_consume, FN, N, __ATOMIC_CONSUME, DUMMY)	\
+  TEST_TY (NAME##_acquire, FN, N, __ATOMIC_ACQUIRE, DUMMY)	\
+  TEST_TY (NAME##_release, FN, N, __ATOMIC_RELEASE, DUMMY)	\
+  TEST_TY (NAME##_acq_rel, FN, N, __ATOMIC_ACQ_REL, DUMMY)	\
+  TEST_TY (NAME##_seq_cst, FN, N, __ATOMIC_SEQ_CST, DUMMY)	\
+
 /* Cross-product of models to test.  */
 #define TEST_MODEL_M1(NAME, FN, N, M)			\
   TEST_TY (NAME##_relaxed, FN, N, M, __ATOMIC_RELAXED)	\
@@ -51,3 +60,7 @@  typedef __uint128_t uint128;
 
 /* Expand functions for a cross-product of memory models and types.  */
 #define TEST_TWO(NAME, FN) TEST_MODEL_M2 (NAME, FN)
+
+/* Expand functions for a set of memory models and types.  */
+#define TEST_ONE(NAME, FN) TEST_MODEL (NAME, FN, 1)
+
diff --git a/gcc/testsuite/gcc.target/aarch64/atomic-inst-swp.c b/gcc/testsuite/gcc.target/aarch64/atomic-inst-swp.c
new file mode 100644
index 0000000..dabc9b9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/atomic-inst-swp.c
@@ -0,0 +1,44 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=armv8-a+lse" } */
+
+/* Test ARMv8.1-A SWP instruction.  */
+
+#include "atomic-inst-ops.inc"
+
+#define TEST TEST_ONE
+
+#define SWAP_ATOMIC(FN, TY, MODEL)					\
+  TY FNNAME (FN, TY) (TY* val, TY foo)					\
+  {									\
+    return __atomic_exchange_n (val, foo, MODEL);			\
+  }
+
+#define SWAP_ATOMIC_NORETURN(FN, TY, MODEL)				\
+  void FNNAME (FN, TY) (TY* val, TY* foo, TY* bar)			\
+  {									\
+    __atomic_exchange (val, foo, bar, MODEL);				\
+  }
+
+
+TEST (swap_atomic, SWAP_ATOMIC)
+TEST (swap_atomic_noreturn, SWAP_ATOMIC_NORETURN)
+
+
+/* { dg-final { scan-assembler-times "swpb\t" 4} } */
+/* { dg-final { scan-assembler-times "swpab\t" 8} } */
+/* { dg-final { scan-assembler-times "swplb\t" 4} } */
+/* { dg-final { scan-assembler-times "swpalb\t" 8} } */
+
+/* { dg-final { scan-assembler-times "swph\t" 4} } */
+/* { dg-final { scan-assembler-times "swpah\t" 8} } */
+/* { dg-final { scan-assembler-times "swplh\t" 4} } */
+/* { dg-final { scan-assembler-times "swpalh\t" 8} } */
+
+/* { dg-final { scan-assembler-times "swp\t" 8} } */
+/* { dg-final { scan-assembler-times "swpa\t" 16} } */
+/* { dg-final { scan-assembler-times "swpl\t" 8} } */
+/* { dg-final { scan-assembler-times "swpal\t" 16} } */
+
+/* { dg-final { scan-assembler-not "ldaxr\t" } } */
+/* { dg-final { scan-assembler-not "stlxr\t" } } */
+/* { dg-final { scan-assembler-not "dmb" } } */
-- 
2.1.4