[PATCH] D39781: [GVNHoist] Fix: PR35222 gvn-hoist incorrectly erases load

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/GVNHoist.cpp:801
+          const BasicBlock *B = V[i]->getParent();
+          if (DT->dominates(IDFB, V[i]->getParent()) && // Ignore spurious PDFs.
+              LInfo->getLoopFor(IDFB) == LInfo->getLoopFor(B)) {
----------------
I don't think this check is overall safe.

The definition of loop here is not going to be sufficient either.

I think if you want to avoid spurious CHI's, you need to do the equivalent of what you see in "A Practical Improvement to the Partial Redundancy Elimination in SSA Form" (https://sites.google.com/site/jongsoopark/home/ssapre.pdf)

which precisely defines which will be unnecessary and the safety conditions.






https://reviews.llvm.org/D39781