| Message ID | mptmsmp4yl7.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | recog: Avoid validate_change shortcut for groups [PR115782] | expand |
On 7/9/24 11:15 PM, Richard Sandiford wrote: > In this PR, due to the -f flags, we ended up with: > > bb1: r10=r10 > ... > bb2: r10=r10 > ... > bb3: ...=r10 > > with bb1->bb2 and bb1->bb3. > > late-combine successfully combined the bb1->bb2 def-use and set > the insn code to NOOP_MOVE_INSN_CODE. The bb1->bb3 combination > then failed for... reasons. At this point, everything should have > been rewound to its original state. > > However, substituting r10=r10 into r10=r10 gives r10=r10, and > validate_change had an early-out for no-op rtl changes. This meant > that validate_change did not register a change for the bb2 insn and > so did not save its old insn code. The NOOP_MOVE_INSN_CODE therefore > persisted even after the attempt had been rewound. > > IMO it'd be too cumbersome and error-prone to expect all users of > validate_change to be aware of this possibility. If code is using > validate_change with in_group=1, I think it has a reasonable expectation > that a change will be registered and that the insn code will be saved > (and restored on cancel). This patch therefore limits the shortcut > to the !in_group case. > > Bootstrapped & regression-tested on aarch64-linux-gnu & x86_64-linux-gnu. > OK to install? > > Richard > > > gcc/ > PR rtl-optimization/115782 > * recog.cc (validate_change_1): Suppress early exit for no-op > changes that are part of a group. > > gcc/testsuite/ > PR rtl-optimization/115782 > * gcc.dg/pr115782.c: New test. OK jeff
diff --git a/gcc/recog.cc b/gcc/recog.cc index 36507f3f57c..7710c55b745 100644 --- a/gcc/recog.cc +++ b/gcc/recog.cc @@ -230,7 +230,12 @@ validate_change_1 (rtx object, rtx *loc, rtx new_rtx, bool in_group, new_len = -1; } - if ((old == new_rtx || rtx_equal_p (old, new_rtx)) + /* When a change is part of a group, callers expect to be able to change + INSN_CODE after making the change and have the code reset to its old + value by a later cancel_changes. We therefore need to register group + changes even if they're no-ops. */ + if (!in_group + && (old == new_rtx || rtx_equal_p (old, new_rtx)) && (new_len < 0 || XVECLEN (new_rtx, 0) == new_len)) return true; diff --git a/gcc/testsuite/gcc.dg/pr115782.c b/gcc/testsuite/gcc.dg/pr115782.c new file mode 100644 index 00000000000..f4d11cc6d0f --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr115782.c @@ -0,0 +1,23 @@ +// { dg-require-effective-target lp64 } +// { dg-options "-O2 -fno-guess-branch-probability -fgcse-sm -fno-expensive-optimizations -fno-gcse" } + +int printf(const char *, ...); +int a, b, c, d, e, f, g, i, j, m, h; +long k, l, n, o; +int main() { + int p = e, r = i << a, q = r & b; + k = 4073709551613; + l = m = c = -(c >> j); + d = g ^ h ^ 4073709551613; + n = q - h; + o = ~d; + f = c * 4073709551613 / 409725 ^ r; + if ((n && m) || (q && j) || a) + return 0; + d = o | p; + if (g) + printf("0"); + d = p; + c++; + return 0; +}