[PATCH] D52827: [LICM] Make LICM able to hoist phis

[John Brawn via Phabricator via llvm-commits]

john.brawn marked 8 inline comments as done.
john.brawn added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:497
+      if (!TrueDestSucc.empty())
+        CommonSucc = *TrueDestSucc.begin();
+    }
----------------
mkazantsev wrote:
> If the size of set is not 1, you are introducing non-deterministic optimizer behavior here. Please assert that the size is 1.
It is actually possible to have more than 1 here. I'll adjust it to have deterministic behaviour for that case (by picking whichever happens to be first in the function's block list).


================
Comment at: lib/Transforms/Scalar/LICM.cpp:681
+    }
+    BasicBlock *BB = Worklist[idx]->getBlock();
+    Worklist.erase(Worklist.begin() + idx);
----------------
mkazantsev wrote:
> How do you ensure that you have found something? Is it correct to do it if nothing was found and `idx` has a default value?
It's actually possible to not find anything, in which case we fall back to picking the first element of the worklist. I've updated the comment to make this clear.


https://reviews.llvm.org/D52827