[PATCH] Fix PR36078, fallout from early loop unrolling

[Richard Guenther]

On Tue, 29 Apr 2008, Zdenek Dvorak wrote:

> Hi,
> 
> > On Tue, 29 Apr 2008, Zdenek Dvorak wrote:
> > 
> > > > This patch adds a missing SSA update after early complete loop unrolling.
> > > > Appearantly this is only needed if unrolling doesn't operate on
> > > > loop-closed SSA form, but it might also paper over a problem with not
> > > > doing so.
> > > 
> > > why does this help?  We already call update_ssa in
> > > try_unroll_loop_completely.
> > 
> > Hm, the difference with the (indeed extra) update_ssa is
> > ...
> > as we run CFG cleanup after updating SSA form in 
> > try_unroll_loop_completely, some constants are propagated from PHIs
> > and the VOPs change during CFG cleanup requiring an SSA update
> > (cfg-cleanup vs. non-cfg-cleanup variant):
> > ...
> >
> > This may be a latent general problem of course, which the patch
> > (bootstrapped and tested ok meanwhile) only papers over.
> 
> this is a known issue with cfg cleanup (see passes.c:904).  An easy fix
> is to call update_ssa after cfg cleanup (or possibly to add the update_ssa
> call to cfg cleanup itself).
> 
> A nicer fix would be to make cfg cleanup preserve ssa form for virtual
> ops.  In theory, this is easy -- the phi node elimination should only
> make the memory addresses more specific, thus it may only make some of
> the vops disappear, in which case we can just "copy propagate" them.  In
> practice, alias analysis somehow manages (or at least, used to manage)
> to introduce new vops, thus making this impossible.

Ok, so with the unrolling code that does

  do
    {
      changed = false;

      FOR_EACH_LOOP (li, loop, LI_ONLY_INNERMOST)
...
          changed |= canonicalize_loop_induction_variables...
...
      if (changed)
        {
          /* This will take care of removing completely unrolled loops
             from the loop structures so we can continue unrolling now
             innermost loops.  */
          cleanup_tree_cfg ();

          /* Clean up the information about numbers of iterations, since
             complete unrolling might have invalidated it.  */
          scev_reset ();
        }
    }
  while (changed);

that is, iterates until one iteration didn't result in more unrolling
(but cleans up the CFG before to enable new unrolling), the patch I
proposed is indeed reasonable.  Correct?

Thanks,
Richard.
k