[PATCH] D47134: [LoopVersioning] Don't modify the list that we iterate over in addPHINodes

[Michael Zolotukhin via Phabricator via llvm-commits]

mzolotukhin accepted this revision.
mzolotukhin added a comment.
This revision is now accepted and ready to land.

Looks good to me! (+ a couple of nitpicks inline)



================
Comment at: lib/Transforms/Utils/LoopVersioning.cpp:143-144
                            &PHIBlock->front());
-      for (auto *User : Inst->users())
+      // Find users that should be updated.
+      SmallVector<User*, 8> UsersToModify;
+      for (User *User : Inst->users())
----------------
I suggest s/UsersToModify/UsersToUpdate/ and remove the comments as the code is pretty clear and self-explanatory.


================
Comment at: lib/Transforms/Utils/LoopVersioning.cpp:145
+      SmallVector<User*, 8> UsersToModify;
+      for (User *User : Inst->users())
         if (!VersionedLoop->contains(cast<Instruction>(User)->getParent()))
----------------
How about renaming variable `User` to `U`? It's shorter and the new variable name won't overlap with the class name (which is not an error, but just a minor inconvenience when one searches for the variable name in the file, for instance).


Repository:
  rL LLVM

https://reviews.llvm.org/D47134