[llvm] [SSAUpdaterBulk] Fix incorrect live-in values for a block. (PR #131762)
Valery Pykhtin via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 20 23:19:47 PDT 2025
https://github.com/vpykhtin updated https://github.com/llvm/llvm-project/pull/131762
>From 51fff2801c287b6b7f585493643c2a2d31b7b828 Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Mon, 17 Mar 2025 18:42:27 +0000
Subject: [PATCH 1/8] [SSAUpdaterBulk] Simplify SSAUpdaterBulk::RewriteAllUses.
NFC. - removed redundant resetLiveInBlocks - avoid temporary PHI holder
container.
---
llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
index cad7ff64c01fb..4bb2fb909c113 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
@@ -143,26 +143,23 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
SmallVector<BasicBlock *, 32> IDFBlocks;
SmallPtrSet<BasicBlock *, 32> LiveInBlocks;
ComputeLiveInBlocks(UsingBlocks, DefBlocks, LiveInBlocks, PredCache);
- IDF.resetLiveInBlocks();
IDF.setLiveInBlocks(LiveInBlocks);
IDF.calculate(IDFBlocks);
// We've computed IDF, now insert new phi-nodes there.
- SmallVector<PHINode *, 4> InsertedPHIsForVar;
for (auto *FrontierBB : IDFBlocks) {
IRBuilder<> B(FrontierBB, FrontierBB->begin());
PHINode *PN = B.CreatePHI(R.Ty, 0, R.Name);
R.Defines[FrontierBB] = PN;
- InsertedPHIsForVar.push_back(PN);
if (InsertedPHIs)
InsertedPHIs->push_back(PN);
}
// Fill in arguments of the inserted PHIs.
- for (auto *PN : InsertedPHIsForVar) {
- BasicBlock *PBB = PN->getParent();
- for (BasicBlock *Pred : PredCache.get(PBB))
- PN->addIncoming(computeValueAt(Pred, R, DT), Pred);
+ for (auto *BB : IDFBlocks) {
+ auto *PHI = cast<PHINode>(&BB->front());
+ for (BasicBlock *Pred : PredCache.get(BB))
+ PHI->addIncoming(computeValueAt(Pred, R, DT), Pred);
}
// Rewrite actual uses with the inserted definitions.
>From 5761cf5bc31c95e516a532dee138c2dfbc7a97e4 Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Mon, 17 Mar 2025 19:01:43 +0000
Subject: [PATCH 2/8] [SSAUpdaterBulk] Fix incorrect live-in values for a
block.
---
.../llvm/Transforms/Utils/SSAUpdaterBulk.h | 4 +-
llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp | 70 +++++++++++++------
.../Transforms/Utils/SSAUpdaterBulkTest.cpp | 10 ++-
3 files changed, 54 insertions(+), 30 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/SSAUpdaterBulk.h b/llvm/include/llvm/Transforms/Utils/SSAUpdaterBulk.h
index ad24cb454d5e7..b2cf29608f58b 100644
--- a/llvm/include/llvm/Transforms/Utils/SSAUpdaterBulk.h
+++ b/llvm/include/llvm/Transforms/Utils/SSAUpdaterBulk.h
@@ -38,7 +38,7 @@ class DominatorTree;
/// is used).
class SSAUpdaterBulk {
struct RewriteInfo {
- DenseMap<BasicBlock *, Value *> Defines;
+ SmallVector<std::pair<BasicBlock *, Value *>, 4> Defines;
SmallVector<Use *, 4> Uses;
StringRef Name;
Type *Ty;
@@ -49,8 +49,6 @@ class SSAUpdaterBulk {
PredIteratorCache PredCache;
- Value *computeValueAt(BasicBlock *BB, RewriteInfo &R, DominatorTree *DT);
-
public:
explicit SSAUpdaterBulk() = default;
SSAUpdaterBulk(const SSAUpdaterBulk &) = delete;
diff --git a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
index 4bb2fb909c113..0e689319b8fac 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
@@ -53,7 +53,7 @@ void SSAUpdaterBulk::AddAvailableValue(unsigned Var, BasicBlock *BB, Value *V) {
LLVM_DEBUG(dbgs() << "SSAUpdater: Var=" << Var
<< ": added new available value " << *V << " in "
<< BB->getName() << "\n");
- Rewrites[Var].Defines[BB] = V;
+ Rewrites[Var].Defines.emplace_back(BB, V);
}
/// Record a use of the symbolic value. This use will be updated with a
@@ -65,21 +65,6 @@ void SSAUpdaterBulk::AddUse(unsigned Var, Use *U) {
Rewrites[Var].Uses.push_back(U);
}
-// Compute value at the given block BB. We either should already know it, or we
-// should be able to recursively reach it going up dominator tree.
-Value *SSAUpdaterBulk::computeValueAt(BasicBlock *BB, RewriteInfo &R,
- DominatorTree *DT) {
- if (!R.Defines.count(BB)) {
- if (DT->isReachableFromEntry(BB) && PredCache.get(BB).size()) {
- BasicBlock *IDom = DT->getNode(BB)->getIDom()->getBlock();
- Value *V = computeValueAt(IDom, R, DT);
- R.Defines[BB] = V;
- } else
- R.Defines[BB] = UndefValue::get(R.Ty);
- }
- return R.Defines[BB];
-}
-
/// Given sets of UsingBlocks and DefBlocks, compute the set of LiveInBlocks.
/// This is basically a subgraph limited by DefBlocks and UsingBlocks.
static void
@@ -117,11 +102,19 @@ ComputeLiveInBlocks(const SmallPtrSetImpl<BasicBlock *> &UsingBlocks,
}
}
+struct BBValueInfo {
+ Value *LiveInValue = nullptr;
+ Value *LiveOutValue = nullptr;
+};
+
/// Perform all the necessary updates, including new PHI-nodes insertion and the
/// requested uses update.
void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
SmallVectorImpl<PHINode *> *InsertedPHIs) {
+ DenseMap<BasicBlock *, BBValueInfo> BBInfos;
for (auto &R : Rewrites) {
+ BBInfos.clear();
+
// Compute locations for new phi-nodes.
// For that we need to initialize DefBlocks from definitions in R.Defines,
// UsingBlocks from uses in R.Uses, then compute LiveInBlocks, and then use
@@ -132,8 +125,8 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
<< " use(s)\n");
SmallPtrSet<BasicBlock *, 2> DefBlocks;
- for (auto &Def : R.Defines)
- DefBlocks.insert(Def.first);
+ for (auto [BB, V] : R.Defines)
+ DefBlocks.insert(BB);
IDF.setDefiningBlocks(DefBlocks);
SmallPtrSet<BasicBlock *, 2> UsingBlocks;
@@ -146,20 +139,52 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
IDF.setLiveInBlocks(LiveInBlocks);
IDF.calculate(IDFBlocks);
+ // Reserve map large enough to reduce growth.
+ BBInfos.init(LiveInBlocks.size() + DefBlocks.size());
+
+ for (auto [BB, V] : R.Defines)
+ BBInfos[BB].LiveOutValue = V;
+
// We've computed IDF, now insert new phi-nodes there.
for (auto *FrontierBB : IDFBlocks) {
IRBuilder<> B(FrontierBB, FrontierBB->begin());
PHINode *PN = B.CreatePHI(R.Ty, 0, R.Name);
- R.Defines[FrontierBB] = PN;
+ BBInfos[FrontierBB].LiveInValue = PN;
if (InsertedPHIs)
InsertedPHIs->push_back(PN);
}
+ // IsLiveOut indicates whether we are computing live-out values (true) or
+ // live-in values (false).
+ std::function<Value *(BasicBlock *, bool)> computeValue =
+ [&](BasicBlock *BB, bool IsLiveOut) -> Value * {
+ auto &BBInfo = BBInfos[BB];
+
+ if (IsLiveOut && BBInfo.LiveOutValue)
+ return BBInfo.LiveOutValue;
+
+ if (BBInfo.LiveInValue)
+ return BBInfo.LiveInValue;
+
+ Value *V = DT->isReachableFromEntry(BB) && PredCache.get(BB).size()
+ ? computeValue(DT->getNode(BB)->getIDom()->getBlock(),
+ /* IsLiveOut = */ true)
+ : UndefValue::get(R.Ty);
+
+ // The call to computeValue for the dominator block can insert another
+ // entry into the map, potentially causing the map to grow and
+ // invalidating the BBInfo reference. Therefore, we need to perform
+ // another map lookup. Simply reserving map size may not be sufficient
+ // as the map could grow further.
+ BBInfos[BB].LiveInValue = V;
+ return V;
+ };
+
// Fill in arguments of the inserted PHIs.
for (auto *BB : IDFBlocks) {
auto *PHI = cast<PHINode>(&BB->front());
for (BasicBlock *Pred : PredCache.get(BB))
- PHI->addIncoming(computeValueAt(Pred, R, DT), Pred);
+ PHI->addIncoming(computeValue(Pred, /* IsLiveOut = */ true), Pred);
}
// Rewrite actual uses with the inserted definitions.
@@ -167,7 +192,10 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
for (Use *U : R.Uses) {
if (!ProcessedUses.insert(U).second)
continue;
- Value *V = computeValueAt(getUserBB(U), R, DT);
+
+ auto *User = cast<Instruction>(U->getUser());
+ BasicBlock *BB = getUserBB(U);
+ Value *V = computeValue(BB, /* IsLiveOut = */ BB != User->getParent());
Value *OldVal = U->get();
assert(OldVal && "Invalid use!");
// Notify that users of the existing value that it is being replaced.
diff --git a/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp b/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
index cfd7fbb445840..b97d8ed67a184 100644
--- a/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
+++ b/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
@@ -236,7 +236,7 @@ TEST(SSAUpdaterBulk, SingleBBLoop) {
Loop->dump();
// Output:
// loop: ; preds = %loop, %entry
- // %i.prev = phi i32 [ %i.prev, %loop ], [ 0, %entry ]
+ // %i.prev = phi i32 [ %i, %loop ], [ 0, %entry ]
// %i = add i32 %i.prev, 1
// %cmp = icmp slt i32 %i, 42
// br i1 %cmp, label %loop, label %exit
@@ -246,8 +246,7 @@ TEST(SSAUpdaterBulk, SingleBBLoop) {
PHINode *Phi = Inserted[0];
EXPECT_EQ(Phi, dyn_cast<PHINode>(I->getOperand(0)));
EXPECT_EQ(Phi->getIncomingValueForBlock(Entry), ConstantInt::get(I32Ty, 0));
- EXPECT_NONFATAL_FAILURE(EXPECT_EQ(Phi->getIncomingValueForBlock(Loop), I),
- "Expected equality of these values");
+ EXPECT_EQ(Phi->getIncomingValueForBlock(Loop), I);
}
TEST(SSAUpdaterBulk, TwoBBLoop) {
@@ -298,15 +297,14 @@ TEST(SSAUpdaterBulk, TwoBBLoop) {
// %i.prev = phi i32 [ %i, %loop ], [ 0, %entry ]
// br label %loop
// loop: ; preds = %loop_header
- // %i = add i32 %i, 1
+ // %i = add i32 %i.prev, 1
// %cmp = icmp slt i32 %i, 42
// br i1 %cmp, label %loop_header, label %exit
#endif
ASSERT_EQ(Inserted.size(), 1u);
PHINode *Phi = Inserted[0];
- EXPECT_NONFATAL_FAILURE(EXPECT_EQ(Phi, dyn_cast<PHINode>(I->getOperand(0))),
- "Expected equality of these values");
+ EXPECT_EQ(Phi, dyn_cast<PHINode>(I->getOperand(0)));
EXPECT_EQ(Phi->getParent(), LoopHdr);
EXPECT_EQ(Phi->getIncomingValueForBlock(Entry), ConstantInt::get(I32Ty, 0));
EXPECT_EQ(Phi->getIncomingValueForBlock(Loop), I);
>From b42403babc608f57d220c1253c7668fa54a8c0c7 Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Tue, 18 Mar 2025 13:27:07 +0000
Subject: [PATCH 3/8] per review fixes.
---
llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp | 21 ++++++++++----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
index 0e689319b8fac..bcbad92d1315b 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
@@ -139,7 +139,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
IDF.setLiveInBlocks(LiveInBlocks);
IDF.calculate(IDFBlocks);
- // Reserve map large enough to reduce growth.
+ // Important: reserve sufficient buckets to prevent map growth. [1]
BBInfos.init(LiveInBlocks.size() + DefBlocks.size());
for (auto [BB, V] : R.Defines)
@@ -166,17 +166,16 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
if (BBInfo.LiveInValue)
return BBInfo.LiveInValue;
- Value *V = DT->isReachableFromEntry(BB) && PredCache.get(BB).size()
+ Value *V = DT->isReachableFromEntry(BB) && !PredCache.get(BB).empty()
? computeValue(DT->getNode(BB)->getIDom()->getBlock(),
- /* IsLiveOut = */ true)
+ /*IsLiveOut=*/true)
: UndefValue::get(R.Ty);
- // The call to computeValue for the dominator block can insert another
- // entry into the map, potentially causing the map to grow and
- // invalidating the BBInfo reference. Therefore, we need to perform
- // another map lookup. Simply reserving map size may not be sufficient
- // as the map could grow further.
- BBInfos[BB].LiveInValue = V;
+ // The call to computeValue can insert new entries into the map:
+ // assume BBInfos shouldn't grow due to [1] above and BBInfo reference is
+ // valid.
+ assert(&BBInfo == &BBInfos[BB] && "Map shouldn't grow!");
+ BBInfo.LiveInValue = V;
return V;
};
@@ -184,7 +183,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
for (auto *BB : IDFBlocks) {
auto *PHI = cast<PHINode>(&BB->front());
for (BasicBlock *Pred : PredCache.get(BB))
- PHI->addIncoming(computeValue(Pred, /* IsLiveOut = */ true), Pred);
+ PHI->addIncoming(computeValue(Pred, /*IsLiveOut=*/true), Pred);
}
// Rewrite actual uses with the inserted definitions.
@@ -195,7 +194,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
auto *User = cast<Instruction>(U->getUser());
BasicBlock *BB = getUserBB(U);
- Value *V = computeValue(BB, /* IsLiveOut = */ BB != User->getParent());
+ Value *V = computeValue(BB, /*IsLiveOut=*/ BB != User->getParent());
Value *OldVal = U->get();
assert(OldVal && "Invalid use!");
// Notify that users of the existing value that it is being replaced.
>From 230c403dd079e8f424b985e3ad0179202cfb4af8 Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Tue, 18 Mar 2025 13:49:06 +0000
Subject: [PATCH 4/8] formatting
---
llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
index bcbad92d1315b..95493a4634e60 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
@@ -194,7 +194,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
auto *User = cast<Instruction>(U->getUser());
BasicBlock *BB = getUserBB(U);
- Value *V = computeValue(BB, /*IsLiveOut=*/ BB != User->getParent());
+ Value *V = computeValue(BB, /*IsLiveOut=*/BB != User->getParent());
Value *OldVal = U->get();
assert(OldVal && "Invalid use!");
// Notify that users of the existing value that it is being replaced.
>From 0497e5cbd1c4017507d28c33f134550e87eadf06 Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Wed, 19 Mar 2025 07:43:56 +0000
Subject: [PATCH 5/8] remove header for EXPECT_NONFATAL_FAILURE
---
llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp b/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
index b97d8ed67a184..841f44cf6bfed 100644
--- a/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
+++ b/llvm/unittests/Transforms/Utils/SSAUpdaterBulkTest.cpp
@@ -15,7 +15,6 @@
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h"
#include "llvm/Support/SourceMgr.h"
-#include "gtest/gtest-spi.h"
#include "gtest/gtest.h"
using namespace llvm;
>From 7b5f2aaa1493740f91b4fe3893c59b0716db19b5 Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Wed, 19 Mar 2025 15:21:40 +0000
Subject: [PATCH 6/8] per review fixes
---
llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
index 95493a4634e60..0063792332bca 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
@@ -139,7 +139,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
IDF.setLiveInBlocks(LiveInBlocks);
IDF.calculate(IDFBlocks);
- // Important: reserve sufficient buckets to prevent map growth. [1]
+ // Reserve sufficient buckets to prevent map growth. [1]
BBInfos.init(LiveInBlocks.size() + DefBlocks.size());
for (auto [BB, V] : R.Defines)
@@ -156,7 +156,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
// IsLiveOut indicates whether we are computing live-out values (true) or
// live-in values (false).
- std::function<Value *(BasicBlock *, bool)> computeValue =
+ std::function<Value *(BasicBlock *, bool)> ComputeValue =
[&](BasicBlock *BB, bool IsLiveOut) -> Value * {
auto &BBInfo = BBInfos[BB];
@@ -167,11 +167,11 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
return BBInfo.LiveInValue;
Value *V = DT->isReachableFromEntry(BB) && !PredCache.get(BB).empty()
- ? computeValue(DT->getNode(BB)->getIDom()->getBlock(),
+ ? ComputeValue(DT->getNode(BB)->getIDom()->getBlock(),
/*IsLiveOut=*/true)
: UndefValue::get(R.Ty);
- // The call to computeValue can insert new entries into the map:
+ // The call to ComputeValue can insert new entries into the map:
// assume BBInfos shouldn't grow due to [1] above and BBInfo reference is
// valid.
assert(&BBInfo == &BBInfos[BB] && "Map shouldn't grow!");
@@ -183,7 +183,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
for (auto *BB : IDFBlocks) {
auto *PHI = cast<PHINode>(&BB->front());
for (BasicBlock *Pred : PredCache.get(BB))
- PHI->addIncoming(computeValue(Pred, /*IsLiveOut=*/true), Pred);
+ PHI->addIncoming(ComputeValue(Pred, /*IsLiveOut=*/true), Pred);
}
// Rewrite actual uses with the inserted definitions.
@@ -194,7 +194,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
auto *User = cast<Instruction>(U->getUser());
BasicBlock *BB = getUserBB(U);
- Value *V = computeValue(BB, /*IsLiveOut=*/BB != User->getParent());
+ Value *V = ComputeValue(BB, /*IsLiveOut=*/BB != User->getParent());
Value *OldVal = U->get();
assert(OldVal && "Invalid use!");
// Notify that users of the existing value that it is being replaced.
>From 119aa5db42cf322520f08ba3fb87ef3a6fe3092e Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Thu, 20 Mar 2025 06:18:50 +0000
Subject: [PATCH 7/8] Use reserve and remove recursion.
---
llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp | 105 +++++++++++++++----
1 file changed, 84 insertions(+), 21 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
index 0063792332bca..fca9a2b24c56d 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
@@ -102,6 +102,51 @@ ComputeLiveInBlocks(const SmallPtrSetImpl<BasicBlock *> &UsingBlocks,
}
}
+#if 0
+struct XXX {
+ struct BBValueInfo {
+ Value *LiveInValue = nullptr;
+ Value *LiveOutValue = nullptr;
+ };
+
+ DominatorTree &DT;
+ PredIteratorCache &PredCache;
+ Type *ValTy;
+ DenseMap<BasicBlock *, BBValueInfo> BBInfos;
+
+ Value *computeLiveInValue(BasicBlock *BB) {
+ return computeLiveInValueImpl(BB, BBInfos[BB]);
+ }
+
+ Value *computeLiveOutValue(BasicBlock *BB) {
+ auto &BBInfo = BBInfos[BB];
+ return BBInfo.LiveOutValue ? BBInfo.LiveOutValue
+ : computeLiveInValueImpl(BB, BBInfo);
+ }
+
+ void clear() { BBInfos.clear(); }
+
+private:
+ Value *computeLiveInValueImpl(BasicBlock *BB, BBValueInfo &BBInfo);
+};
+
+Value *XXX::computeLiveInValueImpl(BasicBlock *BB, BBValueInfo &BBInfo) {
+ if (BBInfo.LiveInValue)
+ return BBInfo.LiveInValue;
+
+ Value *V = DT.isReachableFromEntry(BB) && !PredCache.get(BB).empty()
+ ? computeLiveOutValue(DT.getNode(BB)->getIDom()->getBlock())
+ : UndefValue::get(ValTy);
+
+ // The call to ComputeValue can insert new entries into the map:
+ // assume BBInfos shouldn't grow due to [1] above and BBInfo reference is
+ // valid.
+ assert(&BBInfo == &BBInfos[BB] && "Map shouldn't grow!");
+ BBInfo.LiveInValue = V;
+ return V;
+};
+#endif
+
struct BBValueInfo {
Value *LiveInValue = nullptr;
Value *LiveOutValue = nullptr;
@@ -140,7 +185,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
IDF.calculate(IDFBlocks);
// Reserve sufficient buckets to prevent map growth. [1]
- BBInfos.init(LiveInBlocks.size() + DefBlocks.size());
+ BBInfos.reserve(LiveInBlocks.size() + DefBlocks.size());
for (auto [BB, V] : R.Defines)
BBInfos[BB].LiveOutValue = V;
@@ -156,26 +201,44 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
// IsLiveOut indicates whether we are computing live-out values (true) or
// live-in values (false).
- std::function<Value *(BasicBlock *, bool)> ComputeValue =
- [&](BasicBlock *BB, bool IsLiveOut) -> Value * {
- auto &BBInfo = BBInfos[BB];
-
- if (IsLiveOut && BBInfo.LiveOutValue)
- return BBInfo.LiveOutValue;
-
- if (BBInfo.LiveInValue)
- return BBInfo.LiveInValue;
-
- Value *V = DT->isReachableFromEntry(BB) && !PredCache.get(BB).empty()
- ? ComputeValue(DT->getNode(BB)->getIDom()->getBlock(),
- /*IsLiveOut=*/true)
- : UndefValue::get(R.Ty);
-
- // The call to ComputeValue can insert new entries into the map:
- // assume BBInfos shouldn't grow due to [1] above and BBInfo reference is
- // valid.
- assert(&BBInfo == &BBInfos[BB] && "Map shouldn't grow!");
- BBInfo.LiveInValue = V;
+ auto ComputeValue = [&](BasicBlock *BB, bool IsLiveOut) -> Value * {
+
+ auto *BBInfo = &BBInfos[BB];
+
+ if (IsLiveOut && BBInfo->LiveOutValue)
+ return BBInfo->LiveOutValue;
+
+ if (BBInfo->LiveInValue)
+ return BBInfo->LiveInValue;
+
+ SmallVector<BBValueInfo *, 4> Stack = {BBInfo};
+ Value *V = nullptr;
+
+ while (DT->isReachableFromEntry(BB) && !PredCache.get(BB).empty() &&
+ (BB = DT->getNode(BB)->getIDom()->getBlock())) {
+ BBInfo = &BBInfos[BB];
+
+ if (BBInfo->LiveOutValue) {
+ V = BBInfo->LiveOutValue;
+ break;
+ }
+
+ if (BBInfo->LiveInValue) {
+ V = BBInfo->LiveInValue;
+ break;
+ }
+
+ Stack.emplace_back(BBInfo);
+ }
+
+ if (!V)
+ V = UndefValue::get(R.Ty);
+
+ for (auto *BBInfo : Stack)
+ // Loop above can insert new entries into the BBInfos map: assume the
+ // map shouldn't grow due to [1] and BBInfo references are valid.
+ BBInfo->LiveInValue = V;
+
return V;
};
>From 69d87e4a94dbbe7a0e7cdde69f267ee1e5c5503b Mon Sep 17 00:00:00 2001
From: Valery Pykhtin <valery.pykhtin at amd.com>
Date: Thu, 20 Mar 2025 06:35:58 +0000
Subject: [PATCH 8/8] remove dead code
---
llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp | 50 +-------------------
1 file changed, 2 insertions(+), 48 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
index fca9a2b24c56d..da2686063ad1b 100644
--- a/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
+++ b/llvm/lib/Transforms/Utils/SSAUpdaterBulk.cpp
@@ -102,51 +102,6 @@ ComputeLiveInBlocks(const SmallPtrSetImpl<BasicBlock *> &UsingBlocks,
}
}
-#if 0
-struct XXX {
- struct BBValueInfo {
- Value *LiveInValue = nullptr;
- Value *LiveOutValue = nullptr;
- };
-
- DominatorTree &DT;
- PredIteratorCache &PredCache;
- Type *ValTy;
- DenseMap<BasicBlock *, BBValueInfo> BBInfos;
-
- Value *computeLiveInValue(BasicBlock *BB) {
- return computeLiveInValueImpl(BB, BBInfos[BB]);
- }
-
- Value *computeLiveOutValue(BasicBlock *BB) {
- auto &BBInfo = BBInfos[BB];
- return BBInfo.LiveOutValue ? BBInfo.LiveOutValue
- : computeLiveInValueImpl(BB, BBInfo);
- }
-
- void clear() { BBInfos.clear(); }
-
-private:
- Value *computeLiveInValueImpl(BasicBlock *BB, BBValueInfo &BBInfo);
-};
-
-Value *XXX::computeLiveInValueImpl(BasicBlock *BB, BBValueInfo &BBInfo) {
- if (BBInfo.LiveInValue)
- return BBInfo.LiveInValue;
-
- Value *V = DT.isReachableFromEntry(BB) && !PredCache.get(BB).empty()
- ? computeLiveOutValue(DT.getNode(BB)->getIDom()->getBlock())
- : UndefValue::get(ValTy);
-
- // The call to ComputeValue can insert new entries into the map:
- // assume BBInfos shouldn't grow due to [1] above and BBInfo reference is
- // valid.
- assert(&BBInfo == &BBInfos[BB] && "Map shouldn't grow!");
- BBInfo.LiveInValue = V;
- return V;
-};
-#endif
-
struct BBValueInfo {
Value *LiveInValue = nullptr;
Value *LiveOutValue = nullptr;
@@ -202,8 +157,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
// IsLiveOut indicates whether we are computing live-out values (true) or
// live-in values (false).
auto ComputeValue = [&](BasicBlock *BB, bool IsLiveOut) -> Value * {
-
- auto *BBInfo = &BBInfos[BB];
+ auto *BBInfo = &BBInfos[BB];
if (IsLiveOut && BBInfo->LiveOutValue)
return BBInfo->LiveOutValue;
@@ -213,7 +167,7 @@ void SSAUpdaterBulk::RewriteAllUses(DominatorTree *DT,
SmallVector<BBValueInfo *, 4> Stack = {BBInfo};
Value *V = nullptr;
-
+
while (DT->isReachableFromEntry(BB) && !PredCache.get(BB).empty() &&
(BB = DT->getNode(BB)->getIDom()->getBlock())) {
BBInfo = &BBInfos[BB];
More information about the llvm-commits
mailing list