[PATCH] D45299: API to update MemorySSA for cloned blocks.

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv accepted this revision.
george.burgess.iv added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for this!



================
Comment at: lib/Analysis/MemorySSAUpdater.cpp:839
+                            E = DefToReplaceUses.use_end();
+        for (; UI != E;) {
+          Use &U = *UI;
----------------
asbirlea wrote:
> george.burgess.iv wrote:
> > nit: Can we just do a range-for loop? Looks like we're only calling `set` on the use, which shouldn't cause any invalidation badness.
> I may be missing something, but if I do: 
> `for (Use &U : DefToReplaceUses.uses())`, I get a crash.
> I guess the `set()` does invalidate the iterator? See below, the `++UI` needs to be before `U.set()`.
That's surprising, but WFM. :)


================
Comment at: lib/Analysis/MemorySSAUpdater.cpp:699
+  // deterministic order, store predecessors as SetVectors. The order in each
+  // will be defined by teh order in Updates (fixed) and the order given by
+  // children<> (also fixed). Since we further iterate over these ordered sets,
----------------
s/teh/the


================
Comment at: lib/Analysis/MemorySSAUpdater.cpp:704
+  // An alternate implementation could keep unordered set for the predecessors,
+  // traverse either Updates or children<> each time to get  the deterministic
+  // order, and drop the usage of EdgeCount. This alternate approach would still
----------------
`s/  / /` (two spaces -> one)


Repository:
  rL LLVM

https://reviews.llvm.org/D45299