[PATCH] D56284: [UnrollRuntime] Fix domTree failure in multiexit unrolling

[Anna Thomas via Phabricator via llvm-commits]

anna added a comment.

In D56284#1348693 <https://reviews.llvm.org/D56284#1348693>, @kuhar wrote:

> In D56284#1348681 <https://reviews.llvm.org/D56284#1348681>, @anna wrote:
>
> > the same bug can happen with:
> >
> > 1. latch exit
> > 2. non-immediate successors of latchexit/otherexit.
> >
> >   I have simplified test cases for each of these and we need a more general fix. Working on this.
>
>
> ... these things are extremely bug-prone; I really do suggest using the automatic updater instead of trying to deal with all tricky corner-cases here :)


I've got a naive question about the DTU: Does the automatic updater handle the ImmediateDominator "automatically" for the remaining nodes in the dom tree once identify that one of the nodes in the DT has a change in IDom?

This will drastically help with #2 because what I have as a local fix using the old DT is something like this:

  +   if (DT) {
  +    // Check the dom children of each block in loop and if it is outside the
  +    // current loop, update it to the preheader.
  +    for (auto *BB: L->blocks()) {
  +      auto *DomNodeBB = DT->getNode(BB);
  +      for (auto *DomChild: DomNodeBB->getChildren()) {
  +        if (!L->contains(LI->getLoopFor(DomChild->getBlock()))) {
  +          DT->changeImmediateDominator(DomChild, DT->getNode(PreHeader));
  +        }    
  +      }    
  +    }
  +   }

This is a more general fix using the old DT to handle the non-immediate successors of an exit block in the loop.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56284/new/

https://reviews.llvm.org/D56284