public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r14-8353] aarch64: Don't assert recog success in ldp/stp pass [PR113114]
@ 2024-01-23 10:58 Alex Coplan
  0 siblings, 0 replies; only message in thread
From: Alex Coplan @ 2024-01-23 10:58 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:20e18106fac2d11ee43683291ff11d76da41d50b

commit r14-8353-g20e18106fac2d11ee43683291ff11d76da41d50b
Author: Alex Coplan <alex.coplan@arm.com>
Date:   Thu Jan 18 17:53:01 2024 +0000

    aarch64: Don't assert recog success in ldp/stp pass [PR113114]
    
    The PR shows two different cases where try_promote_writeback produces an
    RTL pattern which isn't recognized.  Currently this leads to an ICE, as
    we assert recog success, but I think it's better just to back out of the
    changes gracefully if recog fails (as we do in the main fuse_pair case).
    
    In theory since we check the ranges here recog shouldn't fail (which is
    why I had the assert in the first place), but the PR shows an edge case
    in the patterns where if we form a pre-writeback pair where the
    writeback offset is exactly -S, where S is the size in bytes of one
    transfer register, we fail to match the expected pattern as the patterns
    look explicitly for plus operands in the mems.  I think fixing this
    would require adding at least four new special-case patterns to
    aarch64.md for what doesn't seem to be a particularly useful variant of
    the insns.  Even if we were to do that, I think it would be GCC 15
    material, and it's better to just punt for GCC 14.
    
    The ILP32 case in the PR is a bit different, as that shows us trying to
    combine a pair with DImode base register operands in the mems together
    with an SImode trailing update of the base register.  This leads to us
    forming an RTL pattern which references the base register in both SImode
    and DImode, which also fails to recog.  Again, I think it's best just to
    take the missed optimization for now.  If we really want to make this
    (try_promote_writeback) work for ILP32, we can try to do it for GCC 15.
    
    gcc/ChangeLog:
    
            PR target/113114
            * config/aarch64/aarch64-ldp-fusion.cc (try_promote_writeback):
            Don't assert recog success, just punt if the writeback pair
            isn't recognized.
    
    gcc/testsuite/ChangeLog:
    
            PR target/113114
            * gcc.c-torture/compile/pr113114.c: New test.
            * gcc.target/aarch64/pr113114.c: New test.

Diff:
---
 gcc/config/aarch64/aarch64-ldp-fusion.cc       | 10 +++++++++-
 gcc/testsuite/gcc.c-torture/compile/pr113114.c |  9 +++++++++
 gcc/testsuite/gcc.target/aarch64/pr113114.c    |  7 +++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/gcc/config/aarch64/aarch64-ldp-fusion.cc b/gcc/config/aarch64/aarch64-ldp-fusion.cc
index 689a8c884bd..19142153f41 100644
--- a/gcc/config/aarch64/aarch64-ldp-fusion.cc
+++ b/gcc/config/aarch64/aarch64-ldp-fusion.cc
@@ -2672,7 +2672,15 @@ try_promote_writeback (insn_info *insn)
   for (unsigned i = 0; i < ARRAY_SIZE (changes); i++)
     gcc_assert (rtl_ssa::restrict_movement_ignoring (*changes[i], is_changing));
 
-  gcc_assert (rtl_ssa::recog_ignoring (attempt, pair_change, is_changing));
+  if (!rtl_ssa::recog_ignoring (attempt, pair_change, is_changing))
+    {
+      if (dump_file)
+	fprintf (dump_file, "i%d: recog failed on wb pair, bailing out\n",
+		 insn->uid ());
+      cancel_changes (0);
+      return;
+    }
+
   gcc_assert (crtl->ssa->verify_insn_changes (changes));
   confirm_change_group ();
   crtl->ssa->change_insns (changes);
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr113114.c b/gcc/testsuite/gcc.c-torture/compile/pr113114.c
new file mode 100644
index 00000000000..978e594eb3d
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr113114.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-funroll-loops" } */
+float val[128];
+float x;
+void bar() {
+  int i = 55;
+  for (; i >= 0; --i)
+    x += val[i];
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/pr113114.c b/gcc/testsuite/gcc.target/aarch64/pr113114.c
new file mode 100644
index 00000000000..5b0383c2435
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr113114.c
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-mabi=ilp32 -O -mearly-ldp-fusion -mlate-ldp-fusion" } */
+void foo_n(double *a) {
+  int i = 1;
+  for (; i < (int)foo_n; i++)
+    a[i] = a[i - 1] + a[i + 1] * a[i];
+}

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

only message in thread, other threads:[~2024-01-23 10:58 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-23 10:58 [gcc r14-8353] aarch64: Don't assert recog success in ldp/stp pass [PR113114] Alex Coplan

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