[PATCH] D65020: [GVN] Do PHI translations across all edges between the load and the unavailable pred.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 15 13:57:50 PDT 2019


fhahn marked 2 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/GVN.cpp:1204
+      // trying to number them.
+      NewInsts.pop_back_val()->eraseFromParent();
     }
----------------
efriedma wrote:
> fhahn wrote:
> > efriedma wrote:
> > > I don't see any explanation for this change?
> > markInstructionForDeletion asserts that the removed instructions are defined in the current basic block. 
> > 
> > But PHI translation may insert instructions in any of the basic blocks traversed during translation. If that happens and translation fails, we have to remove instructions from different basic blocks than the current one. I thought it is best to directly remove them (no other parts of GVN have seen them yet and they are not numbered yet).
> > 
> > I can update the comment to make that clearer.
> That makes sense.  Please update the comment to at least mention the cross-BB issue.
Thanks, I've updated the comment. I hope it is clearer now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65020





More information about the llvm-commits mailing list