[PATCH] D34135: [JumpThreading] Use DT to avoid processing dead blocks

[Brian Rzycki via Phabricator via llvm-commits]

brzycki added a comment.

I have a patch (that relies on my fix from https://reviews.llvm.org/D42717) to also fix this issue with little additional overhead. Because we must flush all pending DT transactions before using LVI analysis we can also check if the block is unreachable from entry. If that's the case we can immediately pessimize threads across the block.  Here's the patch along with a modified version of https://reviews.llvm.org/D42717:

  diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
  index 87a1d5e487e..51c9923ff83 100644
  --- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp
  +++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp
  @@ -617,6 +617,10 @@ bool JumpThreadingPass::ComputeValueKnownInPredecessors(
       // "X < 4" and "X < 3" is known true but "X < 4" itself is not available.
       // Perhaps getConstantOnEdge should be smart enough to do this?
  
  +    auto &DT =
  +        DDT->flush(); // LVI analysis relies on an up-to-date DominatorTree.
  +    if (!DT.isReachableFromEntry(BB))
  +      return false; // Don't jumpthread blocks that will soon be dead.
       for (BasicBlock *P : predecessors(BB)) {
         // If the value is known by LazyValueInfo to be a constant in a
         // predecessor, use that information to try to thread this block.
  @@ -630,6 +634,10 @@ bool JumpThreadingPass::ComputeValueKnownInPredecessors(
  
     /// If I is a PHI node, then we know the incoming values for any constants.
     if (PHINode *PN = dyn_cast<PHINode>(I)) {
  +    auto &DT =
  +        DDT->flush(); // LVI analysis relies on an up-to-date DominatorTree.
  +    if (!DT.isReachableFromEntry(BB))
  +      return false; // Don't jumpthread blocks that will soon be dead.
       for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
         Value *InVal = PN->getIncomingValue(i);
         if (Constant *KC = getKnownConstant(InVal, Preference)) {
  @@ -769,6 +777,10 @@ bool JumpThreadingPass::ComputeValueKnownInPredecessors(
             if (!isa<Constant>(RHS))
               continue;
  
  +          auto &DT = DDT->flush(); // LVI analysis relies on an up-to-date
  +                                   // DominatorTree.
  +          if (!DT.isReachableFromEntry(BB))
  +            return false; // Don't jumpthread blocks that will soon be dead.
             LazyValueInfo::Tristate
               ResT = LVI->getPredicateOnEdge(Pred, LHS,
                                              cast<Constant>(RHS), PredBB, BB,
  @@ -792,6 +804,10 @@ bool JumpThreadingPass::ComputeValueKnownInPredecessors(
  
         if (!isa<Instruction>(CmpLHS) ||
             cast<Instruction>(CmpLHS)->getParent() != BB) {
  +        auto &DT =
  +            DDT->flush(); // LVI analysis relies on an up-to-date DominatorTree.
  +        if (!DT.isReachableFromEntry(BB))
  +          return false; // Don't jumpthread blocks that will soon be dead.
           for (BasicBlock *P : predecessors(BB)) {
             // If the value is known by LazyValueInfo to be a constant in a
             // predecessor, use that information to try to thread this block.
  @@ -820,6 +836,10 @@ bool JumpThreadingPass::ComputeValueKnownInPredecessors(
               match(CmpLHS, m_Add(m_Value(AddLHS), m_ConstantInt(AddConst)))) {
             if (!isa<Instruction>(AddLHS) ||
                 cast<Instruction>(AddLHS)->getParent() != BB) {
  +            auto &DT = DDT->flush(); // LVI analysis relies on an up-to-date
  +                                     // DominatorTree.
  +            if (!DT.isReachableFromEntry(BB))
  +              return false; // Don't jumpthread blocks that will soon be dead.
               for (BasicBlock *P : predecessors(BB)) {
                 // If the value is known by LazyValueInfo to be a ConstantRange in
                 // a predecessor, use that information to try to thread this
  @@ -900,6 +920,10 @@ bool JumpThreadingPass::ComputeValueKnownInPredecessors(
       }
     }
  
  +  auto &DT =
  +      DDT->flush(); // LVI analysis relies on an up-to-date DominatorTree.
  +  if (!DT.isReachableFromEntry(BB))
  +    return false; // Don't jumpthread blocks that will soon be dead.
     // If all else fails, see if LVI can figure out a constant value for us.
     Constant *CI = LVI->getConstant(V, BB, CxtI);
     if (Constant *KC = getKnownConstant(CI, Preference)) {
  @@ -1101,7 +1125,10 @@ bool JumpThreadingPass::ProcessBlock(BasicBlock *BB) {
         // unconditional. Because its no longer interesting as far as jump
         // threading is concerned.
         assert(CondBr->isConditional() && "Threading on unconditional terminator");
  -
  +      auto &DT =
  +          DDT->flush(); // LVI analysis relies on an up-to-date DominatorTree.
  +      if (!DT.isReachableFromEntry(BB))
  +        return false; // Don't jumpthread blocks that will soon be dead.
         LazyValueInfo::Tristate Ret =
           LVI->getPredicateAt(CondCmp->getPredicate(), CondCmp->getOperand(0),
                               CondConst, CondBr);
  @@ -2371,6 +2398,10 @@ bool JumpThreadingPass::TryToUnfoldSelect(CmpInst *CondCmp, BasicBlock *BB) {
       // Now check if one of the select values would allow us to constant fold the
       // terminator in BB. We don't do the transform if both sides fold, those
       // cases will be threaded in any case.
  +    auto &DT =
  +        DDT->flush(); // LVI analysis relies on an up-to-date DominatorTree.
  +    if (!DT.isReachableFromEntry(BB))
  +      return false; // Don't jumpthread blocks that will soon be dead.
       LazyValueInfo::Tristate LHSFolds =
           LVI->getPredicateOnEdge(CondCmp->getPredicate(), SI->getOperand(1),
                                   CondRHS, Pred, BB, CondCmp);

I have verified it does not crash on PR33357-lvi-recursion.ll, passes check-all and test-suite runs.


https://reviews.llvm.org/D34135