From: Kyrylo Tkachov <[email protected]>
init_noce_multiple_sets_info records earlier SET destinations that are
mentioned by a later SET source. When a pseudo is set more than once, only
its most recent prior definition reaches that source.
Recording every definition is unsafe in the second
noce_convert_multiple_sets_1 attempt. The newest definition can be emitted
directly into its target pseudo, making its replacement a no-op. A later
replacement using an older definition then substitutes a stale value.
PR126184 contains the following dependency chain:
c = x + 1;
x = c * y;
c = z + 3;
y = c * x;
The unfixed conversion computes the last multiply with the temporary that
holds `x + 1`, rather than the reaching `z + 3` value. This makes the
testcase abort on AArch64.
The unfixed final sequence forms `x + 1` in x0 and later uses x0 as the
multiply operand:
add x0, x1, 1
csel x1, x1, x3, eq
mul x0, x1, x0
With the fix, x3 retains z until `z + 3` is formed and used by the multiply:
add x3, x3, 3
csel x0, x0, x1, ne
mul x3, x0, x3
Walk definitions from newest to oldest and record only the first match for
each pseudo. Restrict this conversion to pseudo destinations. Hard
registers can have overlapping definitions in different modes, and an exact
RTL replacement cannot represent the value produced by a partial or
mode-changing definition.
Add an AArch64 C execution test for the reported pseudo dependency. Add an
RTL execution test that verifies a mixed-mode hard-register block is left
unconverted.
Bootstrapped and tested on aarch64-none-linux-gnu.
Ok for trunk and GCC 16 branch?
gcc/ChangeLog:
PR rtl-optimization/126184
* ifcvt.cc (init_noce_multiple_sets_info): Record only the most
recent prior definition of each pseudo register.
(bb_ok_for_noce_convert_multiple_sets): Reject hard-register
destinations.
gcc/testsuite/ChangeLog:
PR rtl-optimization/126184
* gcc.target/aarch64/pr126184.c: New test.
* gcc.target/aarch64/pr126184-2.c: New test.
Signed-off-by: Kyrylo Tkachov <[email protected]>
---
gcc/ifcvt.cc | 18 +++--
gcc/testsuite/gcc.target/aarch64/pr126184-2.c | 75 +++++++++++++++++++
gcc/testsuite/gcc.target/aarch64/pr126184.c | 31 ++++++++
3 files changed, 117 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr126184-2.c
create mode 100644 gcc/testsuite/gcc.target/aarch64/pr126184.c
diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index 1df5d54efa3..d75f1a8ed26 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -4225,15 +4225,17 @@ init_noce_multiple_sets_info (basic_block bb,
rtx src = SET_SRC (set);
rtx dest = SET_DEST (set);
- gcc_checking_assert (REG_P (dest));
+ gcc_checking_assert (REG_P (dest) && !HARD_REGISTER_P (dest));
info->need_cmov = bitmap_bit_p (bb_live_out, REGNO (dest));
- /* Check if the current SET's source is the same
- as any previously seen destination.
+ /* Check if the current SET's source mentions any previously seen
+ destination. Keep only the newest definition of each pseudo register.
This is quadratic but the number of insns in BB
is bounded by PARAM_MAX_RTL_IF_CONVERSION_INSNS. */
+ auto_bitmap rewired_regs;
for (int i = count - 1; i >= 0; --i)
- if (reg_mentioned_p (dests[i], src))
+ if (reg_mentioned_p (dests[i], src)
+ && bitmap_set_bit (rewired_regs, REGNO (dests[i])))
insn_info[count]->rewired_src.safe_push (i);
dests.safe_push (dest);
@@ -4270,10 +4272,12 @@ bb_ok_for_noce_convert_multiple_sets (basic_block
test_bb, unsigned *cost)
rtx dest = SET_DEST (set);
rtx src = SET_SRC (set);
- /* Do not handle anything involving memory loads/stores since it might
- violate data-race-freedom guarantees. Make sure we can force SRC
+ /* Dependency rewiring requires pseudo destinations with one fixed
+ mode. Do not handle anything involving memory loads/stores since it
+ might violate data-race-freedom guarantees. Make sure we can force SRC
to a register as that may be needed in try_emit_cmove_seq. */
- if (!REG_P (dest) || contains_mem_rtx_p (src)
+ if (!REG_P (dest) || HARD_REGISTER_P (dest)
+ || contains_mem_rtx_p (src)
|| !noce_can_force_operand (src))
return false;
diff --git a/gcc/testsuite/gcc.target/aarch64/pr126184-2.c
b/gcc/testsuite/gcc.target/aarch64/pr126184-2.c
new file mode 100644
index 00000000000..5e027466041
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr126184-2.c
@@ -0,0 +1,75 @@
+/* PR rtl-optimization/126184 */
+/* { dg-do run } */
+/* { dg-options "-O2 --param=max-rtl-if-conversion-unpredictable-cost=100
-fdump-rtl-ce1" } */
+
+/* Mixed-mode hard-register definitions cannot be rewired through a single
+ temporary. Verify that the multi-set conversion rejects the block. */
+
+unsigned long long __RTL (startwith ("ce1"))
+f (unsigned long long c, unsigned long long x, unsigned long long y)
+{
+(function "f"
+ (param "c" (DECL_RTL (reg/v:DI x4 [ c ]))
+ (DECL_RTL_INCOMING (reg/v:DI x0 [ c ])))
+ (param "x" (DECL_RTL (reg/v:DI <1> [ x ]))
+ (DECL_RTL_INCOMING (reg/v:DI x1 [ x ])))
+ (param "y" (DECL_RTL (reg/v:DI <2> [ y ]))
+ (DECL_RTL_INCOMING (reg/v:DI x2 [ y ])))
+ (insn-chain
+ (block 2
+ (edge-from entry (flags "FALLTHRU"))
+ (cnote 1 [bb 2] NOTE_INSN_BASIC_BLOCK)
+ (cinsn 2 (set (reg/v:DI x4 [ c ]) (reg:DI x0)))
+ (cinsn 3 (set (reg/v:DI <1> [ x ]) (reg:DI x1)))
+ (cinsn 4 (set (reg/v:DI <2> [ y ]) (reg:DI x2)))
+ (cinsn 5 (set (reg:DI <3>)
+ (plus:DI (reg/v:DI <1>) (const_int 11))))
+ (cinsn 6 (set (reg:DI <4>)
+ (plus:DI (reg/v:DI <2>) (const_int -2))))
+ (cinsn 7 (set (reg:CC cc)
+ (compare:CC (reg/v:DI x4) (const_int 0))))
+ (cjump_insn 8 (set (pc)
+ (if_then_else (eq (reg:CC cc) (const_int 0))
+ (label_ref 30) (pc))))
+ (edge-to 3 (flags "FALLTHRU"))
+ (edge-to 4)
+ )
+ (block 3
+ (edge-from 2 (flags "FALLTHRU"))
+ (cnote 9 [bb 3] NOTE_INSN_BASIC_BLOCK)
+ (cinsn 10 (set (reg/v:DI x4 [ c ])
+ (plus:DI (reg/v:DI <1>) (const_int 1))))
+ (cinsn 11 (set (reg:DI <3>)
+ (plus:DI (reg/v:DI x4) (reg/v:DI <2>))))
+ (cinsn 12 (set (reg:SI x4) (const_int 26)))
+ (cinsn 13 (set (reg:DI <4>)
+ (plus:DI (reg/v:DI x4) (reg/v:DI <1>))))
+ (edge-to 4 (flags "FALLTHRU"))
+ )
+ (block 4
+ (edge-from 2)
+ (edge-from 3 (flags "FALLTHRU"))
+ (clabel 30 2)
+ (cnote 31 [bb 4] NOTE_INSN_BASIC_BLOCK)
+ (cinsn 32 (set (reg:DI <5>)
+ (plus:DI (reg:DI <3>) (reg:DI <4>))))
+ (cinsn 33 (set (reg/i:DI x0) (reg:DI <5>)))
+ (cinsn 34 (use (reg/i:DI x0)))
+ (edge-to exit (flags "FALLTHRU"))
+ )
+ )
+ (crtl (return_rtx (reg/i:DI x0)))
+)
+}
+
+int
+main (void)
+{
+ if (f (7, 13, 17) != 70)
+ __builtin_abort ();
+ if (f (0, 13, 17) != 39)
+ __builtin_abort ();
+ return 0;
+}
+
+/* { dg-final { scan-rtl-dump-not "if-conversion succeeded through
noce_convert_multiple_sets" "ce1" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr126184.c
b/gcc/testsuite/gcc.target/aarch64/pr126184.c
new file mode 100644
index 00000000000..920d47401ab
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr126184.c
@@ -0,0 +1,31 @@
+/* PR rtl-optimization/126184 */
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-tree-ter -fno-tree-coalesce-vars -fdump-rtl-ce1" } */
+/* { dg-additional-options
"--param=max-rtl-if-conversion-unpredictable-cost=100" } */
+
+__attribute__ ((noipa)) unsigned long long
+f (unsigned long long c, unsigned long long x, unsigned long long y,
+ unsigned long long z)
+{
+ if (c != 0)
+ {
+ c = x + 1;
+ x = c * y;
+ c = z + 3;
+ y = c * x;
+ }
+
+ return x + y;
+}
+
+int
+main (void)
+{
+ if (f (1, 13, 17, 23) != 6426)
+ __builtin_abort ();
+ if (f (0, 13, 17, 23) != 30)
+ __builtin_abort ();
+ return 0;
+}
+
+/* { dg-final { scan-rtl-dump-times "if-conversion succeeded through
noce_convert_multiple_sets" 1 "ce1" } } */
--
2.50.1 (Apple Git-155)
