[PATCH] D54021: [LoopSimplifyCFG] Teach LoopSimplifyCFG to constant-fold branches and switches
Philip Reames via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 2 17:36:31 PDT 2018
reames added a comment.
Drive by comments.
It looks like you forgot to include your test cases?
================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:182
+ // Now, all exit blocks that are not marked as live are dead.
+ for (auto *ExitBlock : ExitBlocks)
+ if (!LiveExitBlocks.count(ExitBlock))
----------------
Why bother to keep around both sets when given an exit block you can answer liveness questions with only one copy? (Same question on other parallel dead/live structures BTW)
================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:196
+ // The loop is live is its latch is live.
+ LoopIsLive = IsEdgeLive(L.getLoopLatch(), L.getHeader());
+
----------------
I think you need a better name for this variable.
================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:198
+
+ if (LoopIsLive) {
+ BlocksInLoopAfterFolding.insert(L.getLoopLatch());
----------------
Not clear to me why you need this bit of code in this patch.
================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:248
+ for (auto *DeadSucc : DeadSuccessors)
+ DTU.deleteEdge(BB, DeadSucc);
+
----------------
Where's the code for LI updating?
https://reviews.llvm.org/D54021
More information about the llvm-commits
mailing list