[PATCH] D43473: [mem2reg] Use range loops (NFCI)

Bjorn Pettersson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 20 00:17:13 PST 2018


bjope added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp:329
+  for (User *U : AI->users()) {
+    Instruction *I = cast<Instruction>(U);
     if (isa<LoadInst>(I) || isa<StoreInst>(I))
----------------
I think the old code was more safe since it does not depend as much on how the iterators are implemented. The instruction "I" is actually deleted inside  the loop. So stepping the iterator after deleting "I" could be dangerous if stepping of the iterator involves dereferencing of the object that the iterator refers. Or am I missing something?
I would at least assume that the old code was written the way it was just because the datastructure that we iterate over can change during iteration. So it should be more safe to step the iterator to the "next" element first, and then remove the "current" element.



Repository:
  rL LLVM

https://reviews.llvm.org/D43473





More information about the llvm-commits mailing list