From 73ce9611e854d0ed7a27b561e2b02678da0392a5 Mon Sep 17 00:00:00 2001 From: Guozhi Wei Date: Wed, 12 Feb 2025 14:51:21 -0800 Subject: [PATCH 1/3] [JumpThreading] Remove deleted BB from Unreachable Although an unreachable BB is skipped by processBlock, its successor can still be handled by processBlock, and maybeMergeBasicBlockIntoOnlyPred may merge the two BBs and delete the unreachable BB. Then the garbage pointer is left in Unreachable set. This patch removes the invalid BB pointer from the Unreachable set. --- llvm/include/llvm/Transforms/Scalar/JumpThreading.h | 4 ++++ llvm/lib/Transforms/Scalar/JumpThreading.cpp | 8 +++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/llvm/include/llvm/Transforms/Scalar/JumpThreading.h b/llvm/include/llvm/Transforms/Scalar/JumpThreading.h index 7d11fc0ad6938..84292c716a0a9 100644 --- a/llvm/include/llvm/Transforms/Scalar/JumpThreading.h +++ b/llvm/include/llvm/Transforms/Scalar/JumpThreading.h @@ -94,6 +94,10 @@ class JumpThreadingPass : public PassInfoMixin { SmallPtrSet LoopHeaders; #endif + // JumpThreading must not processes blocks unreachable from entry. It's a + // waste of compute time and can potentially lead to hangs. + SmallPtrSet Unreachable; + unsigned BBDupThreshold; unsigned DefaultBBDupThreshold; diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp index 7b221a814aabd..dfa49ba183779 100644 --- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp +++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp @@ -307,12 +307,11 @@ bool JumpThreadingPass::runImpl(Function &F_, FunctionAnalysisManager *FAM_, else BBDupThreshold = DefaultBBDupThreshold; - // JumpThreading must not processes blocks unreachable from entry. It's a - // waste of compute time and can potentially lead to hangs. - SmallPtrSet Unreachable; assert(DTU && "DTU isn't passed into JumpThreading before using it."); assert(DTU->hasDomTree() && "JumpThreading relies on DomTree to proceed."); DominatorTree &DT = DTU->getDomTree(); + + Unreachable.clear(); for (auto &BB : *F) if (!DT.isReachableFromEntry(&BB)) Unreachable.insert(&BB); @@ -1902,6 +1901,9 @@ bool JumpThreadingPass::maybeMergeBasicBlockIntoOnlyPred(BasicBlock *BB) { LVI->eraseBlock(SinglePred); MergeBasicBlockIntoOnlyPred(BB, DTU.get()); + if (Unreachable.count(SinglePred) && DTU->isBBPendingDeletion(SinglePred)) + Unreachable.erase(SinglePred); + // Now that BB is merged into SinglePred (i.e. SinglePred code followed by // BB code within one basic block `BB`), we need to invalidate the LVI // information associated with BB, because the LVI information need not be From 30a97d4acc8c02e47944b4c09ceeab78a5df10b3 Mon Sep 17 00:00:00 2001 From: Guozhi Wei Date: Thu, 13 Feb 2025 18:07:07 -0800 Subject: [PATCH 2/3] [JumpThreading] Bail out maybeMergeBasicBlockIntoOnlyPred if SinglePred is in Unreachable. Merge SinglePred and its successor may delete SinglePred, and causes garbage pointer in Unreachable. Bail out maybeMergeBasicBlockIntoOnlyPred if we found this case. --- llvm/lib/Transforms/Scalar/JumpThreading.cpp | 8 +++++--- llvm/test/Transforms/JumpThreading/pr62908.ll | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp index dfa49ba183779..9cae65bbdcfbc 100644 --- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp +++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp @@ -1894,6 +1894,11 @@ bool JumpThreadingPass::maybeMergeBasicBlockIntoOnlyPred(BasicBlock *BB) { SinglePred == BB || hasAddressTakenAndUsed(BB)) return false; + // MergeBasicBlockIntoOnlyPred may delete SinglePred, we need to avoid + // deleting a BB pointer from Unreachable. + if (Unreachable.count(SinglePred)) + return false; + // If SinglePred was a loop header, BB becomes one. if (LoopHeaders.erase(SinglePred)) LoopHeaders.insert(BB); @@ -1901,9 +1906,6 @@ bool JumpThreadingPass::maybeMergeBasicBlockIntoOnlyPred(BasicBlock *BB) { LVI->eraseBlock(SinglePred); MergeBasicBlockIntoOnlyPred(BB, DTU.get()); - if (Unreachable.count(SinglePred) && DTU->isBBPendingDeletion(SinglePred)) - Unreachable.erase(SinglePred); - // Now that BB is merged into SinglePred (i.e. SinglePred code followed by // BB code within one basic block `BB`), we need to invalidate the LVI // information associated with BB, because the LVI information need not be diff --git a/llvm/test/Transforms/JumpThreading/pr62908.ll b/llvm/test/Transforms/JumpThreading/pr62908.ll index 4c389ee040b90..c41489a3cb5e9 100644 --- a/llvm/test/Transforms/JumpThreading/pr62908.ll +++ b/llvm/test/Transforms/JumpThreading/pr62908.ll @@ -5,7 +5,7 @@ define i32 @test() { ; CHECK-LABEL: define i32 @test() { -; CHECK-NEXT: end: +; CHECK: end: ; CHECK-NEXT: ret i32 0 ; entry: From 6df69dccca926c8e138f8578b2bf011cb1b4d2b7 Mon Sep 17 00:00:00 2001 From: Guozhi Wei Date: Tue, 25 Feb 2025 16:50:43 -0800 Subject: [PATCH 3/3] Regenerate the test result so the whole IR is included. --- llvm/test/Transforms/JumpThreading/pr62908.ll | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/llvm/test/Transforms/JumpThreading/pr62908.ll b/llvm/test/Transforms/JumpThreading/pr62908.ll index c41489a3cb5e9..cfb647c509f8e 100644 --- a/llvm/test/Transforms/JumpThreading/pr62908.ll +++ b/llvm/test/Transforms/JumpThreading/pr62908.ll @@ -5,6 +5,17 @@ define i32 @test() { ; CHECK-LABEL: define i32 @test() { +; CHECK-NEXT: join.thread: +; CHECK-NEXT: br label [[END:%.*]] +; CHECK: unreachable: +; CHECK-NEXT: [[SH_PROM:%.*]] = zext i32 -1 to i64 +; CHECK-NEXT: [[SHL:%.*]] = shl nsw i64 -1, [[SH_PROM]] +; CHECK-NEXT: [[CONV:%.*]] = trunc i64 [[SHL]] to i32 +; CHECK-NEXT: br label [[JOIN:%.*]] +; CHECK: join: +; CHECK-NEXT: [[PHI:%.*]] = phi i32 [ [[CONV]], [[UNREACHABLE:%.*]] ] +; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[PHI]], 0 +; CHECK-NEXT: br i1 [[CMP]], label [[END]], label [[END]] ; CHECK: end: ; CHECK-NEXT: ret i32 0 ;