Index: llvm/include/llvm/Transforms/Scalar/JumpThreading.h =================================================================== --- llvm/include/llvm/Transforms/Scalar/JumpThreading.h +++ llvm/include/llvm/Transforms/Scalar/JumpThreading.h @@ -108,8 +108,14 @@ BPI.reset(); } + enum class ProcessBlockResult { + Unmodified, + Modified, + Threaded, + }; + void FindLoopHeaders(Function &F); - bool ProcessBlock(BasicBlock *BB); + ProcessBlockResult ProcessBlock(BasicBlock *BB); bool MaybeMergeBasicBlockIntoOnlyPred(BasicBlock *BB); void UpdateSSA(BasicBlock *BB, BasicBlock *NewBB, DenseMap &ValueMapping); Index: llvm/lib/Transforms/Scalar/JumpThreading.cpp =================================================================== --- llvm/lib/Transforms/Scalar/JumpThreading.cpp +++ llvm/lib/Transforms/Scalar/JumpThreading.cpp @@ -406,8 +406,12 @@ for (auto &BB : F) { if (Unreachable.count(&BB)) continue; - while (ProcessBlock(&BB)) // Thread all of the branches we can over BB. - Changed = true; + // Thread all of the branches we can over BB. + ProcessBlockResult Result; + do { + Result = ProcessBlock(&BB); + Changed |= Result != ProcessBlockResult::Unmodified; + } while (Result == ProcessBlockResult::Threaded); // Jump threading may have introduced redundant debug values into BB // which should be removed. @@ -997,26 +1001,27 @@ /// ProcessBlock - If there are any predecessors whose control can be threaded /// through to a successor, transform them now. -bool JumpThreadingPass::ProcessBlock(BasicBlock *BB) { +JumpThreadingPass::ProcessBlockResult +JumpThreadingPass::ProcessBlock(BasicBlock *BB) { // If the block is trivially dead, just return and let the caller nuke it. // This simplifies other transformations. if (DTU->isBBPendingDeletion(BB) || (pred_empty(BB) && BB != &BB->getParent()->getEntryBlock())) - return false; + return ProcessBlockResult::Unmodified; // If this block has a single predecessor, and if that pred has a single // successor, merge the blocks. This encourages recursive jump threading // because now the condition in this block can be threaded through // predecessors of our predecessor block. if (MaybeMergeBasicBlockIntoOnlyPred(BB)) - return true; + return ProcessBlockResult::Threaded; if (TryToUnfoldSelectInCurrBB(BB)) - return true; + return ProcessBlockResult::Threaded; // Look if we can propagate guards to predecessors. if (HasGuards && ProcessGuards(BB)) - return true; + return ProcessBlockResult::Threaded; // What kind of constant we're looking for. ConstantPreference Preference = WantInteger; @@ -1027,19 +1032,24 @@ Instruction *Terminator = BB->getTerminator(); if (BranchInst *BI = dyn_cast(Terminator)) { // Can't thread an unconditional jump. - if (BI->isUnconditional()) return false; + if (BI->isUnconditional()) + return ProcessBlockResult::Unmodified; Condition = BI->getCondition(); } else if (SwitchInst *SI = dyn_cast(Terminator)) { Condition = SI->getCondition(); } else if (IndirectBrInst *IB = dyn_cast(Terminator)) { // Can't thread indirect branch with no successors. - if (IB->getNumSuccessors() == 0) return false; + if (IB->getNumSuccessors() == 0) + return ProcessBlockResult::Unmodified; Condition = IB->getAddress()->stripPointerCasts(); Preference = WantBlockAddress; } else { - return false; // Must be an invoke or callbr. + return ProcessBlockResult::Unmodified; // Must be an invoke or callbr. } + // Keep track if we constant folded the condition in this invocation. + bool ConstantFolded = false; + // Run constant folding to see if we can reduce the condition to a simple // constant. if (Instruction *I = dyn_cast(Condition)) { @@ -1050,6 +1060,7 @@ if (isInstructionTriviallyDead(I, TLI)) I->eraseFromParent(); Condition = SimpleVal; + ConstantFolded = true; } } @@ -1078,7 +1089,7 @@ DTU->applyUpdatesPermissive(Updates); if (FI) FI->eraseFromParent(); - return true; + return ProcessBlockResult::Threaded; } // If the terminator of this block is branching on a constant, simplify the @@ -1090,7 +1101,7 @@ << '\n'); ++NumFolds; ConstantFoldTerminator(BB, true, nullptr, DTU); - return true; + return ProcessBlockResult::Threaded; } Instruction *CondInst = dyn_cast(Condition); @@ -1099,8 +1110,9 @@ if (!CondInst) { // FIXME: Unify this with code below. if (ProcessThreadableEdges(Condition, BB, Preference, Terminator)) - return true; - return false; + return ProcessBlockResult::Threaded; + return ConstantFolded ? ProcessBlockResult::Modified + : ProcessBlockResult::Unmodified; } if (CmpInst *CondCmp = dyn_cast(CondInst)) { @@ -1145,19 +1157,19 @@ } DTU->applyUpdatesPermissive( {{DominatorTree::Delete, BB, ToRemoveSucc}}); - return true; + return ProcessBlockResult::Threaded; } // We did not manage to simplify this branch, try to see whether // CondCmp depends on a known phi-select pattern. if (TryToUnfoldSelect(CondCmp, BB)) - return true; + return ProcessBlockResult::Threaded; } } if (SwitchInst *SI = dyn_cast(BB->getTerminator())) if (TryToUnfoldSelect(SI, BB)) - return true; + return ProcessBlockResult::Threaded; // Check for some cases that are worth simplifying. Right now we want to look // for loads that are used by a switch or by the condition for the branch. If @@ -1177,7 +1189,7 @@ // more complex comparisons. if (LoadInst *LoadI = dyn_cast(SimplifyValue)) if (SimplifyPartiallyRedundantLoad(LoadI)) - return true; + return ProcessBlockResult::Threaded; // Before threading, try to propagate profile data backwards: if (PHINode *PN = dyn_cast(CondInst)) @@ -1188,7 +1200,7 @@ // a PHI node in the current block. If we can prove that any predecessors // compute a predictable value based on a PHI node, thread those predecessors. if (ProcessThreadableEdges(CondInst, BB, Preference, Terminator)) - return true; + return ProcessBlockResult::Threaded; // If this is an otherwise-unfoldable branch on a phi node or freeze(phi) in // the current block, see if we can simplify. @@ -1197,19 +1209,22 @@ : CondInst); if (PN && PN->getParent() == BB && isa(BB->getTerminator())) - return ProcessBranchOnPHI(PN); + return ProcessBranchOnPHI(PN) ? ProcessBlockResult::Threaded + : ProcessBlockResult::Unmodified; // If this is an otherwise-unfoldable branch on a XOR, see if we can simplify. if (CondInst->getOpcode() == Instruction::Xor && CondInst->getParent() == BB && isa(BB->getTerminator())) - return ProcessBranchOnXOR(cast(CondInst)); + return ProcessBranchOnXOR(cast(CondInst)) + ? ProcessBlockResult::Threaded + : ProcessBlockResult::Unmodified; // Search for a stronger dominating condition that can be used to simplify a // conditional branch leaving BB. if (ProcessImpliedCondition(BB)) - return true; + return ProcessBlockResult::Threaded; - return false; + return ProcessBlockResult::Unmodified; } bool JumpThreadingPass::ProcessImpliedCondition(BasicBlock *BB) { Index: llvm/test/Transforms/JumpThreading/constant-fold-status.ll =================================================================== --- /dev/null +++ llvm/test/Transforms/JumpThreading/constant-fold-status.ll @@ -0,0 +1,28 @@ +; RUN: opt -jump-threading < %s -S -o - | FileCheck %s + +; Reproducer for PR47297. + +; The pass did previously not report a correct Modified status in the case +; where a terminator's condition was successfully constant folded, but there +; were no other transformations done. This was caught by the pass return +; status check that is hidden under EXPENSIVE_CHECKS. + +; CHECK-LABEL: entry: +; CHECK-NEXT: br i1 icmp eq (i32 ptrtoint (i16* @a to i32), i32 0), label %overflow, label %cont + +@a = internal global i16 0 + +define void @foo(i16 %d) { +entry: + %.not = icmp eq i16 zext (i1 icmp ne (i32 ptrtoint (i16* @a to i32), i32 0) to i16), 0 + br i1 %.not, label %overflow, label %cont + +overflow: ; preds = %entry + call void @bar() + br label %cont + +cont: ; preds = %overflow, %entry + ret void +} + +declare void @bar()