[PATCH] D118051: [VPlan] Remove dead header-phi recipes.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:304
     // Visit all casts connected to IV and in Casts. Collect them.
     // remember them for removal.
     auto &Casts = IV->getInductionDescriptor().getCastInsts();
----------------
Ayal wrote:
> 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.
> 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.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:367
+  // TODO: Remove dead recipes across whole plan.
+  for (VPRecipeBase &R : make_early_inc_range(reverse(*Header))) {
+    if (R.mayHaveSideEffects() ||
----------------
Ayal wrote:
> reverse is needed in order to collect dead chains/DAGs, right?
Yes, it allows us to catch chains of dead recipes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118051/new/

https://reviews.llvm.org/D118051