[PATCH] D63355: [MemorySSA] Don't use template when the clone is a simplified instruction.

[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.

just two stylistic nits; lgtm afterward. thanks for this!



================
Comment at: include/llvm/Analysis/MemorySSAUpdater.h:292
+                        const ValueToValueMapTy &VMap, PhiToDefMap &MPhiMap,
+                        bool useTemplateMUD = true);
   template <typename Iter>
----------------
I think that we need to spell out very clearly in the comment what this new parameter allows. I assume it's something along the lines of "If `true`, the clone was exact. Otherwise, assume that the clone involved simplifications that may have:

- turned a MemoryUse into an instruction that MemorySSA has no representation for, or
- turned a MemoryDef into a MemoryUse or an instruction that MemorySSA has no representation for

No other cases are supported."

In which case, I'd probably spell this bool something like `CloneWasSimplified` or simialar; `useTemplateMUD` is inscrutable for those who don't know how this is implemented IMO


================
Comment at: lib/Analysis/MemorySSAUpdater.cpp:659
     MPhiMap[MPhi] = MPhi->getIncomingValueForBlock(P1);
-  cloneUsesAndDefs(BB, P1, VM, MPhiMap);
+  cloneUsesAndDefs(BB, P1, VM, MPhiMap, false);
 }
----------------
please use /*paramName=*/false


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63355