[PATCH] D141712: [GVN] Improve PRE on load instructions

[Guozhi Wei via Phabricator via llvm-commits]

Carrot added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/GVN.cpp:2703
     for (auto *I : InstrsToErase) {
-      assert(I->getParent() == BB && "Removing instruction from wrong block?");
       LLVM_DEBUG(dbgs() << "GVN removed: " << *I << '\n');
----------------
mkazantsev wrote:
> I think this assert should not fail, and if it fails, you have a bug. `ICF` may keep cached information for it bound to its old parent, and will try to remove it from its new parent. You may get inconsistent state of `ICF` because of it. At least I don't see how you update it.
> 
> You should not move instructions. The right approach is to create a new one.
Add a call to ICF->insertInstructionTo for the new created load instruction. For the deleted instruction ICF->removeInstruction is called in line 2711.

Maybe a naive question, does Load instruction impact implicit control flow?


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

https://reviews.llvm.org/D141712