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

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added inline comments.


================
Comment at: lib/Analysis/MemorySSAUpdater.cpp:1258
 
+void MemorySSAUpdater::tryRemoveTrivialPhis(SmallVectorImpl<WeakVH> &UpdatedPHIs,
+                                            unsigned Start, unsigned Stop) {
----------------
george.burgess.iv wrote:
> 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.
Done in separate cleanup patch. Thanks for the suggestion!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60833