[PATCH] D133760: [VPlan] Support sinking VPScalarIVStepsRecipe.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9577
+    EndPart = StartPart + 1;
+  }
+
----------------
nit: should probably have a utility for all interested users to set up {StartPart, EndPart} from a given State.

May be worth trying to unify this handling of VF=1 within buildScalarSteps (TODO?).


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:125
 
   // Try to sink each replicate recipe in the worklist.
   while (!WorkList.empty()) {
----------------
replicate [or scalar IV steps] recipe


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:136
+      if (!RepR || RepR->isUniform() || RepR->mayHaveSideEffects() ||
+          RepR->mayReadOrWriteMemory())
+        continue;
----------------
Worth having a "canSinkScalar()" in RecipeBase, or VPValue?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:149
     // SinkCandidate. At the moment, we identify such UAV's by looking for the
     // address operands of widened memory recipes.
+    auto CanSinkWithUser = [SinkTo, &NeedsDuplicating, C](VPUser *U) {
----------------
Can we check directly if onlyFirstLaneUsed()?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:168
+      Instruction *I = cast<Instruction>(
+          cast<VPReplicateRecipe>(SinkCandidate)->getUnderlyingValue());
       auto *Clone =
----------------
Could ScalarIVSteps also feed a user using only first lane?
Conservatively check and set NeedsDuplicating only for RecplicateRecipes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133760