diff mbox

Fix PR78725 (v2)

Message ID alpine.LSU.2.20.1612121624070.31313@wotan.suse.de
State New
Headers show

Commit Message

Michael Matz Dec. 12, 2016, 3:29 p.m. UTC
Hi,

On Fri, 9 Dec 2016, Richard Biener wrote:

> On December 9, 2016 4:29:04 PM GMT+01:00, Michael Matz <matz@suse.de> wrote:
> >Hi,
> >
> >if the induction variable on which we want to partition the loop 
> >iterations for loop splitting might overflow we would either need
> >runtime 
> >tests checking if an overflow in fact does happen, or we can simply not
> >
> >split loops on such ones.  I chose the latter.
> >
> >Fixes the testcase, and regstrapped without regressions on x86-64-linux
> >
> >(all languages+Ada).  Okay for trunk?
> 
> OK

Actually there came another report in the same bug, which has a different 
but related cause.  In the testcase the potential split-point is

  if (outer >= inner)

where the final value of 'inner' is known constant in the _outer_ loop.  
But of course we have to consider the actual place of the use: the 
comparison statement which is inside the inner loop, not merely in the 
outer loop.  Fixed with the amended patch.

Regstrapping on x86-64-linux in progress (all langs+ada), but I know that 
it fixes both testcases and doesn't regress in gcc.dg.  Okay for trunk if 
regstrapping succeeds?


Ciao,
Michael.
	PR tree-optimization/78725
	* tree-ssa-loop-split.c (split_at_bb_p): Check for overflow and
	at correct use point.

testsuite/
	PR tree-optimization/78725
	* gcc.dg/pr78725.c: New test.
	* gcc.dg/pr78725-2.c: New test.

Comments

Richard Biener Dec. 12, 2016, 4:06 p.m. UTC | #1
On December 12, 2016 4:29:24 PM GMT+01:00, Michael Matz <matz@suse.de> wrote:
>Hi,
>
>On Fri, 9 Dec 2016, Richard Biener wrote:
>
>> On December 9, 2016 4:29:04 PM GMT+01:00, Michael Matz <matz@suse.de>
>wrote:
>> >Hi,
>> >
>> >if the induction variable on which we want to partition the loop 
>> >iterations for loop splitting might overflow we would either need
>> >runtime 
>> >tests checking if an overflow in fact does happen, or we can simply
>not
>> >
>> >split loops on such ones.  I chose the latter.
>> >
>> >Fixes the testcase, and regstrapped without regressions on
>x86-64-linux
>> >
>> >(all languages+Ada).  Okay for trunk?
>> 
>> OK
>
>Actually there came another report in the same bug, which has a
>different 
>but related cause.  In the testcase the potential split-point is
>
>  if (outer >= inner)
>
>where the final value of 'inner' is known constant in the _outer_ loop.
> 
>But of course we have to consider the actual place of the use: the 
>comparison statement which is inside the inner loop, not merely in the 
>outer loop.  Fixed with the amended patch.
>
>Regstrapping on x86-64-linux in progress (all langs+ada), but I know
>that 
>it fixes both testcases and doesn't regress in gcc.dg.  Okay for trunk
>if 
>regstrapping succeeds?

OK.

Richard.

