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