[PATCH] D54023: [LoopSimplifyCFG] Delete dead in-loop blocks

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:237
+                        << "\n");
+      if (LI.isLoopHeader(BB))
+        LI.erase(LI.getLoopFor(BB));
----------------
Looking at how DeadLoopBlocks get populated, these contain blocks within sub-loops as well.

So, there are 2 ways a subloop can become dead (either properly nested loop or a sibling loop). `BB` here belongs to a subloop and is:
1. header of that loop, which is the case you've handled here, i.e. that loop is removed from LI.
2. latch of that loop, which I don't see being handled. Once latch is removed, that subloop is no longer a loop either.


================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:240
+      LI.removeBlock(BB);
+      DeleteDeadBlock(BB, &DTU);
+      ++NumLoopBlocksDeleted;
----------------
So, this removes the dead block and updates DT.
However, what about this case:
```
deadblock:
  br anotherblock:

anotherblock:
  <no phis, so we're guaranteed there's exactly one pred, which is deadblock>
   
```
DeleteDeadBlock does not recursively remove dead blocks. It handles the case where: `dead block` is *one of the predecessors* of `anotherblock` - see `BasicBlock::removePredecessor`.

I don't think `DeadLoopBlocks` contains `anotherblock`, since it became dead/unreachable when `deadblock` is dead. 



https://reviews.llvm.org/D54023