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

[Florian Hahn via Phabricator via llvm-commits]

fhahn abandoned this revision.
fhahn marked 3 inline comments as done.
fhahn added a comment.

This is no longer needed after aa2414729ebb <https://reviews.llvm.org/rGaa2414729ebbcb2d8f162e9002a3a6aa768b1f9d>



================
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));
     }
----------------
Ayal wrote:
> 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".
> nit: Recipe.getParent() >> VPBB (independent of this patch).


Done in 435e220ba6a41afbeaef6019febddc1b89bf4214


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

Sounds like a good follow up!


================
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>
----------------
Ayal wrote:
> 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()?
Ah right! I had another look and the issue was that operands of instructions that do not need sinking aren't added to the worklist. Should be fixed by aa2414729ebb


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139790