On Tue, Jan 30, 2018 at 08:59:43AM +0100, Richard Biener wrote:
> The issue is that
>
> static bool
> tree_unroll_loops_completely_1 (bool may_increase_size, bool unroll_outer,
> bitmap father_bbs, struct loop *loop)
> {
> struct loop *loop_father;
> bool changed = false;
> struct loop *inner;
> enum unroll_level ul;
>
> /* Process inner loops first. */
> for (inner = loop->inner; inner != NULL; inner = inner->next)
> changed |= tree_unroll_loops_completely_1 (may_increase_size,
> unroll_outer, father_bbs,
> inner);
>
> when recursing tree_unroll_loops_completely_1 appends unrolled
> bodies and loops contained therein in the loop->inner chain. We
> specifically do _not_ allow processing of outer loops here due
> to not up-to-date SSA form and then iterate the unrolling process
> instead:
>
> /* If we changed an inner loop we cannot process outer loops in this
> iteration because SSA form is not up-to-date. Continue with
> siblings of outer loops instead. */
> if (changed)
> return true;
>
> so I think the proper fix is to avoid visiting loops that were added
> in the above iteration over loop->inner. For example by collecting
> the loop->inner chain in a vec and then iterating over that (much
> like FOR_EACH_LOOP does). Or rely on the fact that loop->num of new
> loops will be >= number_of_loops () [number_of_loops () is really
> mis-named, would be a good time to add max_loop_num () or so]
So like this if it passes bootstrap/regtest?
OT, why is loop->num signed rather than unsigned?
2018-01-30 Richard Biener <rguent...@suse.de>
Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/84111
* tree-ssa-loop-ivcanon.c (tree_unroll_loops_completely_1): Skip
inner loops added during recursion, as they don't have up-to-date
SSA form.
* gcc.c-torture/compile/pr84111.c: New test.
--- gcc/tree-ssa-loop-ivcanon.c.jj 2018-01-29 14:05:46.689153783 +0100
+++ gcc/tree-ssa-loop-ivcanon.c 2018-01-30 09:30:21.932069737 +0100
@@ -1373,13 +1373,17 @@ tree_unroll_loops_completely_1 (bool may
bool changed = false;
struct loop *inner;
enum unroll_level ul;
+ unsigned num = number_of_loops (cfun);
- /* Process inner loops first. */
+ /* Process inner loops first. Don't walk loops added by the recursive
+ calls because SSA form is not up-to-date. They can be handled in the
+ next iteration. */
for (inner = loop->inner; inner != NULL; inner = inner->next)
- changed |= tree_unroll_loops_completely_1 (may_increase_size,
- unroll_outer, father_bbs,
- inner);
-
+ if ((unsigned) inner->num < num)
+ changed |= tree_unroll_loops_completely_1 (may_increase_size,
+ unroll_outer, father_bbs,
+ inner);
+
/* If we changed an inner loop we cannot process outer loops in this
iteration because SSA form is not up-to-date. Continue with
siblings of outer loops instead. */
--- gcc/testsuite/gcc.c-torture/compile/pr84111.c.jj 2018-01-29
20:38:10.236528914 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr84111.c 2018-01-29
20:37:53.227522763 +0100
@@ -0,0 +1,31 @@
+/* PR tree-optimization/84111 */
+
+void
+foo (int x, int y, int z)
+{
+ int a = 0;
+ int *b = &x;
+
+ while (a < 1)
+ {
+ int c = y;
+ *b = x;
+ lab:
+ for (a = 0; a < 36; ++a)
+ {
+ *b = 0;
+ if (x != 0)
+ y = 0;
+ while (c < 1)
+ ;
+ }
+ }
+ if (z < 33)
+ {
+ b = (int *) 0;
+ ++y;
+ ++z;
+ if (x / *b != 0)
+ goto lab;
+ }
+}
Jakub
