[PATCH] D149903: [VPlan] Replace IR based truncateToMinimalBitwidths with VPlan version.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

Looks much simpler! Minor last nits.



================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:874
+/// Insert truncates and extends for any truncated instructions as hints to
+/// InstCombine.
+static void
----------------
nit: are these still hints to InstCombine, or to subsequent VPlan cleanups?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:883-886
+  VPBasicBlock *PH =
+      cast<VPBasicBlock>(Plan.getVectorLoopRegion()->getSinglePredecessor());
+  ReversePostOrderTraversal<VPBlockDeepTraversalWrapper<VPBlockBase *>> RPOT(
+      Plan.getEntry());
----------------
?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:933
+            ProcessedRecipes +=
+                MinBWs.contains(dyn_cast<Instruction>(Op->getLiveInIRValue()));
+#endif
----------------
Is it possible for MinBWs not to contain Op's live-in IR value in this case?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:967-969
+  VPTypeAnalysis TypeInfo(SE.getContext());
+  if (!Plan.hasVF(ElementCount::getFixed(1)))
+    truncateToMinimalBitwidths(Plan, MinBWs, TypeInfo);
----------------
nit


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:838
+  for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(
+           vp_depth_first_deep(Plan.getEntry()))) {
+    for (VPRecipeBase &R : make_early_inc_range(*VPBB)) {
----------------
fhahn wrote:
> Ayal wrote:
> > Any order other than depth first would also do, right?
> Yes, I think the order doesn't matter here.
But a (more) expensive RPOT order is needed, to handle defs before uses?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149903