Hi all, This patch fixes the PRs in the ChangeLog that have been reported against my if-conversion patch. The problem occurs when the 'then' block is complex but the else block is empty. In this case the calling code in noce_process_if_block takes the 'else' move (x := b) from the test block. However, we have not checked whether the test block is valid for complex-block if-conversion with bb_valid_for_noce_process_p. Also, that's a case I wasn't particularly targeting when writing the initial patch.
This patch bails out of noce_try_cmove_arith when one of the blocks is complex and the other is empty. I've checked that if-conversion still happens in the cases of interest from the original patch. I've added the testcase from PR 67465 since that one uses __builtin_abort and triggers the problem nicely. The others show the miscompilation using printf seems to go away if I replace it with an abort. I have confirmed manually that the miscompilation goes away on those testcases. PR rtl-optimization/67481 is a testsuite regression on sparc-solaris that Rainer reported. I haven't tested that this patch fixes that, but I suspect that the root cause is the same. Rainer, could you please check that this fixes the regression for you? Bootstrapped and tested on aarch64 and x86_64. Ok for trunk if sparc testing comes ok? Thanks, Kyrill 2015-09-07 Kyrylo Tkachov <kyrylo.tkac...@arm.com> PR rtl-optimization/67456 PR rtl-optimization/67464 PR rtl-optimization/67465 PR rtl-optimization/67481 * ifcvt.c (noce_try_cmove_arith): Bail out if one of the blocks is complex and the other is empty. 2015-09-07 Kyrylo Tkachov <kyrylo.tkac...@arm.com> * gcc.dg/pr67465.c: New test.
commit 2305f7deed793315f04221f718880676cd62474d Author: Kyrylo Tkachov <kyrylo.tkac...@arm.com> Date: Mon Sep 7 14:58:01 2015 +0100 [RTL-ifcvt] PR rtl-optimization/67465: Do not ifcvt complex blocks if the else block is empty diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index d2f5b66..5716dcc 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -2079,7 +2079,14 @@ noce_try_cmove_arith (struct noce_if_info *if_info) } } - if (!a_simple && then_bb && !b_simple && else_bb + /* When one of the blocks is really empty and the other is a complex block + don't do anything. The value 'b' may have come from the test block that + we did not check for if-conversion validity in noce_process_if_block. */ + if ((!a_simple && !else_bb) + || (!b_simple && !then_bb)) + return FALSE; + + if (then_bb && else_bb && (!bbs_ok_for_cmove_arith (then_bb, else_bb) || !bbs_ok_for_cmove_arith (else_bb, then_bb))) return FALSE; diff --git a/gcc/testsuite/gcc.dg/pr67465.c b/gcc/testsuite/gcc.dg/pr67465.c new file mode 100644 index 0000000..321fd38 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr67465.c @@ -0,0 +1,53 @@ +/* { dg-do run } */ +/* { dg-options "-O3 -std=gnu99" } */ + +int a, b, c, d, e, h; + +int +fn1 (int p1) +{ + { + int g[2]; + for (int i = 0; i < 1; i++) + g[i] = 0; + if (g[0] < c) + { + a = (unsigned) (1 ^ p1) % 2; + return 0; + } + } + return 0; +} + +void +fn2 () +{ + for (h = 0; h < 1; h++) + { + for (int j = 0; j < 2; j++) + { + for (b = 1; b; b = 0) + a = 1; + for (; b < 1; b++) + ; + if (e) + continue; + a = 2; + } + fn1 (h); + short k = -16; + d = k > a; + } +} + +int +main () +{ + fn2 (); + + if (a != 2) + __builtin_abort (); + + return 0; +} +