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

[James Greenhalgh <james.greenhalgh@arm.com>]

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.
> 
>