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

[Florian Hahn via llvm-commits]

Author: Florian Hahn
Date: 2023-01-18T15:02:41Z
New Revision: feee22db52259d19a8624318517ef5688abfc67c

URL: https://github.com/llvm/llvm-project/commit/feee22db52259d19a8624318517ef5688abfc67c
DIFF: https://github.com/llvm/llvm-project/commit/feee22db52259d19a8624318517ef5688abfc67c.diff

LOG: [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

Added: 
    

Modified: 
    llvm/lib/Transforms/Vectorize/VPlanCFG.h
    llvm/unittests/Transforms/Vectorize/VPlanTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Vectorize/VPlanCFG.h b/llvm/lib/Transforms/Vectorize/VPlanCFG.h
index cd8db21acab9..5234c9ff650a 100644
--- 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 @@ class VPAllSuccessorsIterator
   }
 
   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 0effff9b745d..899539969b73 100644
--- 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);