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 <[email protected]> * 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 <[email protected]> * gcc.target/aarch64/atomic-inst-ops.inc: (TEST_MODEL): New. (TEST_ONE): New. * gcc.target/aarch64/atomic-inst-swap.c: New.
>From 31226dce8d36be98ca95d9165d4147a3bf84d180 Mon Sep 17 00:00:00 2001 From: Matthew Wahab <[email protected]> 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
