On Thu, Sep 17, 2015 at 05:37:55PM +0100, Matthew Wahab wrote: > Hello, > > ARMv8.1 adds atomic swap and atomic load-operate instructions with > optional memory ordering specifiers. This patch series adds the > instructions to GCC, making them available with -march=armv8.1-a or > -march=armv8+lse, and uses them to implement the __sync and __atomic > builtins. > > The ARMv8.1 swap instruction swaps the value in a register with a value > in memory. The load-operate instructions load a value from memory, > update it with the result of an operation and store the result in > memory. > > This series uses the swap instruction to implement the atomic_exchange > patterns and the load-operate instructions to implement the > atomic_fetch_<op> and atomic_<op>_fetch patterns. For the > atomic_<op>_fetch patterns, the value returned as the result of the > operation has to be recalculated from the loaded data. The ARMv8 BIC > instruction is added so that it can be used for this recalculation. > > The patches in this series > - add and use the atomic swap instruction. > - add the Aarch64 BIC instruction, > - add the ARMv8.1 load-operate instructions, > - use the load-operate instructions to implement the atomic_fetch_<op> > patterns, > - use the load-operate instructions to implement the patterns > atomic_<op>_fetch patterns, > > The code-generation changes in this series are based around a new > function, aarch64_gen_atomic_ldop, which takes the operation to be > implemented and emits the appropriate code, making use of the atomic > instruction. This follows the existing uses aarch64_split_atomic_op for > the same purpose when atomic instructions aren't available. > > This patch adds the ARMv8.1 SWAP instruction and function > aarch64_gen_atomic_ldop and changes the implementation of the > atomic_exchange pattern to the atomic instruction when it is available. > > The general form of the code generated for an atomic_exchange, with > destination D, source S, memory address A and memory order MO is: > > swp<mo><sz> S, D, [A] > > where > <mo> is one of {'', 'a', 'l', 'al'} depending on memory order MO. > <sz> is one of {'', 'b', 'h'} depending on the data size. > > This patch also adds tests for the changes. These reuse the support code > introduced for the atomic CAS tests, adding macros to test functions > taking one memory ordering argument. These are used to iteratively > define functions using the __atomic_exchange builtins, which should be > implemented using the atomic swap. > > Tested the series for aarch64-none-linux-gnu with native bootstrap and > make check. Also tested for aarch64-none-elf with cross-compiled > check-gcc on an ARMv8.1 emulator with +lse enabled by default. > > Ok for trunk?
OK, though I have a question on one patch hunk. > gcc/ > 2015-09-17 Matthew Wahab <matthew.wa...@arm.com> > > * config/aarch64/aarch64-protos.h (aarch64_gen_atomic_ldop): > Declare. > * config/aarch64/aarch64.c (aarch64_emit_atomic_swp): New. > (aarch64_gen_atomic_ldop): New. > (aarch64_split_atomic_op): Fix whitespace and add a comment. > * config/aarch64/atomics.md (UNSPECV_ATOMIC_SWP): New. > (atomic_compare_and_swap<mode>_lse): Remove comments and fix > 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-17 Matthew Wahab <matthew.wa...@arm.com> > > * gcc.target/aarch64/atomic-inst-ops.inc: (TEST_MODEL): New. > (TEST_ONE): New. > * gcc.target/aarch64/atomic-inst-swap.c: New. > > 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? Thanks, James