[PATCH] D48897: [MemorySSA] Add APIs to move memory accesses between blocks, following CFG changes.

[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 with a few comments; thanks again!



================
Comment at: lib/Analysis/MemorySSA.cpp:1483
+    ValueToMemoryAccess.erase(What->getBlock());
+    assert(ValueToMemoryAccess.insert({BB, What}).second &&
+           "Cannot move a Phi to a block that already has one");
----------------
`assert(X)` compiles to nothing in non-asserting builds, so we probably want something more like:

```
bool Inserted = ValueToMemoryAccess.insert({BB, What}).second;
(void)Inserted;
assert(Inserted && "Cannot ...");
```

(the `(void)Inserted;` suppresses unused variable warnings, and forgetting to put it has tripped me up more times than I can count.)


================
Comment at: lib/Analysis/MemorySSAUpdater.cpp:455
+    MSSA->moveTo(MUD, To, MemorySSA::End);
+    Accs = MSSA->getWritableBlockAccesses(From);
+    MUD = NextMUD;
----------------
Please add a quick comment noting that `moveTo` might delete `Accs`, so we need to re-retrieve it here


Repository:
  rL LLVM

https://reviews.llvm.org/D48897