[PATCH] D32494: [Loop Deletion] Delete loops that are never executed

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopDeletion.cpp:188
+  // backedge, and all statements in the loop are invariant. This is a non-loop
+  // if we can move all statements within the loop to the preheader.
+  // We can only remove the loop if there is a preheader that we can
----------------
sanjoy wrote:
> I didn't understand the "has no backedge" part -- if it does not have a backedge how is it a loop?
Actually, what I meant was it's "semantically not having a backedge": in `isLoopDead`, we check to see that only invariant values from the loop are used in the exit block. Then we hoist these invariant values *and their operands* to the preheader. I think this is equivalent to "semantically not having a backedge". I'm just going to remove these comments instead of going into all this detail :)

But just for my own clarification, do you think equivalent, or my statement is a stronger version of `isLoopDead`? 


================
Comment at: lib/Transforms/Scalar/LoopDeletion.cpp:235
+  // loop.
+  if (LoopIsNeverExecuted)
+    BlockCausingLoopUnreachable =
----------------
sanjoy wrote:
> I would just pass in the block that we decided was the last executed block instead of re-deriving it here.  Otherwise we risk the logic here and `isLoopNeverExecuted` diverging.
> 
> [edit: the comment above about splitting the preheader supersedes this comment]
Actually, that's what I did initially (during local modification). However, it required passing that last executed block between 3 functions. I personally didnt like that design either. 

 I like your first comment on splitting the edge from the loop predecessor to the header. I'll try to get that working and keep the core logic as similar as possible.


https://reviews.llvm.org/D32494