[PATCH] D57316: [LoopSimplifyCFG] Use lazy DT update strategy: bug with eager one?

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:325
         Twine(Preheader->getName()).concat("-split"));
     DTU.deleteEdge(Preheader, L.getHeader());
     DTU.insertEdge(NewPreheader, L.getHeader());
----------------
I have not looked at this in detail, but I've had a similar issue at one point in SimpleLoopUnswitch with DomTree. AFAICT, this applies for DTU as well.
Calling deleteEdge and insertEdge one after the other will not do the updates properly.
The correct way to do it is:
```
  SmallVector<DominatorTree::UpdateType, 3> DTUpdates;
  DTUpdates.push_back({DT.Insert, NewPreheader, L.getHeader()});
  DTUpdates.push_back({DT.Insert, Preheader, NewPreheader});
  DTUpdates.push_back({DT.Delete, Preheader, L.getHeader()});
  DTU.applyUpdates(DTUpdates);
```
Perhaps the behavior is different with the `Eager` strategy, but I'm fairly confident using `applyUpdates` is correct for DT.


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

https://reviews.llvm.org/D57316