[PATCH] D117095: [BasicAA] Add support for memmove intrinsic

Wed Jan 12 07:45:50 PST 2022

reames added a comment.

In D117095#3237306 <https://reviews.llvm.org/D117095#3237306>, @ebrevnov wrote:

> I got expected result If I change the mentioned line to:
>
> (AAQI.CI->isNotCapturedBeforeOrAt(Object, Call) || AAResults::onlyAccessesInaccessibleOrArgMem(getModRefBehavior(Call)))

Er, this is definitely unsound in general.  It's plausibly correct for the memtransfer routines since they don't load any pointers from existing state, but not in general.

Looking at it, I think you're actual problem is that we're missing generic handling for the argmemonly + readonly/writeonly param case.  The existing memcpy case should be subsumable by that, and so should the memmove case.

Having said that, generalizing to memmove first, and then generic seems entirely reasonable to me.  (i.e. I have no problem with a cleaned up version of this patch landing.)

================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:1016
   // no-alias or must-alias.
-  if (auto *Inst = dyn_cast<AnyMemCpyInst>(Call)) {
+  if (auto *Inst = dyn_cast<AnyMemTransferInst>(Call)) {
     AliasResult SrcAA =
----------------
fhahn wrote:
> Is this correct?
> 
> LangRef says the following for memmove. Note that the inputs may overlap, which is different to what the requirement stated in the comment.
> 
> ```
> The ‘llvm.memmove.*’ intrinsics copy a block of memory from the source location to the destination location, which may 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 the argument.
> 
> If <len> is 0, it is no-op modulo the behavior of attributes attached to the arguments. If <len> is not a well-defined value, the behavior is undefined. If <len> is not zero, both <dest> and <src> should be well-defined, otherwise the behavior is undefined.
> ```
> 
>  https://llvm.org/docs/LangRef.html#id505
Well, the comment above is definitely wrong after allowing memmove, but I don't see the guarded code depends on the comment at all.  We could just remove the comment.  

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117095