[PATCH] D109068: Fix a missing memoryssa update in breakLoopBackedge

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

The reason the verification didn't trigger right after LoopDeletion is that it's under EXPENSIVE_CHECKS (in verifyOrderingDominationAndDefUses). MSSA is incorrect (without this change) after LoopDeletion.



================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:751
+        if (MSSA)
+          MSSAU->applyUpdates({{DominatorTree::Delete, Latch, Header}}, DT);
         return;
----------------
nikic wrote:
> I think there may be a preference to let MSSAU also update DT for performance reasons, along the lines of:
> ```
> if (MSSA)
>   MSSAU->applyUpdates({{DominatorTree::Delete, Latch, Header}}, DT, /*UpdateDT*/true);
> else
>   DT.applyUpdates({{DominatorTree::Delete, Latch, Header}});
> ```
> But I'm not sure how important this is. @asbirlea will know...
Yes, ideally, let the MSSAU handle DT updates as well.

I'll send a change to the MSSAUpdater shortly so it doesn't matter as much when only deleting edges. I'll be ok with the code as is after that change (right now, with this patch, the deleted edge is readded during the MSSA update and then redeleted).
In general, best practice is to have them updated together.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109068