[JumpThreading] Fix an incorrect Modified status

[dstenb]

Summary

This fixes PR47297.

When ProcessBlock() was able to constant fold the terminator's
condition, but not do any more transformations, the function would
return false, which would lead to the JumpThreading pass returning an
incorrect modified status. This patch makes so that ProcessBlock()
returns true in such cases. This will trigger an unnecessary invocation
of ProcessBlock() in such cases, but this should be rare to occur.

This was caught using the check introduced by D80916.

[efriedma]

I assume that we in such cases don't want to return true from ProcessBlock(), which would trigger another iteration of the function.

Theoretically, it's unnecessary, so we'd want to avoid it the extra analysis cost. But in practice this should be really rare, so I'd prefer to keep the code simpler and continue using a bool return.

[dstenb]

Continue using a bool return instead of a tri-state.

[efriedma]

LGTM

[dstenb]

Thanks for the review!

And for reference, I compiled 100,000 Csmith programs for x86-64 and I only ran into this case 15 times.

llvm/lib/Transforms/Scalar/JumpThreading.cpp

if (BranchInst *BI = dyn_cast<BranchInst>(Terminator)) {
     // Can't thread indirect branch with no successors.
     if (IB->getNumSuccessors() == 0) return false;
     Condition = IB->getAddress()->stripPointerCasts();
     Preference = WantBlockAddress;
   } else {
     return false; // 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<Instruction>(Condition)) {
     Value *SimpleVal =
         ConstantFoldInstruction(I, BB->getModule()->getDataLayout(), TLI);
     if (SimpleVal) {
       I->replaceAllUsesWith(SimpleVal);
       if (isInstructionTriviallyDead(I, TLI))
         I->eraseFromParent();
       Condition = SimpleVal;
+      ConstantFolded = true;
     }
   }
 
   // If the terminator is branching on an undef or freeze undef, we can pick any
   // of the successors to branch to.  Let GetBestDestForJumpOnUndef decide.
   auto *FI = dyn_cast<FreezeInst>(Condition);
   if (isa<UndefValue>(Condition) ||
       (FI && isa<UndefValue>(FI->getOperand(0)) && FI->hasOneUse())) {
bool JumpThreadingPass::ProcessBlock(BasicBlock *BB) {
 
   Instruction *CondInst = dyn_cast<Instruction>(Condition);
 
   // All the rest of our checks depend on the condition being an instruction.
   if (!CondInst) {
     // FIXME: Unify this with code below.
     if (ProcessThreadableEdges(Condition, BB, Preference, Terminator))
       return true;
-    return false;
+    return ConstantFolded;
   }
 
   if (CmpInst *CondCmp = dyn_cast<CmpInst>(CondInst)) {
     // If we're branching on a conditional, LVI might be able to determine
     // it's value at the branch instruction.  We only handle comparisons
     // against a constant at this time.
     // TODO: This should be extended to handle switches as well.
     BranchInst *CondBr = dyn_cast<BranchInst>(BB->getTerminator());

llvm/test/Transforms/JumpThreading/constant-fold-status.ll

+; 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()