[gcc r14-8367] aarch64: Don't record hazards against paired insns [PR113356]

[gcc r14-8367] aarch64: Don't record hazards against paired insns [PR113356]

[Alex Coplan]

https://gcc.gnu.org/g:639ae543449a8d6525303458497bd4b897660ec3

commit r14-8367-g639ae543449a8d6525303458497bd4b897660ec3
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Mon Jan 15 11:07:48 2024 +0000

    aarch64: Don't record hazards against paired insns [PR113356]
    
    For the testcase in the PR, we try to pair insns where the first has
    writeback and the second uses the updated base register.  This causes us
    to record a hazard against the second insn, thus narrowing the move
    range away from the end of the BB.
    
    However, it isn't meaningful to record hazards against the other insn
    in the pair, as this doesn't change which pairs can be formed, and also
    doesn't change where the pair is formed (from the perspective of
    nondebug insns).
    
    To see why this is the case, consider the two cases:
    
     - Suppoe we are finding hazards for insns[0].  If we record a hazard
       against insns[1], then range.last becomes
       insns[1]->prev_nondebug_insn (), but note that this is equivalent to
       inserting after insns[1] (since insns[1] is being changed).
     - Now consider finding hazards for insns[1].  Suppose we record
       insns[0] as a hazard.  Then we set range.first = insns[0], which is a
       no-op.
    
    As such, it seems better to never record hazards against the other insn
    in the pair, as we check whether the insns themselves are suitable for
    combination separately (e.g. for ldp checking that they use distinct
    transfer registers).  Avoiding unnecessarily narrowing the move range
    avoids unnecessarily re-ordering over debug insns.
    
    This should also mean that we can only narrow the move range away from
    the end of the BB in the case that we record a hazard for insns[0]
    against insns[1]->prev_nondebug_insn () or earlier.  This means that for
    the non-call-exceptions case, either the move range includes insns[1],
    or we reject the pair (thus the assert tripped in the PR should always
    hold).
    
    gcc/ChangeLog:
    
            PR target/113356
            * config/aarch64/aarch64-ldp-fusion.cc (ldp_bb_info::try_fuse_pair):
            Don't record hazards against the opposite insn in the pair.
    
    gcc/testsuite/ChangeLog:
    
            PR target/113356
            * gcc.target/aarch64/pr113356.C: New test.

Diff:
---
 gcc/config/aarch64/aarch64-ldp-fusion.cc    | 4 ++--
 gcc/testsuite/gcc.target/aarch64/pr113356.C | 8 ++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gcc/config/aarch64/aarch64-ldp-fusion.cc b/gcc/config/aarch64/aarch64-ldp-fusion.cc
index da69fae22e7..bbb95d6c820 100644
--- a/gcc/config/aarch64/aarch64-ldp-fusion.cc
+++ b/gcc/config/aarch64/aarch64-ldp-fusion.cc
@@ -2216,11 +2216,11 @@ ldp_bb_info::try_fuse_pair (bool load_p, unsigned access_size,
 	  ignore[j] = &XEXP (cand_mems[j], 0);
 
       insn_info *h = first_hazard_after (insns[0], ignore[0]);
-      if (h && *h <= *insns[1])
+      if (h && *h < *insns[1])
 	cand.hazards[0] = h;
 
       h = latest_hazard_before (insns[1], ignore[1]);
-      if (h && *h >= *insns[0])
+      if (h && *h > *insns[0])
 	cand.hazards[1] = h;
 
       if (!cand.viable ())
diff --git a/gcc/testsuite/gcc.target/aarch64/pr113356.C b/gcc/testsuite/gcc.target/aarch64/pr113356.C
new file mode 100644
index 00000000000..0de17a54a53
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr113356.C
@@ -0,0 +1,8 @@
+// { dg-do compile }
+// { dg-options "-Os -fnon-call-exceptions -mearly-ldp-fusion -fno-lifetime-dse -fno-forward-propagate" }
+struct Class1 {
+  virtual ~Class1() {}
+  unsigned Field1;
+};
+struct Class4 : virtual Class1 {};
+int main() { Class4 var1; }