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

Sun Dec 25 12:04:06 PST 2022

fhahn marked 2 inline comments as done.
fhahn added inline comments.

================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp:310
+  ReversePostOrderTraversal<VPBlockRecursiveTraversalWrapper<VPBlockBase *>>
+      RPOT(Plan.getEntry());
+  for (VPBlockBase *VPB : RPOT)
----------------
Ayal wrote:
> 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)?
Unforunately `depth_first` doesn't support `make_early_inc_range` at the moment, I am not sure if that's because of VPBlockRecursiveTraversalWrapper or something else.

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

Done, thanks!

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

================
Comment at: llvm/lib/Transforms/Vectorize/VPlanTransforms.h:42

+  static bool mergeBlocksIntoPredecessors(VPlan &Plan);
+
----------------
Ayal wrote:
> Missing documentation. (Here are in few other transforms, independently of this patch)
Added, thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139927