[PATCH] D57199: [MemorySSA] Extend removeMemoryAccess API to optimize MemoryPhis.

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added inline comments.


================
Comment at: lib/Analysis/MemorySSAUpdater.cpp:1107
+  // Optionally optimize Phi uses. This will recursively remove trivial phis.
+  if (PhisToCheck.size()) {
+    SmallVector<TrackingVH<MemoryPhi>, 16> PhisToOptimize;
----------------
nit: `!PhisToCheck.empty()`, please


================
Comment at: lib/Analysis/MemorySSAUpdater.cpp:1108
+  if (PhisToCheck.size()) {
+    SmallVector<TrackingVH<MemoryPhi>, 16> PhisToOptimize;
+    for (MemoryPhi *MP : PhisToCheck)
----------------
Why is this a vector of `TrackingVH`?

I'm under the impression, though I haven't totally proven it to myself, that `onlySingleValue(MP)` implies that we'll never try to remove/RAUW/etc. `MP` in `tryRemoveTrivialPhi`, unless we're specifically calling `tryRemoveTrivialPhi` on `MP`. So, we shouldn't need to track anything here.

Happy to have this be an `AssertingVH` to assert that these nothing in this vector is dropped before we get to it, though.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57199