[VPlan] Disconnect VPRegionBlock from successors in graph iterator(NFCI)

This updates the VPAllSuccessorsIterator to not connect the
VPRegionBlock itself to its successors. The successors are connected to
the exit block of the region. At the moment, this doesn't change any
exisint functionality.

But the new schema ensures the following property when used for
VPDominatorTree:

1. Entry & exit blocks of regions dominate the successors of the region.

This allows for convenient checking of dominance between defs and uses
that are not defined in the same region. I will share a follow-up patch
to use it for the VPDominatorTree soon.

Depends on D140500.

Reviewed By: Ayal

Differential Revision: https://reviews.llvm.org/D140511

diff --git a/llvm/lib/Transforms/Vectorize/VPlanCFG.h b/llvm/lib/Transforms/Vectorize/VPlanCFG.h
index cd8db21..5234c9f 100644 (file)
--- a/llvm/lib/Transforms/Vectorize/VPlanCFG.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanCFG.h
@@ -144,7 +144,9 @@ struct GraphTraits<Inverse<VPRegionBlock *>>
 /// entry node of VPRegionBlocks. Exit blocks of a region implicitly have their
 /// parent region's successors. This ensures all blocks in a region are visited
 /// before any blocks in a successor region when doing a reverse post-order
-// traversal of the graph.
+// traversal of the graph. Region blocks themselves traverse only their entries
+// directly and not their successors. Those will be traversed when a region's
+// exiting block is traversed
 template <typename BlockPtrTy>
 class VPAllSuccessorsIterator
     : public iterator_facade_base<VPAllSuccessorsIterator<BlockPtrTy>,
@@ -166,9 +168,8 @@ class VPAllSuccessorsIterator
   /// both the const and non-const operator* implementations.
   template <typename T1> static T1 deref(T1 Block, unsigned SuccIdx) {
     if (auto *R = dyn_cast<VPRegionBlock>(Block)) {
-      if (SuccIdx == 0)
-        return R->getEntry();
-      SuccIdx--;
+      assert(SuccIdx == 0);
+      return R->getEntry();
     }
 
     // For exit blocks, use the next parent region with successors.
@@ -188,12 +189,13 @@ public:
   }
 
   static VPAllSuccessorsIterator end(BlockPtrTy Block) {
+    if (auto *R = dyn_cast<VPRegionBlock>(Block)) {
+      // Traverse through the region's entry node.
+      return {R, 1};
+    }
     BlockPtrTy ParentWithSuccs = getBlockWithSuccs(Block);
     unsigned NumSuccessors =
         ParentWithSuccs ? ParentWithSuccs->getNumSuccessors() : 0;
-
-    if (auto *R = dyn_cast<VPRegionBlock>(Block))
-      return {R, NumSuccessors + 1};
     return {Block, NumSuccessors};
   }
 

diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
index 0effff9..8995399 100644 (file)
--- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
@@ -398,9 +398,8 @@ TEST(VPBasicBlockTest, TraversingIteratorTest) {
     SmallVector<const VPBlockBase *> FromIterator(
         VPAllSuccessorsIterator<VPBlockBase *>(R1),
         VPAllSuccessorsIterator<VPBlockBase *>::end(R1));
-    EXPECT_EQ(2u, FromIterator.size());
+    EXPECT_EQ(1u, FromIterator.size());
     EXPECT_EQ(R1BB1, FromIterator[0]);
-    EXPECT_EQ(R2, FromIterator[1]);
 
     // Depth-first.
     VPBlockRecursiveTraversalWrapper<VPBlockBase *> Start(R1);