Re: [PATCH/RFC] combine_completed global variable.

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

Hi!

On Thu, Jul 07, 2022 at 08:40:38PM +0100, Roger Sayle wrote:
> Although I've not been able to reproduce your ICE (using gcc135
> on the compile farm), I completely agree with Segher's analysis
> that the Achilles heel with my approach/patch is that there's
> currently no way for the backend/recog to know that we're in a
> pass after combine.

To know combine has been run at least once already, yes.  It currently
won't run more than one of course, but the only thing that really stops
running it more than once was historically that combine was one of the
more expensive passes.  Now otoh it is really quite cheap.

Currently combine is one of the last passes before RA.  It likely will
be nice to run combine quite early as well (or a limited version of it
that is), just like other optimisation passes already are, including its
closest relative fwprop.

> Rather than give up on this optimization (and a similar one for
> I386.md where test;sete can be replaced by xor $1 when combine
> knows that nonzero_bits is 1, but loses that information afterwards),
> I thought I'd post this "strawman" proposal to add a combine_completed
> global variable, matching the reload_completed and regstack_completed
> global variables already used (to track progress) by the middle-end.

It should be called differently, given the above.

> I was wondering if I could ask you could test the attached patch
> in combination with my previous rs6000.md patch (with the obvious
> change of reload_completed to combine_completed) to confirm
> that it fixes the problems you were seeing.
> 
> Segher/Richard, would this sort of patch be considered acceptable?
> Or is there a better approach/solution?

There is the thing I have been working on for about a year now, that
makes combine's nonzero_bits superfluous, by making the generic version
more capable than combine's version.  That is a better way forward imo,
it solves many other problems as well.  But it isn't ready yet.

Your patch is fine with a name change (and documentation change)
indicating it means that combine has run at least once.

Thanks!


Segher