[llvm] [SimplifyCFG][NFC] Improve compile time for TryToSimplifyUncondBranchFromEmptyBlock optimization. (PR #110715)

Amara Emerson via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 9 10:11:45 PDT 2024


https://github.com/aemerson updated https://github.com/llvm/llvm-project/pull/110715

>From 03d9ec6be394ad82cd2840e5c199df4dfe43c2a5 Mon Sep 17 00:00:00 2001
From: Amara Emerson <amara at apple.com>
Date: Wed, 2 Oct 2024 12:29:10 -0700
Subject: [PATCH 1/3] [SimplifyCFG][NFC] Improve compile time for
 TryToSimplifyUncondBranchFromEmptyBlock optimization.

In some pathological cases this optimization can spend an unreasonable amount of
time populating the set for predecessors of the successor block. This change
sinks some of that initializing to the point where it's actually necessary so we
can take advantage of the existing early-exits.

rdar://137063034
---
 llvm/lib/Transforms/Utils/Local.cpp | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 7659fc69196151..6c2e332ef3c590 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1020,10 +1020,11 @@ static void replaceUndefValuesInPhi(PHINode *PN,
 // Only when they shares a single common predecessor, return true.
 // Only handles cases when BB can't be merged while its predecessors can be
 // redirected.
+// \p SuccPredsOut may be partially populated with the predecessors of Succ.
 static bool
 CanRedirectPredsOfEmptyBBToSucc(BasicBlock *BB, BasicBlock *Succ,
                                 const SmallPtrSetImpl<BasicBlock *> &BBPreds,
-                                const SmallPtrSetImpl<BasicBlock *> &SuccPreds,
+                                SmallPtrSetImpl<BasicBlock *> &SuccPredsOut,
                                 BasicBlock *&CommonPred) {
 
   // There must be phis in BB, otherwise BB will be merged into Succ directly
@@ -1042,7 +1043,8 @@ CanRedirectPredsOfEmptyBBToSucc(BasicBlock *BB, BasicBlock *Succ,
 
   // Get the single common predecessor of both BB and Succ. Return false
   // when there are more than one common predecessors.
-  for (BasicBlock *SuccPred : SuccPreds) {
+  for (BasicBlock *SuccPred : predecessors(Succ)) {
+    SuccPredsOut.insert(SuccPred);
     if (BBPreds.count(SuccPred)) {
       if (CommonPred)
         return false;
@@ -1166,7 +1168,7 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
     return false;
 
   SmallPtrSet<BasicBlock *, 16> BBPreds(pred_begin(BB), pred_end(BB));
-  SmallPtrSet<BasicBlock *, 16> SuccPreds(pred_begin(Succ), pred_end(Succ));
+  SmallPtrSet<BasicBlock *, 16> SuccPreds;
 
   // The single common predecessor of BB and Succ when BB cannot be killed
   BasicBlock *CommonPred = nullptr;
@@ -1182,6 +1184,10 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
   if ((!BBKillable && !BBPhisMergeable) || introduceTooManyPhiEntries(BB, Succ))
     return false;
 
+  // SuccPreds may not have been fully filled by CanRedirectPredsOfEmptyBBToSucc
+  // so finish it here.
+  SuccPreds.insert(pred_begin(Succ), pred_end(Succ));
+
   // Check to see if merging these blocks/phis would cause conflicts for any of
   // the phi nodes in BB or Succ. If not, we can safely merge.
 

>From 7a4c5218fd0a47f5a102ea9d30e9ca163ddf5de8 Mon Sep 17 00:00:00 2001
From: Amara Emerson <amara at apple.com>
Date: Mon, 7 Oct 2024 13:03:30 -0700
Subject: [PATCH 2/3] Don't unnecessarily cache.

---
 llvm/lib/Transforms/Utils/Local.cpp | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 6c2e332ef3c590..22a9ba9155b40b 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1020,11 +1020,9 @@ static void replaceUndefValuesInPhi(PHINode *PN,
 // Only when they shares a single common predecessor, return true.
 // Only handles cases when BB can't be merged while its predecessors can be
 // redirected.
-// \p SuccPredsOut may be partially populated with the predecessors of Succ.
 static bool
 CanRedirectPredsOfEmptyBBToSucc(BasicBlock *BB, BasicBlock *Succ,
                                 const SmallPtrSetImpl<BasicBlock *> &BBPreds,
-                                SmallPtrSetImpl<BasicBlock *> &SuccPredsOut,
                                 BasicBlock *&CommonPred) {
 
   // There must be phis in BB, otherwise BB will be merged into Succ directly
@@ -1044,7 +1042,6 @@ CanRedirectPredsOfEmptyBBToSucc(BasicBlock *BB, BasicBlock *Succ,
   // Get the single common predecessor of both BB and Succ. Return false
   // when there are more than one common predecessors.
   for (BasicBlock *SuccPred : predecessors(Succ)) {
-    SuccPredsOut.insert(SuccPred);
     if (BBPreds.count(SuccPred)) {
       if (CommonPred)
         return false;
@@ -1168,7 +1165,6 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
     return false;
 
   SmallPtrSet<BasicBlock *, 16> BBPreds(pred_begin(BB), pred_end(BB));
-  SmallPtrSet<BasicBlock *, 16> SuccPreds;
 
   // The single common predecessor of BB and Succ when BB cannot be killed
   BasicBlock *CommonPred = nullptr;
@@ -1177,16 +1173,13 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
 
   // Even if we can not fold BB into Succ, we may be able to redirect the
   // predecessors of BB to Succ.
-  bool BBPhisMergeable =
-      BBKillable ||
-      CanRedirectPredsOfEmptyBBToSucc(BB, Succ, BBPreds, SuccPreds, CommonPred);
+  bool BBPhisMergeable = BBKillable || CanRedirectPredsOfEmptyBBToSucc(
+                                           BB, Succ, BBPreds, CommonPred);
 
   if ((!BBKillable && !BBPhisMergeable) || introduceTooManyPhiEntries(BB, Succ))
     return false;
 
-  // SuccPreds may not have been fully filled by CanRedirectPredsOfEmptyBBToSucc
-  // so finish it here.
-  SuccPreds.insert(pred_begin(Succ), pred_end(Succ));
+  SmallPtrSet<BasicBlock *, 16> SuccPreds(pred_begin(Succ), pred_end(Succ));
 
   // Check to see if merging these blocks/phis would cause conflicts for any of
   // the phi nodes in BB or Succ. If not, we can safely merge.

>From 1e602db132508cfdd2e116979f11ed964d737fdd Mon Sep 17 00:00:00 2001
From: Amara Emerson <amara at apple.com>
Date: Tue, 8 Oct 2024 14:46:33 -0700
Subject: [PATCH 3/3] Sink SuccPreds further.

---
 llvm/lib/Transforms/Utils/Local.cpp | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 22a9ba9155b40b..d8c3386db402c9 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1179,8 +1179,6 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
   if ((!BBKillable && !BBPhisMergeable) || introduceTooManyPhiEntries(BB, Succ))
     return false;
 
-  SmallPtrSet<BasicBlock *, 16> SuccPreds(pred_begin(Succ), pred_end(Succ));
-
   // Check to see if merging these blocks/phis would cause conflicts for any of
   // the phi nodes in BB or Succ. If not, we can safely merge.
 
@@ -1301,7 +1299,7 @@ bool llvm::TryToSimplifyUncondBranchFromEmptyBlock(BasicBlock *BB,
     // All predecessors of BB (except the common predecessor) will be moved to
     // Succ.
     Updates.reserve(Updates.size() + 2 * pred_size(BB) + 1);
-
+    SmallPtrSet<BasicBlock *, 16> SuccPreds(pred_begin(Succ), pred_end(Succ));
     for (auto *PredOfBB : predecessors(BB)) {
       // Do not modify those common predecessors of BB and Succ
       if (!SuccPreds.contains(PredOfBB))



More information about the llvm-commits mailing list