From 03d9ec6be394ad82cd2840e5c199df4dfe43c2a5 Mon Sep 17 00:00:00 2001 From: Amara Emerson 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 7659fc6919615..6c2e332ef3c59 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 &BBPreds, - const SmallPtrSetImpl &SuccPreds, + SmallPtrSetImpl &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 BBPreds(pred_begin(BB), pred_end(BB)); - SmallPtrSet SuccPreds(pred_begin(Succ), pred_end(Succ)); + SmallPtrSet 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 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 6c2e332ef3c59..22a9ba9155b40 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 &BBPreds, - SmallPtrSetImpl &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 BBPreds(pred_begin(BB), pred_end(BB)); - SmallPtrSet 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 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 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 22a9ba9155b40..d8c3386db402c 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 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 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))