[GVN][NFC] Mark instruction for deletion instead of immediate erasing in LoadPRE

[mkazantsev]

Summary

It is done to uniformly handle instructions removal.

[reames]

LGTM w/comment applied.

Meta comment: It looks from the comment at the top of processBlock that moving away from InstsToErase entirely would actually be a better direction, but the moment, we can be consistent.

lib/Transforms/Scalar/GVN.cpp

1838 ↗

(On Diff #120569)

Looking at the code for InstsToErase, it looks like there's an invariant that the map is empty between every instruction. Can you add an assert here to that effect if you agree with the analysis?

[mkazantsev]

lib/Transforms/Scalar/GVN.cpp

1838 ↗

(On Diff #120569)

I will double-check this and commit as a separate NFC.

llvm/trunk/lib/Transforms/Scalar/GVN.cpp

for (auto &PredLoad : PredLoads) {
     }
 
     PredLoad.second = LoadPtr;
   }
 
   if (!CanDoPRE) {
     while (!NewInsts.empty()) {
       Instruction *I = NewInsts.pop_back_val();
-      if (MD) MD->removeInstruction(I);
-      I->eraseFromParent();
+      markInstructionForDeletion(I);
     }
     // HINT: Don't revert the edge-splitting as following transformation may
     // also need to split these critical edges.
     return !CriticalEdgePred.empty();
   }
 
   // Okay, we can eliminate this load by inserting a reload in the predecessor
   // and using PHI construction to get the value in the other predecessors, do