https://gcc.gnu.org/g:f559ac896942ffe0e2315d0a4d8b8b517a16d607
commit r15-7282-gf559ac896942ffe0e2315d0a4d8b8b517a16d607
Author: Richard Sandiford <richard.sandif...@arm.com>
Date: Thu Jan 30 08:59:30 2025 +0000
pair-fusion: Check for invalid use arrays [PR118320]
As Andrew says in the bugzilla comments, this PR is about a case where
we tried to fuse two stores of x0, one in which x0 was defined and one
in which it was undefined. merge_access_arrays failed on the conflict,
but the failure wasn't caught.
Normally the hazard detection code would fail if the instructions
had incompatible uses. However, an undefined use doesn't impose
many restrictions on movements. I think this is likely to be the
only case where hazard detection isn't enough.
As Andrew notes in bugzilla, it might be possible to allow uses
of defined and undefined values to be merged to the defined value.
But that sounds dangerous in the general case, as an rtl-ssa-level
decision. We might run the risk of turning conditional UB into
unconditional UB. And LLVM proves that the definition of "undef"
isn't simple.
gcc/
PR rtl-optimization/118320
* pair-fusion.cc (pair_fusion_bb_info::fuse_pair): Commonize
the merge of input_uses and return early if it fails.
gcc/testsuite/
PR rtl-optimization/118320
* g++.dg/torture/pr118320.C: New test.
Diff:
---
gcc/pair-fusion.cc | 32 +++++++++++++++++++++-----------
gcc/testsuite/g++.dg/torture/pr118320.C | 15 +++++++++++++++
2 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/gcc/pair-fusion.cc b/gcc/pair-fusion.cc
index 602e572ab6c7..5708d0f3b671 100644
--- a/gcc/pair-fusion.cc
+++ b/gcc/pair-fusion.cc
@@ -1730,6 +1730,24 @@ pair_fusion_bb_info::fuse_pair (bool load_p,
input_uses[i] = remove_note_accesses (attempt, input_uses[i]);
}
+ // Get the uses that the pair instruction would have, and punt if
+ // the unpaired instructions use different definitions of the same
+ // register. That would normally be caught as a side-effect of
+ // hazard detection below, but this check also deals with cases
+ // in which one use is undefined and the other isn't.
+ auto new_uses = merge_access_arrays (attempt,
+ drop_memory_access (input_uses[0]),
+ drop_memory_access (input_uses[1]));
+ if (!new_uses.is_valid ())
+ {
+ if (dump_file)
+ fprintf (dump_file,
+ " load pair: i%d and i%d use different definiitions of"
+ " the same register\n",
+ insns[0]->uid (), insns[1]->uid ());
+ return false;
+ }
+
// Edge case: if the first insn is a writeback load and the
// second insn is a non-writeback load which transfers into the base
// register, then we should drop the writeback altogether as the
@@ -1852,11 +1870,7 @@ pair_fusion_bb_info::fuse_pair (bool load_p,
input_defs[1]);
gcc_assert (pair_change->new_defs.is_valid ());
- pair_change->new_uses
- = merge_access_arrays (attempt,
- drop_memory_access (input_uses[0]),
- drop_memory_access (input_uses[1]));
- gcc_assert (pair_change->new_uses.is_valid ());
+ pair_change->new_uses = new_uses;
set_pair_pat (pair_change);
}
else
@@ -1877,9 +1891,7 @@ pair_fusion_bb_info::fuse_pair (bool load_p,
case Action::CHANGE:
{
set_pair_pat (change);
- change->new_uses = merge_access_arrays (attempt,
- input_uses[0],
- input_uses[1]);
+ change->new_uses = new_uses;
auto d1 = drop_memory_access (input_defs[0]);
auto d2 = drop_memory_access (input_defs[1]);
change->new_defs = merge_access_arrays (attempt, d1, d2);
@@ -1907,9 +1919,7 @@ pair_fusion_bb_info::fuse_pair (bool load_p,
auto new_insn = crtl->ssa->create_insn (attempt, INSN, pair_pat);
change = make_change (new_insn);
change->move_range = move_range;
- change->new_uses = merge_access_arrays (attempt,
- input_uses[0],
- input_uses[1]);
+ change->new_uses = new_uses;
gcc_assert (change->new_uses.is_valid ());
auto d1 = drop_memory_access (input_defs[0]);
diff --git a/gcc/testsuite/g++.dg/torture/pr118320.C
b/gcc/testsuite/g++.dg/torture/pr118320.C
new file mode 100644
index 000000000000..228d79860b5d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr118320.C
@@ -0,0 +1,15 @@
+// { dg-additional-options "-fno-tree-sra" } */
+
+struct T{
+ long f[2];
+};
+void f1(int);
+void g(T&);
+void f1()
+{
+ T a;
+ a.~T(); // To force the clobber
+ a.f[1] = 1;
+ T b = a;
+ g(b);
+}
