[llvm] c2bef38 - [VPlan] Remove setEntry to avoid leaks when replacing entry.

[Florian Hahn via llvm-commits]

Author: Florian Hahn
Date: 2023-05-04T19:12:02+01:00
New Revision: c2bef381fae95c40a8f8af0fa5674fd5640c582f

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

LOG: [VPlan] Remove setEntry to avoid leaks when replacing entry.

Update the HCFG builder to directly connect the created CFG to the
existing Plan's entry. This allows removing `setEntry`, which can cause
leaks when the existing entry is replaced.

Should fix
https://lab.llvm.org/buildbot/#/builders/5/builds/33455/steps/13/logs/stdio

Added: 
    

Modified: 
    llvm/lib/Transforms/Vectorize/VPlan.h
    llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
    llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.h
    llvm/unittests/Transforms/Vectorize/VPlanTestBase.h

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index a7bd00a9bb50..0360066daa23 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -2305,12 +2305,6 @@ class VPlan {
   VPBasicBlock *getEntry() { return Entry; }
   const VPBasicBlock *getEntry() const { return Entry; }
 
-  VPBasicBlock *setEntry(VPBasicBlock *Block) {
-    Entry = Block;
-    Block->setPlan(this);
-    return Entry;
-  }
-
   /// The trip count of the original loop.
   VPValue *getTripCount() const {
     assert(TripCount && "trip count needs to be set before accessing it");

diff  --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index 10fefae1cacf..f6e3a2a16db8 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -73,9 +73,8 @@ class PlainCFGBuilder {
   PlainCFGBuilder(Loop *Lp, LoopInfo *LI, VPlan &P)
       : TheLoop(Lp), LI(LI), Plan(P) {}
 
-  /// Build plain CFG for TheLoop. Return the pre-header VPBasicBlock connected
-  /// to a new VPRegionBlock (TopRegion) enclosing the plain CFG.
-  VPBasicBlock *buildPlainCFG();
+  /// Build plain CFG for TheLoop  and connects it to Plan's entry.
+  void buildPlainCFG();
 };
 } // anonymous namespace
 
@@ -254,7 +253,7 @@ void PlainCFGBuilder::createVPInstructionsForVPBB(VPBasicBlock *VPBB,
 }
 
 // Main interface to build the plain CFG.
-VPBasicBlock *PlainCFGBuilder::buildPlainCFG() {
+void PlainCFGBuilder::buildPlainCFG() {
   // 1. Scan the body of the loop in a topological order to visit each basic
   // block after having visited its predecessor basic blocks. Create a VPBB for
   // each BB and link it to its successor and predecessor VPBBs. Note that
@@ -267,7 +266,8 @@ VPBasicBlock *PlainCFGBuilder::buildPlainCFG() {
   BasicBlock *ThePreheaderBB = TheLoop->getLoopPreheader();
   assert((ThePreheaderBB->getTerminator()->getNumSuccessors() == 1) &&
          "Unexpected loop preheader");
-  VPBasicBlock *ThePreheaderVPBB = getOrCreateVPBB(ThePreheaderBB);
+  VPBasicBlock *ThePreheaderVPBB = Plan.getEntry();
+  BB2VPBB[ThePreheaderBB] = ThePreheaderVPBB;
   ThePreheaderVPBB->setName("vector.ph");
   for (auto &I : *ThePreheaderBB) {
     if (I.getType()->isVoidTy())
@@ -371,20 +371,17 @@ VPBasicBlock *PlainCFGBuilder::buildPlainCFG() {
   // have a VPlan couterpart. Fix VPlan phi nodes by adding their corresponding
   // VPlan operands.
   fixPhiNodes();
-
-  return ThePreheaderVPBB;
 }
 
-VPBasicBlock *VPlanHCFGBuilder::buildPlainCFG() {
+void VPlanHCFGBuilder::buildPlainCFG() {
   PlainCFGBuilder PCFGBuilder(TheLoop, LI, Plan);
-  return PCFGBuilder.buildPlainCFG();
+  PCFGBuilder.buildPlainCFG();
 }
 
 // Public interface to build a H-CFG.
 void VPlanHCFGBuilder::buildHierarchicalCFG() {
-  // Build Top Region enclosing the plain CFG and set it as VPlan entry.
-  VPBasicBlock *EntryVPBB = buildPlainCFG();
-  Plan.setEntry(EntryVPBB);
+  // Build Top Region enclosing the plain CFG.
+  buildPlainCFG();
   LLVM_DEBUG(Plan.setName("HCFGBuilder: Plain CFG\n"); dbgs() << Plan);
 
   VPRegionBlock *TopRegion = Plan.getVectorLoopRegion();

diff  --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.h b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.h
index 2d52990af268..299ae36155cb 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.h
@@ -57,9 +57,8 @@ class VPlanHCFGBuilder {
   // are introduced.
   VPDominatorTree VPDomTree;
 
-  /// Build plain CFG for TheLoop. Return the pre-header VPBasicBlock connected
-  /// to a new VPRegionBlock (TopRegion) enclosing the plain CFG.
-  VPBasicBlock *buildPlainCFG();
+  /// Build plain CFG for TheLoop and connects it to Plan's entry.
+  void buildPlainCFG();
 
 public:
   VPlanHCFGBuilder(Loop *Lp, LoopInfo *LI, VPlan &P)

diff  --git a/llvm/unittests/Transforms/Vectorize/VPlanTestBase.h b/llvm/unittests/Transforms/Vectorize/VPlanTestBase.h
index 86aea915fe0d..d9372174fdfa 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanTestBase.h
+++ b/llvm/unittests/Transforms/Vectorize/VPlanTestBase.h
@@ -78,8 +78,7 @@ class VPlanTestBase : public testing::Test {
     auto Plan = VPlan::createInitialVPlan(
         SE->getBackedgeTakenCount(LI->getLoopFor(LoopHeader)), *SE);
     VPlanHCFGBuilder HCFGBuilder(LI->getLoopFor(LoopHeader), LI.get(), *Plan);
-    VPBasicBlock *EntryVPBB = HCFGBuilder.buildPlainCFG();
-    Plan->setEntry(EntryVPBB);
+    HCFGBuilder.buildPlainCFG();
     return Plan;
   }
 };