Re: Unloop no longer looping loops in loop-ch

[Richard Biener <rguenther@suse.de>]

On Wed, 26 Apr 2023, Jan Hubicka wrote:

> > > -      if (precise)
> > > +      if (precise
> > > +	  && get_max_loop_iterations_int (loop) == 1)
> > > +	{
> > > +	  if (dump_file && (dump_flags & TDF_DETAILS))
> > > +	    fprintf (dump_file, "Loop %d no longer loops.\n", loop->num);
> > 
> > but max loop iterations is 1 ...?
> 
> I first check for loops with 0 iterations, push them to unlooping list
> and avoid any header copying (it is useless).
> At this patch we already did header duplication and verified that the
> maximal number of iterations will drop by 1 since there is no way loop
> can terminate except for the header tests we peeled out.
> 
> So 1 would turn to 0 in the loop info update and it seems useless to do
> it.
> > 
> > > +	  loops_to_unloop.safe_push (loop);
> > > +	  loops_to_unloop_nunroll.safe_push (0);
> > > +	}
> > > +      else if (precise)
> > >  	{
> > >  	  if (dump_file && (dump_flags & TDF_DETAILS))
> > >  	    fprintf (dump_file,
> > > @@ -688,6 +699,12 @@ ch_base::copy_headers (function *fun)
> > >  	  BITMAP_FREE (exit_bbs);
> > >  	}
> > >      }
> > > +  if (loops_to_unloop.length())
> > 
> >   !loops_to_unloop.is_empty ()
> I updated that in my copy of the patch.
> > 
> > > +    {
> > > +      bool irred_invalidated;
> > > +      unloop_loops (loops_to_unloop, loops_to_unloop_nunroll, NULL, &irred_invalidated);
> > > +      changed = true;
> > > +    }
> > >    free (bbs);
> > >    free (copied_bbs);
> > 
> > 
> > Since we run VN on the header copies I wonder if, since you remove
> > edges, we need to run CFG cleanup before this and updating SSA form?
> > For safety we usually let CFG cleanup do the actual CFG manipulation
> > and just change cond jumps to if (0) or if (1)?
> 
> I do unlooping only after the VN so I think I am safe here.

Ah OK.

The patch is OK then.

Thanks,
Richard.