[PATCH] D60833: [MemorySSA] Teach LoopSimplify to preserve MemorySSA.

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added inline comments.


================
Comment at: lib/Analysis/MemorySSAUpdater.cpp:1258
 
+void MemorySSAUpdater::tryRemoveTrivialPhis(SmallVectorImpl<WeakVH> &UpdatedPHIs,
+                                            unsigned Start, unsigned Stop) {
----------------
asbirlea wrote:
> george.burgess.iv wrote:
> > nit: Would it work just as well to take an `ArrayRef<WeakVH>` here and drop `Start`/`Stop` params? (That way, we can also simplify the loop to a `for` over `UpdatedPHIs`)
> > 
> > If not, please make UpdatedPhis a const&
> Yes, it would work for this patch. I added the range to reuse it in insertDef, where the start and stop are different, but I didn't include that change here since it's not related.
> Please let me know if you prefer to have the Start/Stop removed here and re-added in the follow up patch that cleans the other use cases.
I was hoping more that the ArrayRef would make it so we don't need Start/End to begin with. Slicing/dicing those is trivial, and makes it more obvious what you're doing at the callsite IMO.

I don't feel super strongly either way, though; feel free to do Start/Stop if you think that it's cleaner.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60833