Re: [PATCH] rtl-optimization/101523 - avoid re-combine after noop 2->2 combination

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Wed, Apr 03, 2024 at 01:07:41PM +0200, Richard Biener wrote:
> The following avoids re-walking and re-combining the instructions
> between i2 and i3 when the pattern of i2 doesn't change.
> 
> Bootstrap and regtest running ontop of a reversal of 
> r14-9692-g839bc42772ba7a.

Please include that in the patch (or series, preferably).

> It brings down memory use frmo 9GB to 400MB and compile-time from
> 80s to 3.5s.  r14-9692-g839bc42772ba7a does better in both metrics
> but has shown code generation regressions across acrchitectures.
> 
> OK to revert r14-9692-g839bc42772ba7a?

No.

The patch solved a very real problem.  How does your replacement handle
that?  You don't say.  It looks like it only battles symptoms a bit,
instead :-(

We had this before: 3->2 combinations that leave an instruction
identical to what was there before.  This was just a combination with
context as well.  The only reason this wasn't a huge problem then
already was because this is a 3->2 combination, even if it really is a
2->1 one it still is beneficial in all the same cases.  But in the new
case it can iterate indefinitely -- well not quite, but some polynomial
number of times, for a polynomial at least of degree three, possibly
more :-(

With this patch you need to show combine still is linear.  I don't think
it is, but some deeper analysis might show it still is.

  ~ - ~ - ~

What should *really* be done is something that has been on the wish list
for decades: an uncse pass.

The things that combine no longer works on after my patch are actually
1->1 combinations (which we never do currently, although we probably
should); or alternatively, an un-CSE followed by a 2->1 combination.

We can do the latter of course, but we need to do an actual uncse first!
Somewhere before combine, and then redo a CSE after it.  An actual CSE,
not doing ten gazillion other things.


Segher