This is an archive of the discontinued LLVM Phabricator instance.

[VPlan] Remove dead header-phi recipes.
ClosedPublic

Authored by fhahn on Jan 24 2022, 9:35 AM.

Details

Summary

This patch adds a new transform to remove dead recipes. For now, it only
removes dead header-phi recipes, to keep the number tests that require
updating manageable. Future patches will extend this to remove dead
recipes across the whole plan.

Diff Detail

Event Timeline

fhahn created this revision.Jan 24 2022, 9:35 AM
fhahn requested review of this revision.Jan 24 2022, 9:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2022, 9:35 AM
Herald added a subscriber: vkmr. · View Herald Transcript
fhahn updated this revision to Diff 408244.Feb 13 2022, 2:52 AM

Ping :)

Rebased, remove all recipes from header, also added check for uses outside the loop not modeled yet in VPlan.

Ayal added inline comments.Feb 19 2022, 11:26 PM
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
304

Would be good to further clarify, and possibly simplify?
A sequence of IR Casts has potentially been recorded for each IV descriptor, which need to be bypassed when the IV is vectorized, because the vectorized IV will produce the desired casted value. This sequence forms a def-use chain and is provided in reverse order, ending with the cast that uses the IV phi. This method searches for the recipe of each such cast and eliminates it.

This optimization is mandatory, right? Retaining these cast recipes may result in redundant illegal ext/trunc of same type, as opposed to redundant legal ands/shifts?

Note that "Only the last instruction in the cast sequence is expected to have uses outside the induction def-use chain" (getCastsForInductionPHI()), so it would suffice to RAUW only the last (first) cast on the list and erase all others. But here we disconnect all casts from each other and hook each one to IV phi directly, to simplify removing them later as useless recipes. But removeDeadRecipes() below should be able to collect useless chains of recipes?

Perhaps "RecipeCast" would be a better name than "User"; it is the recipe of an IRCast we're looking for, User is how we find it.

379

reverse is needed in order to collect dead chains/DAGs, right?

fhahn updated this revision to Diff 410143.Feb 20 2022, 5:41 AM

Add clarifying comment, only replace the last cast recipe in the chain with the original IV.

fhahn updated this revision to Diff 410144.Feb 20 2022, 5:43 AM
fhahn marked an inline comment as done.

Fix formatting.

fhahn marked an inline comment as done.Feb 20 2022, 5:44 AM
fhahn added inline comments.
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
304

Would be good to further clarify, and possibly simplify?

Great point! I update the code to just replace the final cast value in the chain and added a clarifying comment.

379

Yes, it allows us to catch chains of dead recipes.

Ayal added a comment.Feb 20 2022, 2:28 PM

minor nits

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
304

Worth emphasizing "need to be" - unlike other VPlanTransforms/VPlan2VPlan optimizations, this optimization is mandatory.
Perhaps better call it "removeObsoleteInductionCasts()"? (comment is indep. of this patch)

379

That may be worth pointing out.

fhahn updated this revision to Diff 410257.Feb 21 2022, 2:31 AM
fhahn marked an inline comment as done.

Updated comments: changed need to be to *must ..*, added sentence about reverse order.

fhahn marked 2 inline comments as done.Feb 21 2022, 2:33 AM
fhahn added inline comments.
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
304

Worth emphasizing "need to be" - unlike other VPlanTransforms/VPlan2VPlan optimizations, this optimization is mandatory.

I updated the section to use *must ...* to emphasize it.

Perhaps better call it "removeObsoleteInductionCasts()"? (comment is indep. of this patch)

Will do independent of this patch.

379

Thanks, I added a sentence.

Ayal accepted this revision.Feb 21 2022, 9:49 AM

Looks good to me, thanks!

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
304

Continued thoughts: this can be thought of as two separate optimizations - folding adjacent casts into one, and folding a cast into an IV. Can potentially be recognized by analyzing VPlan rather than the underlying IR, if/when done so will turn into a non-mandatory optimization.

This revision is now accepted and ready to land.Feb 21 2022, 9:49 AM
This revision was automatically updated to reflect the committed changes.
fhahn marked 2 inline comments as done.