[PATCH] D21613: [DSE] Avoid iterator invalidation bugs.

[Chad Rosier via llvm-commits]

mcrosier added inline comments.

================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:69
@@ -70,1 +68,3 @@
+deleteDeadInstruction(Instruction *I, Instruction *CurrInst,
+                      MemoryDependenceResults &MD, const TargetLibraryInfo &TLI,
                       SmallSetVector<Value *, 16> *ValueSet = nullptr) {
----------------
eli.friedman wrote:
> Maybe it would make sense to pass in a BasicBlock::iterator* and make this function update it, rather than returning a BasicBlock::iterator?
Sure.

================
Comment at: lib/Transforms/Scalar/DeadStoreElimination.cpp:822
@@ -814,3 +821,3 @@
     if (CallInst *F = isFreeCall(Inst, TLI)) {
       MadeChange |= handleFree(F, AA, MD, DT, TLI);
       continue;
----------------
eli.friedman wrote:
> handleFree can invalidate BBI, I think?
How so?  IIUC, deleting the 'free' instruction would invalidate the iterator, but I'm not convinced that can happen.  Some care is needed to make sure we pass a valid iterator to MD-getPointerDependencyFrom(), but I believe I've handled that correctly.


http://reviews.llvm.org/D21613