[PATCH] D55120: [MemCpyOpt] memset->memcpy forwarding with undef tail

[Nikita Popov via Phabricator via llvm-commits]

nikic marked an inline comment as done.
nikic added inline comments.


================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:173
+      Loc = MemoryLocation::getForArgument(II, 0, TLI);
+      return ModRefInfo::Mod;
     default:
----------------
efriedma wrote:
> This probably makes sense, but it looks like it'll impact other transforms; would it be possible to test separately?  Or does it actually not have any impact on other transforms for some reason?
> 
> Looks fine otherwise.
It seems that it indeed does not impact other transforms, at least not in any way that I can see. The main consumer of `GetLocation()` is `getDependency()`, for which:

 * GVN only deals with loads.
 * DSE has it's own mechanism for determining location.
 * MemCpyOpt does not call getDependency on memsets (prior to this change).

The other consumer is `getCallSiteDependencyFrom()`, where previously memset would have been treated as a normal call-site. However in both cases (known location or call-site) the result will ultimately be determinded by AA.

---

What I'm unsure about is whether this is correct wrt handling of volatile. I see in the code above that an ordered (and non-monotonic) StoreInst will return ModRef with empty location. Maybe memset needs the same treatment for volatile?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55120