[PATCH] D139790: [VPlan] Consider all recipes in replicate blocks as sink candidates.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:125
         if (auto *Def = Op->getDefiningRecipe())
-          WorkList.insert(std::make_pair(RepR->getParent(), Def));
+          WorkList.insert(std::make_pair(Recipe.getParent(), Def));
     }
----------------
nit: Recipe.getParent() >> VPBB (independent of this patch).

Perhaps a Replicating Region, which represents an "if-then" structured-control-flow hammock, should provide an interface for retrieving its (then) "replicating (basic) block".


================
Comment at: llvm/test/Transforms/LoopVectorize/vplan-sink-scalars-and-merge.ll:38
+; CHECK-NEXT:     vp<[[STEPS:%.+]]> = SCALAR-STEPS vp<[[CAN_IV]]>, ir<1>
 ; CHECK-NEXT:     REPLICATE ir<%gep.b> = getelementptr ir<@b>, ir<0>, vp<[[STEPS]]>
 ; CHECK-NEXT:     REPLICATE ir<%lv.b> = load ir<%gep.b>
----------------
fhahn wrote:
> Ayal wrote:
> > The iterative scan up use-def chains stops at GEPs?
> At the moment only replicate & scalar-steps recipe can be sunk.
Right, but curious why the use-def scan failed to sink vp<[[STEPS]]> used by sunk REPLICATEs of GEPS while scanning all recipes in replicate block succeeded. Is this due to the now iterative nature of applying sinkScalarOperands()?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139790