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.
Details
- Reviewers
Ayal gilr rengolin - Commits
- rGda740492b02b: [VPlan] Remove dead header-phi recipes.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Ping :)
Rebased, remove all recipes from header, also added check for uses outside the loop not modeled yet in VPlan.
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp | ||
---|---|---|
303–310 | Would be good to further clarify, and possibly simplify? 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. | |
366 | reverse is needed in order to collect dead chains/DAGs, right? |
Add clarifying comment, only replace the last cast recipe in the chain with the original IV.
minor nits
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp | ||
---|---|---|
303–310 | Worth emphasizing "need to be" - unlike other VPlanTransforms/VPlan2VPlan optimizations, this optimization is mandatory. | |
366 | That may be worth pointing out. |
Updated comments: changed need to be to *must ..*, added sentence about reverse order.
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp | ||
---|---|---|
303–310 |
I updated the section to use *must ...* to emphasize it.
Will do independent of this patch. | |
366 | Thanks, I added a sentence. |
Looks good to me, thanks!
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp | ||
---|---|---|
303–310 | 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. |
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.