[PATCH] D86815: [LangRef] Remove non-overlapping guarantee from memcpy.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added a comment.

In D86815#2245455 <https://reviews.llvm.org/D86815#2245455>, @aqjune wrote:

> Would we need a change in MemCpyOptimizer too?
> I see that the optimization is assuming memcpys don't allow partial overlapping (https://godbolt.org/z/h8jvo5 ).

I think with the update (either completely overlap or not overlap at all), the example in the link should be still allowed (either %a and %b don't overlap or the first memcpy is a no-op)



================
Comment at: llvm/docs/LangRef.rst:12477-12478
-The '``llvm.memcpy.*``' intrinsics copy a block of memory from the
-source location to the destination location, which are not allowed to
-overlap. It copies "len" bytes of memory over. If the argument is known
-to be aligned to some boundary, this can be specified as an attribute on
----------------
rsmith wrote:
> I think we only want to allow the source and destination to be the same, and still want to disallow all other cases of overlap. We still want to be able to lower `llvm.memcpy` to `memcpy` not `memmove`.
Thanks for the clarification!


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:982
   // no-aliases the other.
   if (auto *Inst = dyn_cast<AnyMemCpyInst>(Call)) {
     AliasResult SrcAA, DestAA;
----------------
rsmith wrote:
> This would presumably now be correct for `memmove` too.
yes probably. I can add that in a separate patch.


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:989-994
+    if (SrcAA == MustAlias && DestAA == NoAlias)
       // Loc is exactly the memcpy source thus disjoint from memcpy dest.
       return ModRefInfo::Ref;
-    if ((DestAA = getBestAAResults().alias(MemoryLocation::getForDest(Inst),
-                                           Loc, AAQI)) == MustAlias)
+    if (DestAA == MustAlias && SrcAA == NoAlias)
       // The converse case.
       return ModRefInfo::Mod;
----------------
rsmith wrote:
> These two `if`s appear to now be equivalent to what would happen anyway in the general-case code below, and can presumably be deleted.
Looks like that indeed! Simplified,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86815