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;
+}
+

Reply via email to