[PATCH] D88725: [GVN LoadPRE] Extend the scope of optimization by using context to prove safety of speculation

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/GVN.cpp:1247
+    if (CriticalEdgePred.size())
+      if (!isSafeToSpeculativelyExecute(LI, LoadBB->getFirstNonPHI(), DT))
+        return false;
----------------
mkazantsev wrote:
> Why is that needed? We split critical edges right below. As far as I understand, we will insert loads into newly created split blocks which is safe.
Imaging that we have a guard for null check in load's basic block and we plan to skip it (speculate). If we hoist load over this guard we can end up with a load from null address. This is not correct. We must ensure that on end of spilt block == entry to our basic block address can be proved a non-null.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88725/new/

https://reviews.llvm.org/D88725