| Message ID | HE1PR08MB0507FEFCF4FF37E4E98CCDBFE7D70@HE1PR08MB0507.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
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.
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.
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; + } +}