[PATCH] D38944: [GVN] Handle removal of first implicit CF instruction correctly

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/GVN.cpp:1202
       if (MD) MD->removeInstruction(I);
-      I->eraseFromParent();
+      eraseFromParent(I);
     }
----------------
Please don't do this.
Just mark them all for deletion, unless there is a very good reason not to.



================
Comment at: lib/Transforms/Scalar/GVN.cpp:2106
       DEBUG(verifyRemoved(*I));
-      (*I)->eraseFromParent();
+      eraseFromParent(*I);
     }
----------------
If you are going to try to abstract the deletion, you should do all the updating this wants (IE MD->removeInstrruction, etc)

Please don't call it eraseFromPaernt, that's meaningless. Please call it deleteInstruction or something


================
Comment at: lib/Transforms/Scalar/GVN.cpp:2303
   DEBUG(verifyRemoved(CurInst));
-  CurInst->eraseFromParent();
-  OI->invalidateBlock(CurrentBlock);
+  eraseFromParent(CurInst);
   ++NumGVNInstr;
----------------
Ditto


https://reviews.llvm.org/D38944