[PATCH] D54021: [LoopSimplifyCFG] Teach LoopSimplifyCFG to constant-fold branches and switches

[Philip Reames via Phabricator via llvm-commits]

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