[PATCH] D119070: [MachineCombiner] Update iterator while deleting instructions

[Darshan Bhat via Phabricator via llvm-commits]

DarshanRamakant added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineCombiner.cpp:489
   for (auto *InstrPtr : DelInstrs) {
+    // If the instructions to be deleted is pointed by cuurent iterator
+    // update the iterator.
----------------
arsenm wrote:
> Typo cuurent
Thanks will fix it. 


================
Comment at: llvm/lib/CodeGen/MachineCombiner.cpp:492
+    if (InstrPtr == &*BlockItr) {
+      BlockItr++;
+    }
----------------
arsenm wrote:
> I've never looked at this pass before, but this looks like a strange structure. Why can't this pre-increment the iterator before making changes to the function?
BlockItr will be pointing to the next instruction to be processed. If the instruction marked for delete happens to be exactly that, then  line 570 will become a dangling pointer and will have an undefined behavior. 



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

https://reviews.llvm.org/D119070