On Wed, May 09, 2018 at 09:33:30AM +0200, Eric Botcazou wrote:
> > Now, neither of the two branches needs to have LR restored at all,
> > because both of the branches end up in an infinite loop.
> >
> > This patch makes spread_component return a boolean saying if anything
> > was changed, and if so, it is called again. This obviously is finite
> > (there is a finite number of basic blocks, each with a finite number
> > of components, and spread_components can only assign more components
> > to a block, never less). I also instrumented the code, and on a
> > bootstrap+regtest spread_components made changes a maximum of two
> > times. Interestingly though it made changes on two iterations in
> > a third of the cases it did anything at all!
>
> I don't know the code much so I don't see why this solves the problem.
When I wrote the code I thought it would reach the fixpoint after just
one iteration. This is not true though, not e.g. in the motivating
example with no-return paths through the function.
It didn't generate wrong code btw, just pretty silly^W^Wnot terribly
good code.
> > 2018-05-08 Segher Boessenkool <seg...@kernel.crashing.org>
> >
> > PR rtl-optimization/85645
> > * shrink-wrap.c (spread_components): Return a boolean saying if
> > anything was changed.
> > (try_shrink_wrapping_separate): Iterate spread_components until
> > nothing changes anymore.
>
> OK if you add a comment in try_shrink_wrapping_separate with the rationale.
I made it say this:
/* Try to minimize the number of saves and restores. Do this as long as
it changes anything. This does not iterate more than a few times. */
int spread_times = 0;
while (spread_components (components))
{
spread_times++;
if (dump_file)
fprintf (dump_file, "Now spread %d times.\n", spread_times);
}
Thanks for the quick reviews,
Segher
