From: Kyrylo Tkachov <[email protected]>
If-conversion emits its replacement sequence at the end of the test block.
Expanding a conditional move there can require a fresh comparison, which
writes a condition-code register. end_ifcvt_sequence already rejects a
sequence that would destroy the condition code tested by the branch, but
cc_in_cond only reports that register when the branch reads it directly.
A branch such as AArch64 CBZ tests a general register, so the guard was
inert and an unrelated live condition code could be destroyed.
For the testcase at -O2 on aarch64, late-combine sinks a cset into the
join block, which leaves the flags live across the branch:
bb2: cmp w3, 1 // sets cc
cset w4, ls
cbz w1, .L2 // does not touch cc
bb5: cinc w2, w2, ls // reads cc
ce2 then if-converted bb3 and bb4 and inserted "cmp w1, 0" ahead of the
branch, so cinc read the wrong flags:
cmp w3, 1
cset w4, ls
cmp w1, 0 // clobbers the live flags
csel w2, w2, w3, eq
cinc w2, w2, ls // reads cmp w1, 0
rtl.h documents that ports in the "lowered" form, which includes aarch64
before register allocation, may keep the flags live between instructions,
so if-conversion has to respect that. Reject a generated sequence that
writes a condition-code register while it is live on exit from the test
block. Condition codes are recognised by mode class, as in cc_in_cond,
which covers ports that do not define TARGET_FLAGS_REGNUM. DF liveness is
already up to date here and the same paths query it for pseudos.
noce_convert_multiple_sets validates its sequence itself rather than
through end_ifcvt_sequence, and reaches noce_emit_cmove in the same way,
so it gets the same check.
PR126747 is the same defect reached from a different direction. At -Os the
multiplication overflow idiom becomes one .MUL_OVERFLOW, so both arms read a
single cset, and late-combine folds it into the second one:
bb2: cmp xzr, x0, lsr 32 // sets cc
cset w3, ne
cbz w0, .L4 // does not touch cc
bb5: cinc w0, w0, ne // reads cc
ce2 if-converted bb3 the same way and "cmp w0, 0" landed ahead of the cinc, so
foo (1, 1) returned 1 instead of 0.
Bootstrapped and tested on aarch64-none-linux.
Ok for trunk?
Thanks,
Kyrill
gcc/ChangeLog:
PR rtl-optimization/126501
PR rtl-optimization/126747
* ifcvt.cc (noce_clobbers_live_cc_p): New function.
(end_ifcvt_sequence): Use it to reject sequences that clobber a
condition-code register that is live out of the test block.
(noce_convert_multiple_sets): Likewise.
gcc/testsuite/ChangeLog:
PR rtl-optimization/126501
PR rtl-optimization/126747
* gcc.c-torture/execute/pr126501.c: New test.
* gcc.c-torture/execute/pr126747.c: New test.
Signed-off-by: Kyrylo Tkachov <[email protected]>
---
gcc/ifcvt.cc | 37 +++++++++++
.../gcc.c-torture/execute/pr126501.c | 64 +++++++++++++++++++
.../gcc.c-torture/execute/pr126747.c | 26 ++++++++
3 files changed, 127 insertions(+)
create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr126501.c
create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr126747.c
diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index 5ea25f8fbe7..dbff3727706 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -1094,6 +1094,34 @@ cc_in_cond (rtx cond)
return NULL_RTX;
}
+/* Return true if SEQ writes a condition-code register that still holds a
+ value which is live on exit from TEST_BB.
+
+ An if-converted sequence is emitted immediately before the jump that ends
+ TEST_BB, and the conversion then removes that jump, so a condition code
+ that is live out of TEST_BB is one which a later block still reads.
+ Expanding a conditional move can emit a fresh comparison, and that would
+ destroy it. */
+
+static bool
+noce_clobbers_live_cc_p (basic_block test_bb, rtx_insn *seq)
+{
+ HARD_REG_SET written;
+ CLEAR_HARD_REG_SET (written);
+ for (rtx_insn *insn = seq; insn; insn = NEXT_INSN (insn))
+ note_stores (insn, record_hard_reg_sets, &written);
+
+ bitmap live_out = df_get_live_out (test_bb);
+ hard_reg_set_iterator hrsi;
+ unsigned int regno;
+ EXECUTE_IF_SET_IN_HARD_REG_SET (written, 0, regno, hrsi)
+ if (GET_MODE_CLASS (reg_raw_mode[regno]) == MODE_CC
+ && bitmap_bit_p (live_out, regno))
+ return true;
+
+ return false;
+}
+
/* Return sequence of instructions generated by if conversion. This
function calls end_sequence() to end the current stream, ensures
that the instructions are unshared, recognizable non-jump insns.
@@ -1128,6 +1156,10 @@ end_ifcvt_sequence (struct noce_if_info *if_info)
|| (cc && set_of (cc, insn)))
return NULL;
+ /* CC above only covers the condition code read by the converted branch. */
+ if (noce_clobbers_live_cc_p (if_info->test_bb, seq))
+ return NULL;
+
return seq;
}
@@ -4011,6 +4043,11 @@ noce_convert_multiple_sets (struct noce_if_info *if_info)
|| recog_memoized (insn) == -1)
return false;
+ /* This path does not go through end_ifcvt_sequence, so apply the same rule
+ about condition codes that outlive the if-region. */
+ if (noce_clobbers_live_cc_p (test_bb, seq))
+ return false;
+
emit_insn_before_setloc (seq, if_info->jump,
INSN_LOCATION (insn_info.last ()->unmodified_insn));
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr126501.c
b/gcc/testsuite/gcc.c-torture/execute/pr126501.c
new file mode 100644
index 00000000000..c86f195ba2b
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr126501.c
@@ -0,0 +1,64 @@
+/* PR rtl-optimization/126501 */
+/* If-conversion emitted a fresh comparison at the end of the test block
+ while a condition-code value set earlier in that block was still live
+ in the join block. */
+
+struct F { unsigned int f0 : 2, f1 : 2, f2 : 2; };
+struct G { unsigned int g : 2; };
+struct S { struct F f; struct G g[6]; };
+
+__attribute__((noipa)) int
+f (struct S *p, int x)
+{
+ p->g[1].g = (1 >= p->f.f1);
+ p->g[2].g = x ? p->f.f0 : p->f.f2;
+ p->g[0].g = p->g[1].g + p->g[2].g;
+ return p->g[0].g;
+}
+
+__attribute__((noipa)) int
+ref (struct S *p, int x)
+{
+ volatile int f0 = p->f.f0, f1 = p->f.f1, f2 = p->f.f2;
+ volatile int a = (1 >= f1);
+ volatile int b = (x ? f0 : f2);
+
+ p->g[1].g = a;
+ p->g[2].g = b;
+ p->g[0].g = (int) p->g[1].g + (int) p->g[2].g;
+ return p->g[0].g;
+}
+
+__attribute__((noipa)) int
+opaque (int v)
+{
+ return v;
+}
+
+int
+main (void)
+{
+ struct S s;
+ int i, j, k, xi;
+
+ for (i = 0; i < 4; i++)
+ for (j = 0; j < 4; j++)
+ for (k = 0; k < 4; k++)
+ for (xi = 0; xi < 2; xi++)
+ {
+ int x = opaque (xi);
+ int got, want;
+
+ __builtin_memset (&s, 0, sizeof s);
+ s.f.f0 = i; s.f.f1 = j; s.f.f2 = k;
+ got = f (&s, x);
+
+ __builtin_memset (&s, 0, sizeof s);
+ s.f.f0 = i; s.f.f1 = j; s.f.f2 = k;
+ want = ref (&s, x);
+
+ if (got != want)
+ __builtin_abort ();
+ }
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr126747.c
b/gcc/testsuite/gcc.c-torture/execute/pr126747.c
new file mode 100644
index 00000000000..f9e59701915
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr126747.c
@@ -0,0 +1,26 @@
+/* PR rtl-optimization/126747 */
+/* Both overflow tests read one condition-code value that late-combine left
+ live across the branches. If-conversion emitted a fresh comparison at the
+ end of the test block and destroyed it. */
+
+volatile int c[2];
+
+__attribute__((noipa)) int
+foo (unsigned x, unsigned y)
+{
+ unsigned r = x * y;
+ int t = 0;
+ if (c[0]) { int u = 0; if (x != 0) u = (r / x != y); t += u; }
+ if (c[1]) { int u = 0; if (x != 0) u = (r / x != y); t += u; }
+ return t;
+}
+
+int
+main (void)
+{
+ c[0] = 1;
+ c[1] = 1;
+ if (foo (1, 1) != 0)
+ __builtin_abort ();
+ return 0;
+}
--
2.50.1 (Apple Git-155)
