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

[bryant via Phabricator via llvm-commits]

bryant added inline comments.


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:773
   }
 
+  if (auto *Inst = dyn_cast<MemCpyInst>(CS.getInstruction())) {
----------------
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.



Repository:
  rL LLVM

https://reviews.llvm.org/D27034