[PATCH] D111611: [NFC] [LoopPeel] Change the way DT is updated for loop exits

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 14 00:40:22 PDT 2021


mkazantsev requested changes to this revision.
mkazantsev added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Transforms/Utils/LoopPeel.cpp:826
+  SmallDenseMap<BasicBlock *, BasicBlock *> LoopBlocksIDoms;
+  for (Loop::block_iterator BB = L->block_begin(), E = L->block_end(); BB != E;
+       ++BB)
----------------
mkazantsev wrote:
> ```
> for (auto *BB : L->blocks())
> ```
> 
*BB


================
Comment at: llvm/lib/Transforms/Utils/LoopPeel.cpp:891
   // Finally DomtTree must be correct.
-  assert(DT->verify(DominatorTree::VerificationLevel::Fast));
+  if (DTU.hasDomTree())
+    assert(DTU.getDomTree().verify(DominatorTree::VerificationLevel::Fast));
----------------
dmakogon wrote:
> mkazantsev wrote:
> > Lazy DTU will only apply updates when it goes out of scope, or when it flushes. `simplifyLoop` below uses DT and needs it to be up to date. I'd suggest calling `DTU.flush()` here to apply all pending updates and leave assert as is.
> Calling DTU.getDomTree flushes all updates to DT
And you only call it in assert, i.e. in debug mode. It will simply not work in release. Please do it properly. I insist it to be flush + old assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111611



More information about the llvm-commits mailing list