[PATCH] D28414: Simplify conditional branch on constant condition and remove unreachable blocks in LoopUnswitch.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:1374
+        BI->eraseFromParent();
+        Worklist.push_back(NBI);
+      }
----------------
trentxintong wrote:
> efriedma wrote:
> > See the comment near the beginning of LoopUnswitch::TryTrivialLoopUnswitch for a description of all the state you aren't updating correctly.
> > 
> > Also, I think you meant "isa<ConstantInt>(BI->getCondition())".
> Thanks @efriedma. In llvm, a change in CFG like this, do we usually expect to update the dominator tree dynamic/manually or we are ok with DT->recalculate.
LoopUnswitch already calls DT->recalculate... but only once loop, and only if we unswitched that loop.  In theory, doing it that way is O(N^2) in the number of loops in the function (since DT->recalculate scans the whole function), so it's something we should fix at some point, it doesn't seem to cause issues in practice.  If you want to call it more frequently than that, it could lead to problems.


https://reviews.llvm.org/D28414