This is an archive of the discontinued LLVM Phabricator instance.

[LV] Add IRBuilder folder to simplify GEP x, 0.
Needs ReviewPublic

Authored by fhahn on Jun 11 2023, 3:20 PM.

Details

Summary

Add LVFolder to simplify GEP x, 0 -> x. Further simplifications will
follow in later patches. The goal of the folder is to remove some
obviously redundant instructions, to generate more compact IR, which in
turn should make the tests easier to read.

At the moment, new folders would need to implement the whole interface.
To avoid that, update ConstantFolder to allow sub-classes.

Note that using the more powerful InstSimplifyFolder does not work,
because it requires the input IR to be in a valid state.

There are a few remaining tests to update.

Diff Detail

Event Timeline

fhahn created this revision.Jun 11 2023, 3:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2023, 3:20 PM
fhahn requested review of this revision.Jun 11 2023, 3:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2023, 3:20 PM
fhahn added a comment.Jun 12 2023, 2:37 AM

Interestingly, this is also causing some binary changes, so is presumably catching same missed simplifications or enabling additional/earlier simplifications: https://llvm-compile-time-tracker.com/compare.php?from=2ba7b618c96d64e341366bf46d959404fe413468&to=e3753cc0f9c8404f21dd7486febfb4766f4c9659&stat=size-text

nikic added a subscriber: nikic.Jun 12 2023, 2:44 AM
nikic added inline comments.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
481

Any reason to base this on ConstantFolder rather than TargetFolder?

fhahn marked an inline comment as done.Jun 12 2023, 3:57 AM
fhahn added inline comments.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
481

No particular reason, it could definitely also be TargetFolder. In general, I think those sort of lightweight simplifications could be beneficial for most/all users of ConstantFolder/TargetFolder, so it might make sense to also use them there eventually?

Ayal added inline comments.Jun 15 2023, 7:17 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
9757

Is this (the only place) where the redundant gep-with-zero are generated?

Would be good to ensure recipes are (more) aware of the code they generate, and potentially account for it cost-wise, rather than folding this inside IRBuilder?

fhahn updated this revision to Diff 535782.Jun 29 2023, 7:18 AM
fhahn marked 2 inline comments as done.

Rebase, fix remaining tests failures and ping :)

fhahn added inline comments.Jun 29 2023, 7:21 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
481

Changed to TargetFolder.

9757

Redundant GEPs can also at least be generated in the if() part and in vectorizeInterleaveGroup. Agreed that it would be good to include this during cost estimates once we do it per recipe; but I *think* then we can just ignore such redundant GEPs during the cost calculation.

Ayal added a comment.Jun 30 2023, 3:00 PM

Trying to see if we can catch all redundant cases and reason about optimizing them explicitly during recipe execution (and potentially earlier, e.g., if every GEP would be represented by a recipe) rather than during Builder code-gen.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
2661

Idx is zero iff !reverse and Index == 0. Reverse sets Idx according to (negative) last lane which cannot be zero: -((VF-1)*|IG|+Index).

2684
9749

NumElt is zero iff Part is zero, being set to -Part * RunTimeVF.

9756–9757

Increment is zero iff Part is zero, being set to State.VF * Part.

VPlan code-gen should ideally be straightforward, potentially at the expense of more accurate modelling in VPlan recipes. Rather than delegating folding optimizations to the Builder at code-gen, trying to see if recipes could reason more accurately about the code they represent. Some optimizations relate specifically to the first unrolled part.