public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/vendors/ARM/heads/morello)] aarch64: Make aarch64_atomic_exchange<mode> require !TARGET_LSE
@ 2022-05-06 14:43 Matthew Malcomson
  0 siblings, 0 replies; only message in thread
From: Matthew Malcomson @ 2022-05-06 14:43 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:d2006d5de48b5e3c1d6062e095c0eef5e8e23242

commit d2006d5de48b5e3c1d6062e095c0eef5e8e23242
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Tue Apr 26 10:47:09 2022 +0100

    aarch64: Make aarch64_atomic_exchange<mode> require !TARGET_LSE
    
    aarch64_atomic_exchange<mode>_lse comes after
    aarch64_atomic_exchange<mode> and contains the same pattern,
    just with fewer clobbers.  This doesn't matter when we first
    expand the instructions, since we generate a pattern without
    clobbers for LSE and a pattern with clobbers otherwise.
    
    However, if combine finds a valid optimisation later, it will
    try the patterns in order and pick the first one that matches.
    This matching process ignores trailing clobbers, since combine
    can add those itself where necessary.
    
    The net result is that combine can end up turning an LSE
    operation into a slower non-LSE one.  I don't have a standalone
    testcase for this, but it was shown up by tests in later patches.
    
    Three possible fixes are:
    
    (1) Differentiate the main part of the patterns (e.g. by using
        different unspecs).
    
    (2) Swap the patterns around.
    
    (3) Make aarch64_atomic_exchange<mode> require !TARGET_LSE.
    
    (1) feels like a hack, and would be a non-starter if we had direct rtl
    codes for representing the operation.  (2) would be OK, but the ordering
    requirement is subtle enough that it would need a comment.  That would
    leave us having to explain away why we allow both patterns for LSE,
    but choose an order in which the second will never actually be used
    (for LSE) in practice.
    
    The patch therefore goes for (3).

Diff:
---
 gcc/config/aarch64/atomics.md | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index 9d651e32749..f3a220f806c 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -226,7 +226,11 @@
       UNSPECV_ATOMIC_EXCHG))
    (clobber (reg:CC CC_REGNUM))
    (clobber (match_scratch:SI 4 "=&r"))]
-  ""
+  ;; The LSE version of this pattern has the same form as this base Armv8-A
+  ;; version, but without the clobbers.  We always want to use the LSE version
+  ;; where possible, so prevent combine from converting an LSE pattern into
+  ;; a base Armv8-A one.
+  "!TARGET_LSE"
   "#"
   "&& epilogue_completed"
   [(const_int 0)]


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-05-06 14:43 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-06 14:43 [gcc(refs/vendors/ARM/heads/morello)] aarch64: Make aarch64_atomic_exchange<mode> require !TARGET_LSE Matthew Malcomson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).