[PATCH] D123537: [VPlan] Model first exit values in VPlan.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8887
 
+  // Introduce recipes modeling the exit values.
+  Builder.setInsertPoint(MiddleBlock, MiddleBlock->getFirstNonPhi());
----------------
This stretches the already too long method to exceed 400 lines... please outline.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8894
+        continue;
+      VPValue *V = Plan->getOrAddVPValue(
+          ExitPhi.getIncomingValueForBlock(OrigLoop->getLoopLatch()));
----------------
This should get rather than add, right?

Check if V is a header phi recipe, whose exit values are not represented yet?

Exit values of inductions are disconnected from the loop body by precomputing them in preheader, so modeling them as VPInstructions in the middle block would require also modeling their precomputation in the preheader?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8901
+  // Remove exit value recipes for cases not handled yet. Those include exit
+  // values of induction and reduction chains and first-order recurrence phis.
+  for (VPRecipeBase &R : HeaderVPBB->phis()) {
----------------
Can alternatively first identify all VPValues in loop which are Not-To-Feed-Exit-VPInstruction, then wrap every middle-block loop-closed phi with an Exit VPInstruction provided it does not use an NTFEV VPValue? Instead of collecting NTFEV's and calling removeExitUsers() for each.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:818
+    if (getParent()->getPlan()->isUniformAfterVectorization(getOperand(0)))
+      Lane = VPLane::getFirstLane();
+    ExitPhi->addIncoming(
----------------
Indicate that ExitValue usesScalars()?
Perhaps worth emphasizing in the name - reduction epilogues using vectors probably deserve a separate "Exit Value" recipe.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:822
+        State.CFG.VPBB2IRBB[getParent()]);
+
+    break;
----------------
nit: empty line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123537