From 316884c4fa5ce092308ce19206d5abdb8f701563 Mon Sep 17 00:00:00 2001 From: Justin Lebar Date: Tue, 26 May 2026 09:16:02 -0700 Subject: [PATCH] [JumpThreading] NFC: Add comment referencing #199408. Leave a breadcrumb indicating that not `freeze`'ing a select's condition when turning it into a conditional branch is unsound (unless we can prove that the cond is never poison). Nonetheless this doesn't seem to cause real-world problems afaict. --- llvm/lib/Transforms/Scalar/JumpThreading.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp index 415136b612ac2..602228bcb8289 100644 --- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp +++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp @@ -2789,6 +2789,13 @@ void JumpThreadingPass::unfoldSelectInstr(BasicBlock *Pred, BasicBlock *BB, PredTerm->removeFromParent(); PredTerm->insertInto(NewBB, NewBB->end()); // Create a conditional branch and update PHI nodes. + // + // Note: Technically we should `freeze` the condition before using it in a + // conditional branch, unless we can prove it's not poison: select-on-on + // poison isn't UB, but branch-on-poison is. But doing this causes + // performance regressions, and we haven't been able to find a source-level + // correctness issue it fixes. See + // https://github.com/llvm/llvm-project/pull/199408#issuecomment-4545013881. auto *BI = CondBrInst::Create(SI->getCondition(), NewBB, BB, Pred); BI->applyMergedLocation(PredTerm->getDebugLoc(), SI->getDebugLoc()); BI->copyMetadata(*SI, {LLVMContext::MD_prof});