>
>Ciao,
>Michael.
>	PR tree-optimization/78725
>	* tree-ssa-loop-split.c (split_at_bb_p): Check for overflow and
>	at correct use point.
>
>testsuite/
>	PR tree-optimization/78725
>	* gcc.dg/pr78725.c: New test.
>	* gcc.dg/pr78725-2.c: New test.
>
>diff --git a/gcc/testsuite/gcc.dg/pr78725-2.c
>b/gcc/testsuite/gcc.dg/pr78725-2.c
>new file mode 100644
>index 0000000..9de489e
>--- /dev/null
>+++ b/gcc/testsuite/gcc.dg/pr78725-2.c
>@@ -0,0 +1,19 @@
>+/* { dg-do run } */
>+/* { dg-options "-O3 -fsplit-loops" } */
>+
>+int a, b, c;
>+
>+int main ()
>+{
>+  int d;
>+  for (; c < 1; c++)
>+    for (d = 0; d < 3; d++)
>+      for (b = 0; b < 1; b++)
>+	if (c >= d)
>+	  a = 1;
>+
>+  if (a != 1)
>+    __builtin_abort ();
>+
>+  return 0;
>+}
>diff --git a/gcc/testsuite/gcc.dg/pr78725.c
>b/gcc/testsuite/gcc.dg/pr78725.c
>new file mode 100644
>index 0000000..ed95790
>--- /dev/null
>+++ b/gcc/testsuite/gcc.dg/pr78725.c
>@@ -0,0 +1,19 @@
>+/* { dg-do run } */
>+/* { dg-options "-O3 -fsplit-loops" } */
>+
>+int fn1 (int b, int c)
>+{
>+    return c < 0 || c > 31 ? 0 : b >> c;
>+}
>+
>+unsigned char d = 255; 
>+int e, f;
>+
>+int main ()
>+{
>+  for (; f < 2; f++)
>+    e = fn1 (1, d++);
>+  if (e != 1)
>+    __builtin_abort ();
>+  return 0; 
>+}
>diff --git a/gcc/tree-ssa-loop-split.c b/gcc/tree-ssa-loop-split.c
>index dac68e6..84c0627 100644
>--- a/gcc/tree-ssa-loop-split.c
>+++ b/gcc/tree-ssa-loop-split.c
>@@ -102,10 +102,11 @@ split_at_bb_p (struct loop *loop, basic_block bb,
>tree *border, affine_iv *iv)
> 
>   tree op0 = gimple_cond_lhs (stmt);
>   tree op1 = gimple_cond_rhs (stmt);
>+  struct loop *useloop = loop_containing_stmt (stmt);
> 
>-  if (!simple_iv (loop, loop, op0, iv, false))
>+  if (!simple_iv (loop, useloop, op0, iv, false))
>     return NULL_TREE;
>-  if (!simple_iv (loop, loop, op1, &iv2, false))
>+  if (!simple_iv (loop, useloop, op1, &iv2, false))
>     return NULL_TREE;
> 
>   /* Make it so that the first argument of the condition is
>@@ -122,6 +123,8 @@ split_at_bb_p (struct loop *loop, basic_block bb,
>tree *border, affine_iv *iv)
>     return NULL_TREE;
>   if (!integer_zerop (iv2.step))
>     return NULL_TREE;
>+  if (!iv->no_overflow)
>+    return NULL_TREE;
> 
>   if (dump_file && (dump_flags & TDF_DETAILS))
>     {
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/pr78725-2.c b/gcc/testsuite/gcc.dg/pr78725-2.c
new file mode 100644
index 0000000..9de489e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr78725-2.c
@@ -0,0 +1,19 @@ 
+/* { dg-do run } */
+/* { dg-options "-O3 -fsplit-loops" } */
+
+int a, b, c;
+
+int main ()
+{
+  int d;
+  for (; c < 1; c++)
+    for (d = 0; d < 3; d++)
+      for (b = 0; b < 1; b++)
+	if (c >= d)
+	  a = 1;
+
+  if (a != 1)
+    __builtin_abort ();
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/pr78725.c b/gcc/testsuite/gcc.dg/pr78725.c
new file mode 100644
index 0000000..ed95790
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr78725.c
@@ -0,0 +1,19 @@ 
+/* { dg-do run } */
+/* { dg-options "-O3 -fsplit-loops" } */
+
+int fn1 (int b, int c)
+{
+    return c < 0 || c > 31 ? 0 : b >> c;
+}
+
+unsigned char d = 255; 
+int e, f;
+
+int main ()
+{
+  for (; f < 2; f++)
+    e = fn1 (1, d++);
+  if (e != 1)
+    __builtin_abort ();
+  return 0; 
+}
diff --git a/gcc/tree-ssa-loop-split.c b/gcc/tree-ssa-loop-split.c
index dac68e6..84c0627 100644
--- a/gcc/tree-ssa-loop-split.c
+++ b/gcc/tree-ssa-loop-split.c
@@ -102,10 +102,11 @@  split_at_bb_p (struct loop *loop, basic_block bb, tree *border, affine_iv *iv)
 
   tree op0 = gimple_cond_lhs (stmt);
   tree op1 = gimple_cond_rhs (stmt);
+  struct loop *useloop = loop_containing_stmt (stmt);
 
-  if (!simple_iv (loop, loop, op0, iv, false))
+  if (!simple_iv (loop, useloop, op0, iv, false))
     return NULL_TREE;
-  if (!simple_iv (loop, loop, op1, &iv2, false))
+  if (!simple_iv (loop, useloop, op1, &iv2, false))
     return NULL_TREE;
 
   /* Make it so that the first argument of the condition is
@@ -122,6 +123,8 @@  split_at_bb_p (struct loop *loop, basic_block bb, tree *border, affine_iv *iv)
     return NULL_TREE;
   if (!integer_zerop (iv2.step))
     return NULL_TREE;
+  if (!iv->no_overflow)
+    return NULL_TREE;
 
   if (dump_file && (dump_flags & TDF_DETAILS))
     {