[PATCH] D34516: [LoopDeletion] Update exits correctly when multiple duplicate edges from an exiting block

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopDeletion.cpp:250
     if (LoopIsNeverExecuted)
-      P->setIncomingValue(j, UndefValue::get(P->getType()));
-    P->setIncomingBlock(j, Preheader);
-    for (unsigned i = 1; i < ExitingBlocks.size(); ++i)
-      P->removeIncomingValue(ExitingBlocks[i]);
+      P->setIncomingValue(PredIndex, UndefValue::get(P->getType()));
+    P->setIncomingBlock(PredIndex, Preheader);
----------------
efriedma wrote:
> Do you need to call setIncomingValue() in the case where LoopIsNeverExecuted is false?
We don't need to. When LoopIsNeverExecuted is false, all the exiting blocks have the same 'value', which gets moved to the preheader. This is also the reason why in the original implementation (before this patch), we keep just one exiting block and remove all others. 

As a follow-on NFC, I'm going to move this undef value setting into a prepass before calling this function `deleteDeadLoop`.


================
Comment at: lib/Transforms/Scalar/LoopDeletion.cpp:253
+    // Removes all incoming values from all other exiting blocks (including
+    // duplicate values from ExitingBlock).
+    // Nuke all entries except the zero'th entry which is the preheader entry.
----------------
efriedma wrote:
> There is no variable named ExitingBlock.
Oops, stale comment. I'll remove the mention of ExitingBlock.


https://reviews.llvm.org/D34516