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

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added a comment.

In https://reviews.llvm.org/D34135#978053, @uabelho wrote:

> In https://reviews.llvm.org/D34135#977113, @brzycki wrote:
>
> > Hi @uabelho, in order to actually use the DT you have to perform a flush on DDT like so:
> >
> >   DominatorTree *DT = DDT->flush();
> >   !DT->isReachableFromEntry(BB);
> >   ...
> >
> >
> > This flush() call should only happen when you're absolutely sure you need the DT because it's potentially expensive. But the whole point of the deferred nature is to flush when you need it, and in this case you need it.
>
>
> Great, I'll try this out then.


I've done something like this in ProcessBlock which prevents LVI from crashing anymore.

In JumpThreadingPass::runImpl thn there is also this code that I haven't done anything with:

  while (ProcessBlock(BB))
    Changed = true;
  
  ++I;
  
  // Don't thread branches over a block that's slated for deletion.
  if (DDT->pendingDeletedBB(BB))
    continue;
  
  // If the block is trivially dead, zap it.  This eliminates the successor
  // edges which simplifies the CFG.
  if (pred_empty(BB) &&
      BB != &BB->getParent()->getEntryBlock()) {
    DEBUG(dbgs() << "  JT: Deleting dead block '" << BB->getName()
          << "' with terminator: " << *BB->getTerminator() << '\n');
    LoopHeaders.erase(BB);
    LVI->eraseBlock(BB);
    DeleteDeadBlock(BB, DDT);
    Changed = true;
    continue;
  }

Should we use DT here as well in some way? I did a quick attempt changing the pred_empty(BB) stuff above
to use DT in the same way as I did in processBlock and then I hit an assert in DeleteDeadBlock saying the block
wasn't dead after all anyway.

Or should we add

  if (!DDT->flush().isReachableFromEntry(BB))
    continue;

?
That alone seems to solve the problem, even if I remove the fix in ProcessBlock.


https://reviews.llvm.org/D34135