Re: [RFC]: Remove Mem/address type assumption in combiner

[Hans-Peter Nilsson <hp at bitrange dot com>]

On Sat, 16 May 2015, Segher Boessenkool wrote:
> On Sat, May 16, 2015 at 12:36:38PM -0400, Hans-Peter Nilsson wrote:
> > On Sat, 16 May 2015, Segher Boessenkool wrote:
> > > On Fri, May 15, 2015 at 10:40:48PM -0400, Hans-Peter Nilsson wrote:
> > > > I confess the test-case-"guarded" addi pattern should have been
> > > > expressed with a shift in addition to the multiplication.
> > >
> > > But they wouldn't ever match so they might very well have bitrotted
> > > by now :-(
> >
> > It seems you're saying that the canonicalization to "ashift"
> > didn't work *at all*, when starting with an expression from an
> > address?  I knew it failed in part, but always thought it was
> > just a partial failure.
>
> With a plus or minus combine would always write it as a mult.
> I don't think any other pass would create this combination.  I
> haven't tested it though.

Ports probably also generate that internally at various RTL
passes, something that takes a bit more than an at-a-glance code
inspection.

> > > > ("In
> > > > addition to" as the canonically wrong one used to be the
> > > > combine-matching pattern; I'm not sure I should really drop that
> > > > just yet.)
> > >
> > > It is harmless to leave it in.  It will rot though, eventually --
> > > better take it out before then.  Add some gcc_unreachable, perhaps.
> >
> > I've been trying to come up with valid reasons there'd be ever
> > be canonicalization by multiplication, but failed so I guess
> > I'll rip it out.
>
> The "unreachable" thing should quickly tell you if that guess is wrong.
> Not something you want to leave in a production compiler, of course.

I think you misunderstood; I mean that I pondered
"philosophical" reasons to change the canonical representation;
if there was some reasoning, current or looking forward, where
we'd "add back" mult as non-address-canonical RTL.  For the
current scheme, keeping the matching-patterns but with
assertions added might be helpful, yes.

(People looking for practical clues: for spotting dead target
code there are besides the alluded-to gcc_unreachable(), also
the options to call internal_error() with a friendly string or
generating invalid assembly with a descriptive mnemonic.)

Actually, there are two things you could help with:

- (pointer-to) step-by-step instructions to recreate the Linux
(kernel :) build, as those you did before for a multiple of
targets.  I'd like to know I fixed your observations.

- add a preferred canonicalization function to do conversion
to/from memory-address-canonical RTL.  Like
fwprop.c:canonicalize_address (just not static :) and maybe also
a canonicalize_nonaddress.  At the moment, ports call
replace_equiv_address (family of functions) when changing
address RTL, but that code-path (at a glance) doesn't
canonicalize whatever non-address-canonical RTL you throw at it.
Maybe it should?

> > > Looks like quite some work for you, I'm sorry about that,
> >
> > It's almost over, at least the editing part...
>
> Great to hear that!

Thanks for your enthusiasm!

Well, I've known about this wart for 6+ years as seen in PR37939
(but before that likely at least as long), so I *should* thank
you for fixing it; it's just that the added churn and required
tweaks has somewhat of a sour taste. :)

brgds, H-P