[llvm] [Bitcode] Fix incomplete deduplication of PHI entries (PR #162860)
Florian Stamer via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 10 07:47:50 PDT 2025
https://github.com/FlStamerGS updated https://github.com/llvm/llvm-project/pull/162860
>From 650b53d2818d2dc5431d703dccce639e1a693c68 Mon Sep 17 00:00:00 2001
From: Florian Stamer <florian.stamer at guardsquare.com>
Date: Fri, 10 Oct 2025 16:15:13 +0200
Subject: [PATCH] [Bitcode] Fix incomplete deduplication of PHI entries
---
llvm/include/llvm/IR/BasicBlock.h | 4 ++
llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 9 +--
llvm/lib/IR/BasicBlock.cpp | 20 ++++++
llvm/test/Bitcode/constexpr-to-instr-dups.ll | 27 ++++++++
.../Bitcode/constexpr-to-instr-dups.ll.bc | Bin 1436 -> 1572 bytes
llvm/unittests/IR/BasicBlockTest.cpp | 58 ++++++++++++++++++
6 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index 533808e0666d5..dfd44d2243bfb 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -703,6 +703,10 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
/// block \p New instead of to it.
LLVM_ABI void replaceSuccessorsPhiUsesWith(BasicBlock *New);
+ /// Update all phi nodes in this basic block by deduplicating the references
+ /// to \p BB.
+ LLVM_ABI void deduplicatePhiUsesOf(BasicBlock *BB);
+
/// Return true if this basic block is an exception handling block.
bool isEHPad() const { return getFirstNonPHIIt()->isEHPad(); }
diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
index aaee1f0a7687c..88422aa0c7a8c 100644
--- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -6105,18 +6105,14 @@ Error BitcodeReader::parseFunctionBody(Function *F) {
// seen value here, to avoid expanding a constant expression multiple
// times.
auto It = Args.find(BB);
- BasicBlock *EdgeBB = ConstExprEdgeBBs.lookup({BB, CurBB});
if (It != Args.end()) {
- // If this predecessor was also replaced with a constexpr basic
- // block, it must be de-duplicated.
- if (!EdgeBB) {
- PN->addIncoming(It->second, BB);
- }
+ PN->addIncoming(It->second, BB);
continue;
}
// If there already is a block for this edge (from a different phi),
// use it.
+ BasicBlock *EdgeBB = ConstExprEdgeBBs.lookup({BB, CurBB});
if (!EdgeBB) {
// Otherwise, use a temporary block (that we will discard if it
// turns out to be unnecessary).
@@ -6944,6 +6940,7 @@ Error BitcodeReader::parseFunctionBody(Function *F) {
BranchInst::Create(To, EdgeBB);
From->getTerminator()->replaceSuccessorWith(To, EdgeBB);
To->replacePhiUsesWith(From, EdgeBB);
+ To->deduplicatePhiUsesOf(EdgeBB);
EdgeBB->moveBefore(To);
}
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 3642e935397cb..49b09bd959335 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -658,6 +658,26 @@ void BasicBlock::replaceSuccessorsPhiUsesWith(BasicBlock *New) {
this->replaceSuccessorsPhiUsesWith(this, New);
}
+void BasicBlock::deduplicatePhiUsesOf(BasicBlock *BB) {
+ // N.B. This might not be a complete BasicBlock, so don't assume
+ // that it ends with a non-phi instruction.
+ for (Instruction &I : *this) {
+ PHINode *PN = dyn_cast<PHINode>(&I);
+ if (!PN)
+ break;
+ // Since the order of basic blocks in a PHINode are sorted we can
+ // use the index of \p BB + 1 as the first index to check for duplicates.
+ unsigned Idx = PN->getBasicBlockIndex(BB) + 1;
+ while (Idx < PN->getNumIncomingValues()) {
+ BasicBlock *DuplicateBB = PN->getIncomingBlock(Idx);
+ if (DuplicateBB != BB) {
+ break;
+ }
+ PN->removeIncomingValue(Idx);
+ }
+ }
+}
+
bool BasicBlock::isLandingPad() const {
return isa<LandingPadInst>(getFirstNonPHIIt());
}
diff --git a/llvm/test/Bitcode/constexpr-to-instr-dups.ll b/llvm/test/Bitcode/constexpr-to-instr-dups.ll
index dee9490b616fa..d439a6dcf0c7b 100644
--- a/llvm/test/Bitcode/constexpr-to-instr-dups.ll
+++ b/llvm/test/Bitcode/constexpr-to-instr-dups.ll
@@ -28,3 +28,30 @@ cont:
[1, %nonconst]
ret i32 %res
}
+
+define i32 @test_multiple_phis(i32 %arg) {
+entry:
+ switch i32 %arg, label %cont [
+ i32 1, label %cont
+ i32 2, label %nonconst
+ ]
+
+nonconst: ; preds = %entry
+ %cmp = icmp ne i32 %arg, 2
+ br i1 %cmp, label %cont, label %cont
+
+; CHECK-LABEL: phi.constexpr:
+; CHECK-NEXT: %constexpr = ptrtoint ptr @foo to i32
+; CHECK-NEXT: %constexpr1 = or i32 %constexpr, 5
+; CHECK-NEXT: br label %cont
+
+
+; CHECK-LABEL: cont:
+; CHECK-NEXT: %phi1 = phi i32 [ 0, %phi.constexpr ], [ 1, %nonconst ], [ 1, %nonconst ]
+; CHECK-NEXT: %phi2 = phi i32 [ %constexpr1, %phi.constexpr ], [ 1, %nonconst ], [ 1, %nonconst ]
+; CHECK-NEXT: ret i32 %phi2
+cont: ; preds = %nonconst, %nonconst, %entry, %entry
+ %phi1 = phi i32 [ 0, %entry ], [ 0, %entry ], [ 1, %nonconst ], [ 1, %nonconst ]
+ %phi2 = phi i32 [ or (i32 ptrtoint (ptr @foo to i32), i32 5), %entry ], [ or (i32 ptrtoint (ptr @foo to i32), i32 5), %entry ], [ 1, %nonconst ], [ 1, %nonconst ]
+ ret i32 %phi2
+}
diff --git a/llvm/test/Bitcode/constexpr-to-instr-dups.ll.bc b/llvm/test/Bitcode/constexpr-to-instr-dups.ll.bc
index 7897f51322fcc3e9ce738c66fc90d36dde5c1fb1..15a3eeb449d8780f44eedf7707916aa474a7fff3 100644
GIT binary patch
delta 478
zcmbQky at Y3ih~V1aTl~2L8+;fU7#I|J7#OlAs%F>o0y#!XCpZ+6+!z$KRc7!gssaf~
zb)MEG6B3M4ViZ)8n;xvNWHK;tG7>OgHDEGu7XWHu at Z#qXoaE5fGlhZC#A(KytjF7{
z{q!F{{3Q1GuGjA8C6(PV?dM~2%oy4D7$kytTR>W1pn$=7m9r3p2c`HNlA0YJZf9U<
zh~U}y^F0%z`sA0)vl!VY&tY-mNs<AnJuK6xDBv{t6^mT`Gy{$^#SE+$eFV!GJsz7h
zUG})lDbRG;$IO6XVj`EbiwD%EL<I&0Ss+`O#aRdBXci!zBBaqI$l%Q|<&ehVBB1<X
znS(%M1$oS6TqVvJaX1+S2x&AhSu_gsOkrV5U<hDfm2P1Mh8P!6?=pzJ^%xd!bFg6#
zl4kxW(~--}EFcL~%D_;lz`(!^q>cCllv)_KFljL8f`l1jfgum_wGi`^N)SISKfffk
zxCDgbb4zndG7EB2;|nq}iw({64D<|=^Ye;JQY#9IbW8GeGlA?PUA~mkf?~a#9FT4x
F0071Sd4T``
delta 341
zcmZ3&GlzSEh~VDeTh?<2Hux|yFfb_cFfe#bRL#x=(hN#zECQRHnobG?0f`BPhXOpf
zIMtkwu{s&4aq!JjW at uJYVP*_wP-ZA*0qJG%;^z>Y<j~eLg at Mt;N#ffKj?7QfTN#+y
z8F*H*Ie`?wfCWP*FH`GGCI*Hi!_A6}@0l2NCkwL7l5A%^Z?UJ@`i8URiNlstnyq`B
zEqj=4PcU0gUc=(V6C?rBe^{nbk%wvWFBZ9aH3No;iJZ<Z9_kJpe#VlA6b#O at D)e6F
z at Kg3>4-hyjk_ff0P=SGg8OS!`6Hsbl+yJx|0%C!l6b3Sdn5Wc&MAGu}OHzwV49)cn
z^vo2D4b3bqO$|+q%~DLwl2QyTlFSVaO-#*93 at nok4UJ7qL`}?0ER)PFL3;FZav)|h
F004YlPpSX_
diff --git a/llvm/unittests/IR/BasicBlockTest.cpp b/llvm/unittests/IR/BasicBlockTest.cpp
index 1f726dbfe2325..4e7277a5fc84c 100644
--- a/llvm/unittests/IR/BasicBlockTest.cpp
+++ b/llvm/unittests/IR/BasicBlockTest.cpp
@@ -94,6 +94,64 @@ TEST(BasicBlockTest, PhiRange) {
}
}
+TEST(BasicBlockTest, DeduplicatePhiEntries) {
+ LLVMContext Context;
+
+ // Create the main block.
+ std::unique_ptr<BasicBlock> BB(BasicBlock::Create(Context));
+
+ // Create some predecessors of it.
+ std::unique_ptr<BasicBlock> BB1(BasicBlock::Create(Context));
+ BranchInst::Create(BB.get(), BB1.get());
+
+ // Make sure this doesn't crash if there are no phis.
+ int PhiCount = 0;
+ for (auto &PN : BB->phis()) {
+ (void)PN;
+ PhiCount++;
+ }
+ ASSERT_EQ(PhiCount, 0) << "empty block should have no phis";
+
+ // Make it a cycle.
+ auto *BI = BranchInst::Create(BB.get(), BB.get());
+
+ // Now insert some PHI nodes.
+ auto *Int32Ty = Type::getInt32Ty(Context);
+ auto *P1 = PHINode::Create(Int32Ty, /*NumReservedValues*/ 3, "phi.1", BI);
+ auto *P2 = PHINode::Create(Int32Ty, /*NumReservedValues*/ 3, "phi.2", BI);
+
+ // Some non-PHI nodes.
+ auto *Sum = BinaryOperator::CreateAdd(P1, P2, "sum", BI);
+
+ // Now wire up the incoming values that are interesting.
+ P1->addIncoming(P2, BB.get());
+ P2->addIncoming(Sum, BB.get());
+
+ // Wire up the rest of the incoming values, we want duplicate entries.
+ for (auto &PN : BB->phis()) {
+ auto *SomeValue = UndefValue::get(Int32Ty);
+ PN.addIncoming(SomeValue, BB1.get());
+ PN.addIncoming(SomeValue, BB1.get());
+ }
+
+ // Check that the phi nodes have 3 incoming values.
+ EXPECT_EQ(P1->getNumIncomingValues(), 3u);
+ EXPECT_EQ(P2->getNumIncomingValues(), 3u);
+
+ // Deduplicate the incoming values for BB1.
+ BB->deduplicatePhiUsesOf(BB1.get());
+
+ // Check that the phi nodes have 2 incoming values.
+ EXPECT_EQ(P1->getNumIncomingValues(), 2u);
+ EXPECT_EQ(P2->getNumIncomingValues(), 2u);
+
+ // Check that the incoming values are unique.
+ for (auto &PN : BB->phis()) {
+ EXPECT_EQ(BB.get(), PN.getIncomingBlock(0));
+ EXPECT_EQ(BB1.get(), PN.getIncomingBlock(1));
+ }
+}
+
#define CHECK_ITERATORS(Range1, Range2) \
EXPECT_EQ(std::distance(Range1.begin(), Range1.end()), \
std::distance(Range2.begin(), Range2.end())); \
More information about the llvm-commits
mailing list