Page MenuHomePhabricator

[AArch64][GlobalISel] Add a post-legalizer combiner + remove undef stores
ClosedPublic

Authored by paquette on Apr 24 2020, 8:42 PM.

Details

Summary

Add a post-legalizer combiner with a very simple combine. This supersedes D77250, which did equivalent work in the selector.

This can be done pre-legalization or post-legalization. Post-legalization is more likely to hit, since G_IMPLICIT_DEFs tend to appear during legalization. There's no reason to not do it pre-legalization though-- if it can be caught earlier, great.

(I also think that it might be worth reimplementing D78769 using a target-specific post-legalization combine too after thinking about it for a while.)

Diff Detail

Event Timeline

paquette created this revision.Apr 24 2020, 8:42 PM
aemerson added inline comments.
llvm/lib/Target/AArch64/AArch64PostLegalizerCombiner.cpp
94

@dsanders is this SelectionDAGFallbackAnalysis necessary for a combiner?

Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2020, 2:00 PM
dsanders added inline comments.Apr 28 2020, 3:19 PM
llvm/lib/Target/AArch64/AArch64PostLegalizerCombiner.cpp
94

Unfortunately yes but it should be removable when fallbacks are disabled. The only thing it's doing is marking StackProtector as preserved.

When I was adding -debugify-and-strip-all I found that it's not safe to let some analysis passes run more than once and IIRC StackProtector was one of them.

At the moment, I don't think this is really necessary for -O0 (unless you have some data to suggest otherwise). (I also think at some point we should audit the combines for necessity at -O0, since the PreLegalizerCombiner is matching all of them.)

I committed this on behalf of Jessica in 49a4f3f7d88f61a81279de3d4e1c734ab0363228 with the change of only enabling with optimizations enabled.

This revision was not accepted when it landed; it landed in state Needs Review.May 21 2020, 6:57 PM
This revision was automatically updated to reflect the committed changes.