[PATCH] D126618: [VPlan] Replace CondBit with BranchOnCond VPInstruction.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

Should tests check for new BranchOnCond VPInstruction?

Are the users of CondBit/Predicate ok w/ this change (VPlanPredicator)?



================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:734
+    }
+
+    if (getParent() == TopRegion->getExitingBasicBlock()) {
----------------
Similar to VPBranchOnMaskRecipe (have all terminator recipes share this somehow?):
  // Replace the temporary unreachable terminator with a new conditional branch,
  // hooking it up to backward destination now and to forward destination(s) later when they are created.

Handle loop branches of all loops, not only outermost?

Set exit successor to nullptr to be set later, rather than Builder.GetInsertBlock()?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:553
 
   /// Remove all the successors of this block and set to null its condition bit
+  void clearSuccessors() { Successors.clear(); }
----------------
Update comment.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp:60
+               VPI->getOpcode() == VPInstruction::BranchOnCount;
+      return isa<VPBranchOnMaskRecipe>(R);
+    }();
----------------
May be useful to have an isaTerminator(Recipe).

Can have VPBB hold its 'singleton' terminator recipe (instead of CondBit) separately from all other recipes, analogous to the idea for Region to hold its singleton canonical IV recipe directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126618