[PATCH] D27034: [AliasAnalysis] Teach BasicAA about memcpy.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:773
   }
 
+  if (auto *Inst = dyn_cast<MemCpyInst>(CS.getInstruction())) {
----------------
bryant wrote:
> reames wrote:
> > This entire change should be unnecessary.  We have readonly and writeonly attributes with these semantics and the intrinsic should already be declared with them.  
> > 
> > Reading further, it looks like you're change is mainly aimed at the no alias property right?  If so, take a look at the noalias parameter attribute.  It might be we've forgotten to tag the intrinsic correctly.
> > This entire change should be unnecessary. We have readonly and writeonly attributes with these semantics and the intrinsic should already be declared with them.
> 
> Including write-/readonly inside memcpy's declaration with my initial example doesn't work, at least not with `-basicaa`:
> 
>     declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32, i1) nounwind
> 
>     define void @source_clobber(i8* %a, i8* %b) {
>     ; CHECK-LABEL: @source_clobber(
>     ; CHECK-NEXT:  ; 1 = MemoryDef(liveOnEntry)
>     ; CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 128, i32 1, i1 false)
>     ; CHECK-NEXT:  ; MemoryUse(liveOnEntry)
>     ; CHECK-NEXT:    [[X:%.*]] = load i8, i8* %b
>     ; CHECK-NEXT:    ret void
>     ;
>       call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 128, i32 1, i1 false)
>       %x = load i8, i8* %b
>       ret void
>     }
> 
>     09:57:21 ~> opt -disable-output -basicaa -aa-eval -print-all-alias-modref-info memcpy-test.ll
>     Function: source_clobber: 2 pointers, 1 call sites
>       MayAlias:     i8* %a, i8* %b
>       Both ModRef:  Ptr: i8* %a     <->  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 128, i32 1, i1 false)
>       Both ModRef:  Ptr: i8* %b     <->  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 128, i32 1, i1 false)
> 
> > Reading further, it looks like you're change is mainly aimed at the no alias property right? If so, take a look at the noalias parameter attribute. It might be we've forgotten to tag the intrinsic correctly.
> 
> It's not always the case that parameters can satisfy the `noalias` attribute. Suppose, for instance, that the previous function was instead
> 
>     define void @f(i8* %a, i8* %b, i8* %c, i8* %d)
> 
> where a and b could alias c and d, respectively. Then neither a nor b fits noalias requirement. Yet, even in such a case, the LangRef constraint on memcpy should be sufficient to deduce that a no-alias b.
> 
I don't understand what your comment means. How are %c and %d accessed?

Just in case it is not clear, noalias's semantics are similar in this regard to C's restrict semantics. noalias means that memory accessed through the attributed pointer, or any pointers based on that pointer, are not accessed through a pointer not based on the attributed pointer. In C, memcpy certainly has restrict on its pointer arguments, and we could add noalias on ours as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D27034