@@ -2202,10 +2202,6 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
}
}
- /* If insn to set up A clobbers any registers B depends on, try to
- swap insn that sets up A with the one that sets up B. If even
- that doesn't help, punt. */
-
modified_in_a = emit_a != NULL_RTX && modified_in_p (orig_b, emit_a);
if (tmp_b && then_bb)
{
@@ -2220,31 +2216,33 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
}
}
- if (emit_a || modified_in_a)
+
+ modified_in_b = emit_b != NULL_RTX && modified_in_p (orig_a, emit_b);
+ if (tmp_a && else_bb)
{
- modified_in_b = emit_b != NULL_RTX && modified_in_p (orig_a, emit_b);
- if (tmp_b && else_bb)
+ FOR_BB_INSNS (else_bb, tmp_insn)
+ /* Don't check inside insn_b. We will have changed it to emit_b
+ with a destination that doesn't conflict. */
+ if (!(insn_b && tmp_insn == insn_b)
+ && modified_in_p (orig_a, tmp_insn))
{
- FOR_BB_INSNS (else_bb, tmp_insn)
- /* Don't check inside insn_b. We will have changed it to emit_b
- with a destination that doesn't conflict. */
- if (!(insn_b && tmp_insn == insn_b)
- && modified_in_p (orig_a, tmp_insn))
- {
- modified_in_b = true;
- break;
- }
+ modified_in_b = true;
+ break;
}
- if (modified_in_b)
- goto end_seq_and_fail;
+ }
+ /* If insn to set up A clobbers any registers B depends on, try to
+ swap insn that sets up A with the one that sets up B. If even
+ that doesn't help, punt. */
+ if (modified_in_a && !modified_in_b)
+ {
if (!noce_emit_bb (emit_b, else_bb, b_simple))
goto end_seq_and_fail;
if (!noce_emit_bb (emit_a, then_bb, a_simple))
goto end_seq_and_fail;
}
- else
+ else if (!modified_in_a)
{
if (!noce_emit_bb (emit_a, then_bb, a_simple))
goto end_seq_and_fail;
@@ -2252,6 +2250,8 @@ noce_try_cmove_arith (struct noce_if_info *if_info)
if (!noce_emit_bb (emit_b, else_bb, b_simple))
goto end_seq_and_fail;
}
+ else
+ goto end_seq_and_fail;
target = noce_emit_cmove (if_info, x, code, XEXP (if_info->cond, 0),
XEXP (if_info->cond, 1), a, b);
new file mode 100644
@@ -0,0 +1,30 @@
+int b, c, d, e = 1, f, g, h, j;
+
+static int
+fn1 ()
+{
+ int a = c;
+ if (h)
+ return 9;
+ g = (c || b) % e;
+ if ((g || f) && b)
+ return 9;
+ e = d;
+ for (c = 0; c > -4; c--)
+ ;
+ if (d)
+ c--;
+ j = c;
+ return d;
+}
+
+int
+main ()
+{
+ fn1 ();
+
+ if (c != -4)
+ __builtin_abort ();
+
+ return 0;
+}
Hi all, In this fix I want to simplify the control flow of the code that chooses the order in which to emit the then and else basic blocks (and their associated emit_a and emit_b instructions). Currently we check the then block and only if there is a modification there we check the else block and make a decision there. IMO it's much simpler if we check both blocks and write the logic that chooses the order as a simple IF-ELSEIF-ELSE block that only emits the blocks and doesn't try to do any other checks. The bug in the logic that was preventing the clobber check from being performed in this PR was in the code: if (emit_a || modified_in_a) { modified_in_b = emit_b != NULL_RTX && modified_in_p (orig_a, emit_b); if (tmp_b && else_bb) { FOR_BB_INSNS (else_bb, tmp_insn) where the second if condition should have been: if (tmp_a && else_bb) Just changing the tmp_b to tmp_a in that condition would have fixed the wrong-code part of this PR as we would have ended up rejecting if-conversion. However, there is a valid if-conversion opportunity here, we just have to emit emit_a followed by else_bb, which the current control flow made awkward, which is why I'm suggesting this small rewrite. Bootstrapped and tested on x86_64, aarch64, arm. Ok for trunk? Thanks, Kyrill 2015-12-03 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR rtl-optimization/68624 * ifcvt.c (noce_try_cmove_arith): Check clobbers of temp regs in both blocks if they exist and simplify the logic choosing the order to emit them in. 2015-12-03 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR rtl-optimization/68624 * gcc.c-torture/execute/pr68624.c: New test.