[PATCH PR68021]Set ratio to 1 when computing the value of biv cand by itself

[PATCH PR68021]Set ratio to 1 when computing the value of biv cand by itself

[Bin Cheng]

[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]

Hi,
This is another way to fix PR68021, and I think it's the least intrusive way.  The issue is triggered in a special case in which cand is a original biv, and use denotes the value of the biv itself.  In this case, the use is added specifically for the original biv, as a result, get_computation_aff isn't called for the <cand, use> pair before rewriting the use.  It is possible that constant_multiple_of/operand_equal_q could fail because of inconsistent fold behavior.  The fold behavior is fully described in PR68021.
This patch fixes IVOPT part of issue by setting ratio to 1, because it is known that the use has the value of the biv cand.

Bootstrap and test on x86_64 and aarch64.  Is it OK if no failures?

Thanks,
bin

2016-02-09  Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/68021
	* tree-ssa-loop-ivopts.c (get_computation_aff): Set ratio to 1 if
	when computing the value of biv cand by itself.

gcc/testsuite/ChangeLog
2016-02-09  Bin Cheng  <bin.cheng@arm.com>

	PR tree-optimization/68021
	* gcc.dg/tree-ssa/pr68021.c: New test.

[-- Attachment #2: pr68021-20160209.txt --]
[-- Type: text/plain, Size: 1287 bytes --]

diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 4026d28..48facec 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -3741,7 +3741,15 @@ get_computation_aff (struct loop *loop,
       var = fold_convert (uutype, var);
     }
 
-  if (!constant_multiple_of (ustep, cstep, &rat))
+  /* Ratio is 1 when computing the value of biv cand by itself.  */
+  if (cand->pos == IP_ORIGINAL && cand->incremented_at == use->stmt)
+    {
+      gcc_assert (is_gimple_assign (use->stmt));
+      gcc_assert (use->iv->ssa_name == cand->var_after);
+      gcc_assert (gimple_assign_lhs (use->stmt) == cand->var_after);
+      rat = 1;
+    }
+  else if (!constant_multiple_of (ustep, cstep, &rat))
     return false;
 
   /* In case both UBASE and CBASE are shortened to UUTYPE from some common
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr68021.c b/gcc/testsuite/gcc.dg/tree-ssa/pr68021.c
new file mode 100644
index 0000000..f60b1ff
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr68021.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+char a;
+void fn1 (char *p1, int p2, int p3)
+{
+  int i, x;
+  for (i = 0; i < 10; i++)
+    {
+      for (x = 0; x < p3; x++)
+	{
+	  *p1 = a;
+	  p1--;
+	}
+      p1 += p2;
+    }
+}

Re: [PATCH PR68021]Set ratio to 1 when computing the value of biv cand by itself

[Richard Biener]

On Wed, Feb 10, 2016 at 12:34 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
> Hi,
> This is another way to fix PR68021, and I think it's the least intrusive way.  The issue is triggered in a special case in which cand is a original biv, and use denotes the value of the biv itself.  In this case, the use is added specifically for the original biv, as a result, get_computation_aff isn't called for the <cand, use> pair before rewriting the use.  It is possible that constant_multiple_of/operand_equal_q could fail because of inconsistent fold behavior.  The fold behavior is fully described in PR68021.
> This patch fixes IVOPT part of issue by setting ratio to 1, because it is known that the use has the value of the biv cand.
>
> Bootstrap and test on x86_64 and aarch64.  Is it OK if no failures?

Ok, but please add a comment why we have this special-casing instead
of relying on constant_multiple_of.

Thanks,
Richard.

> Thanks,
> bin
>
> 2016-02-09  Bin Cheng  <bin.cheng@arm.com>
>
>         PR tree-optimization/68021
>         * tree-ssa-loop-ivopts.c (get_computation_aff): Set ratio to 1 if
>         when computing the value of biv cand by itself.
>
> gcc/testsuite/ChangeLog
> 2016-02-09  Bin Cheng  <bin.cheng@arm.com>
>
>         PR tree-optimization/68021
>         * gcc.dg/tree-ssa/pr68021.c: New test.

Re: [PATCH PR68021]Set ratio to 1 when computing the value of biv cand by itself

[Bin.Cheng]

On Wed, Feb 10, 2016 at 1:27 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Wed, Feb 10, 2016 at 12:34 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
> > Hi,
> > This is another way to fix PR68021, and I think it's the least intrusive way.  The issue is triggered in a special case in which cand is a original biv, and use denotes the value of the biv itself.  In this case, the use is added specifically for the original biv, as a result, get_computation_aff isn't called for the <cand, use> pair before rewriting the use.  It is possible that constant_multiple_of/operand_equal_q could fail because of inconsistent fold behavior.  The fold behavior is fully described in PR68021.
> > This patch fixes IVOPT part of issue by setting ratio to 1, because it is known that the use has the value of the biv cand.
> >
> > Bootstrap and test on x86_64 and aarch64.  Is it OK if no failures?
>
> Ok, but please add a comment why we have this special-casing instead
> of relying on constant_multiple_of.
Done, patch applied at revision 233269.

Thanks,
bin
>
>
> Thanks,
> Richard.
>
> > Thanks,
> > bin
> >
> > 2016-02-09  Bin Cheng  <bin.cheng@arm.com>
> >
> >         PR tree-optimization/68021
> >         * tree-ssa-loop-ivopts.c (get_computation_aff): Set ratio to 1 if
> >         when computing the value of biv cand by itself.
> >
> > gcc/testsuite/ChangeLog
> > 2016-02-09  Bin Cheng  <bin.cheng@arm.com>
> >
> >         PR tree-optimization/68021
> >         * gcc.dg/tree-ssa/pr68021.c: New test.