[PATCH] D139927: [VPlan] Remove redundant blocks by merging them into predecessors.

Ayal Zaks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 20 06:05:39 PST 2022


Ayal added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9170
-  // TODO: Fold block earlier once all VPlan transforms properly maintain a
-  // VPBasicBlock as exit.
-  VPBlockUtils::tryToMergeBlockIntoPredecessor(TopRegion->getExiting());
----------------
nit: not sure what is meant here by "earlier" or improper maintenance of VPlan transforms?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:310
+  ReversePostOrderTraversal<VPBlockRecursiveTraversalWrapper<VPBlockBase *>>
+      RPOT(Plan.getEntry());
+  for (VPBlockBase *VPB : RPOT)
----------------
Is RPOT needed (due to destructive / make_early_inc_range) or would depth_first suffice?

Suffice to iterate over blocksOnly<VPBasicBlock> and tryToMerge[Basic]BlockIntoPredecessor(VPBasicBlock)?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:312
+  for (VPBlockBase *VPB : RPOT)
+    Changed |= VPBlockUtils::tryToMergeBlockIntoPredecessor(VPB) != nullptr;
+  return Changed;
----------------
Is tryToMergeBlockIntoPredecessor() still needed (by buildVPlanWithVPRecipes()), or can this pass absorb it?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.h:42
 
+  static bool mergeBlocksIntoPredecessors(VPlan &Plan);
+
----------------
Missing documentation. (Here are in few other transforms, independently of this patch)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139927



More information about the llvm-commits mailing list