Additional peephole pass(es)

Additional peephole pass(es)

[Stefan Franke]

Hi there,

 

is there a chance that a patch would be accepted if it adds (an) additional
peephole pass(es)?

 

I'm not content with the capabilities of the combine pass and a convenient
way would be to insert an additional pass in front/after the combine pass.
It's way easier to maintain than the spaghetti code in combine and ss long
there is nothing defined in the cpu's md file, the pass gets skipped, so the
overhead for non-users is almost non existent.

 

Right now I'm applying the same set as in the final peephole run, but I
would add a separate keyword per pass, e.g. peephole_precombine, etc. p.p.

 

Your thoughts?

 

/cheers Stefan

 

Re: Additional peephole pass(es)

[Oleg Endo]

Hi,

On Mon, 2020-04-20 at 12:20 +0200, Stefan Franke wrote:
> 
> is there a chance that a patch would be accepted if it adds (an) additional
> peephole pass(es)?
>  
> 
> I'm not content with the capabilities of the combine pass and a convenient
> way would be to insert an additional pass in front/after the combine pass.
> It's way easier to maintain than the spaghetti code in combine and ss long
> there is nothing defined in the cpu's md file, the pass gets skipped, so the
> overhead for non-users is almost non existent.
>  
> 
> Right now I'm applying the same set as in the final peephole run, but I
> would add a separate keyword per pass, e.g. peephole_precombine, etc. p.p.
>  
> 
> Your thoughts?
> 

Yeah, it's true, combine doesn't catch every possible combination and
the canonical form is also sometimes questionable.  You can add
additional passes in the backend as you like.  If a particular
additional pass for a particular backend results in significant
improvements in code quality or such, then why not?

You can also do "manual combine" of certain insns in the split1 pass
that runs after combine.  For some examples you can browse through the
SH backend.

The problem with peepholes is that they tend to break easily because
they depend on insn order.  If something else in the compiler decides
to insert insns between two related insns, peephole patterns usually
break.

Cheers,
Oleg

Re: Additional peephole pass(es)

[Segher Boessenkool]

Hi!

On Tue, Apr 21, 2020 at 02:34:09AM +0900, Oleg Endo wrote:
> Yeah, it's true, combine doesn't catch every possible combination and
> the canonical form is also sometimes questionable.

Yeah.  Bug reports welcome!

This isn't "canonical" usually; but all targets depend more or less on
what insns combine used to generate.  Even for comparably tiny changes
(that are a net good thing for all targets!) people become very unhappy
already, so yeah it is some kinf of "de facto" canonical: but it is not
that we promised this is the form we would use, and it is not that we
think this is the best and we should not change it; the problem just is
inertia.

> You can add
> additional passes in the backend as you like.  If a particular
> additional pass for a particular backend results in significant
> improvements in code quality or such, then why not?
> 
> You can also do "manual combine" of certain insns in the split1 pass
> that runs after combine.  For some examples you can browse through the
> SH backend.

In combine itself runs split as well (one insn to two).  And you can
add instruction patterns to your machine description that are only there
for combine: you split them to separate insns immediately again (maybe
this is what you meant though?)

> The problem with peepholes is that they tend to break easily because
> they depend on insn order.  If something else in the compiler decides
> to insert insns between two related insns, peephole patterns usually
> break.

Another big problem is that it is very, very hard to write a *correct*
(non-trivial) peephole2.


Segher

Re: Additional peephole pass(es)

[Oleg Endo]

On Fri, 2020-04-24 at 16:36 -0500, Segher Boessenkool wrote:
> 
> In combine itself runs split as well (one insn to two).  And you can
> add instruction patterns to your machine description that are only there
> for combine: you split them to separate insns immediately again (maybe
> this is what you meant though?)

No, that's not what I meant.  What I meant was really custom "combine-
like" steps which combine would never do by itself.  split1 gives you a
simple "interface" to hook into each insn without writing a full custom
pass.

For example, on SH I've used that to implement additional elimination
of sign/zero extending memory loads.  On RX I did the same to fold some
in-memory operations, which neither combine nor peephole passes would
catch otherwise.

> Another big problem is that it is very, very hard to write a *correct*
> (non-trivial) peephole2.
> 

I agree.  Combine patterns are easier to deal with.

Cheers,
Oleg

Re: Additional peephole pass(es)

[Segher Boessenkool]

On Sat, Apr 25, 2020 at 01:04:45PM +0900, Oleg Endo wrote:
> On Fri, 2020-04-24 at 16:36 -0500, Segher Boessenkool wrote:
> > 
> > In combine itself runs split as well (one insn to two).  And you can
> > add instruction patterns to your machine description that are only there
> > for combine: you split them to separate insns immediately again (maybe
> > this is what you meant though?)
> 
> No, that's not what I meant.  What I meant was really custom "combine-
> like" steps which combine would never do by itself.  split1 gives you a
> simple "interface" to hook into each insn without writing a full custom
> pass.

Ah right, the T reg passes.

For GCC 11 I want to do 1-to-1 combinations as well, which helps for a
bunch of things, including it can perhaps make your sett/clrt pass
superfluous.  We'll see :-)

The treg_combine pass...  I don't see why when the problem is inside a
single basic block, why then combine cannot handle it?

But of course most of the time it is over BB edges.  Maybe fwprop or
Richard's proposed new "combine2" can handle this generically?

> For example, on SH I've used that to implement additional elimination
> of sign/zero extending memory loads.  On RX I did the same to fold some
> in-memory operations, which neither combine nor peephole passes would
> catch otherwise.

Yeah, we need some more infrastructure to start to tackle that
generically (and I'm not at all sure that that will help enough).


Segher

Re: Additional peephole pass(es)

[Segher Boessenkool]

Hi!

On Mon, Apr 20, 2020 at 12:20:35PM +0200, Stefan Franke wrote:
> is there a chance that a patch would be accepted if it adds (an) additional
> peephole pass(es)?

That would need some serious justification.

> I'm not content with the capabilities of the combine pass

Sorry to hear that.  Do you have any concrete complaints?

> and a convenient
> way would be to insert an additional pass in front/after the combine pass.
> It's way easier to maintain than the spaghetti code in combine and ss long

Spaghetti code?  Heh.  There is a lot of run-on code; there is a little
bit of action-at-a-distance; and almost all other sins imaginable are
committed somewhere as well, but spaghetti?  Not so much :-)

> there is nothing defined in the cpu's md file, the pass gets skipped, so the
> overhead for non-users is almost non existent.
> 
>  
> 
> Right now I'm applying the same set as in the final peephole run, but I
> would add a separate keyword per pass, e.g. peephole_precombine, etc. p.p.
> 
>  
> 
> Your thoughts?

It probably would help if you could start with an example that shows
that an extra peephole pass would help.


Segher