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.