[Lint] Don't warn about noalias argument aliasing if other argument is byval

[uabelho]

Summary

When using byval, the data is effectively copied as part of the call
anyway, so we aren't actually passing the pointer and thus there is no
reason to issue a warning.

[uabelho]

Quite similar to D40009.

[rnk]

lib/Analysis/Lint.cpp

269

AttributeList is a lightweight pointer wrapper that is typically passed by value. No need for const & here.

270–273

This can be simplified to just CS.getAttributes().

275

for (Value *OtherArg : CS.args()) { ?

[uabelho]

Addressed two of three comments.

[uabelho]

Thanks for the comments!

lib/Analysis/Lint.cpp

275

I could probably change it but I'm hesitant to do it since BI is also used in "AI != BI" below to skip over when "OtherArg == Arg" and I don't want to mess up anything just to do some kind of unrelated refactoring.

[uabelho]

Ok, or do you want me to do something with the for loop anyway?

[uabelho]

Ping. Ok to submit?

[rnk]

lgtm

Apparently I filled this in but never hit submit.

[uabelho]

In D40118#961538, @rnk wrote:

lgtm

Apparently I filled this in but never hit submit.

Thanks! Pushed it now.

lib/Analysis/Lint.cpp

for (; AI != AE; ++AI) {
         Assert(Formal->getType() == Actual->getType(),
                "Undefined behavior: Call argument type mismatches "
                "callee parameter type",
                &I);
 
         // Check that noalias arguments don't alias other arguments. This is
         // not fully precise because we don't know the sizes of the dereferenced
         // memory regions.
-        if (Formal->hasNoAliasAttr() && Actual->getType()->isPointerTy())
-          for (CallSite::arg_iterator BI = CS.arg_begin(); BI != AE; ++BI)
+        if (Formal->hasNoAliasAttr() && Actual->getType()->isPointerTy()) {
+          const AttributeList &PAL =
+            CS.isCall() ?
+             cast<CallInst>(CS.getInstruction())->getAttributes() :
+             cast<InvokeInst>(CS.getInstruction())->getAttributes();
+
+          unsigned ArgNo = 0;
+          for (CallSite::arg_iterator BI = CS.arg_begin(); BI != AE; ++BI) {
+            // Skip ByVal arguments since they will be memcpy'd to the callee's
+            // stack so we're not really passing the pointer anyway.
+            if (PAL.hasParamAttribute(ArgNo++, Attribute::ByVal))
+              continue;
             if (AI != BI && (*BI)->getType()->isPointerTy()) {
               AliasResult Result = AA->alias(*AI, *BI);
               Assert(Result != MustAlias && Result != PartialAlias,
                      "Unusual: noalias argument aliases another argument", &I);
             }
+          }
+        }
 
         // Check that an sret argument points to valid memory.
         if (Formal->hasStructRetAttr() && Actual->getType()->isPointerTy()) {
           Type *Ty =
             cast<PointerType>(Formal->getType())->getElementType();
           visitMemoryReference(I, Actual, DL->getTypeStoreSize(Ty),
                                DL->getABITypeAlignment(Ty), Ty,
                                MemRef::Read | MemRef::Write);

test/Analysis/Lint/noalias-byval.ll

+; RUN: opt < %s -lint -disable-output 2>&1 | FileCheck %s
+
+%s = type { i8 }
+
+; Function Attrs: argmemonly nounwind
+declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture writeonly, i8* nocapture readonly, i32, i32, i1) #0
+
+; Function Attrs: argmemonly nounwind
+declare void @llvm.memset.p0i8.i8.i32(i8* nocapture writeonly, i8, i32, i32, i1) #0
+
+declare void @f1(%s* noalias nocapture sret, %s* nocapture readnone)
+
+define void @f2() {
+entry:
+  %c = alloca %s
+  %tmp = alloca %s
+  %0 = bitcast %s* %c to i8*
+  %1 = bitcast %s* %tmp to i8*
+  call void @llvm.memset.p0i8.i8.i32(i8* %0, i8 0, i32 1, i32 1, i1 false)
+  call void @f1(%s* sret %c, %s* %c)
+  ret void
+}
+
+; Lint should complain about us passing %c to both arguments since one of them
+; is noalias.
+; CHECK: Unusual: noalias argument aliases another argument
+; CHECK-NEXT: call void @f1(%s* sret %c, %s* %c)
+
+declare void @f3(%s* noalias nocapture sret, %s* byval nocapture readnone)
+
+define void @f4() {
+entry:
+  %c = alloca %s
+  %tmp = alloca %s
+  %0 = bitcast %s* %c to i8*
+  %1 = bitcast %s* %tmp to i8*
+  call void @llvm.memset.p0i8.i8.i32(i8* %0, i8 0, i32 1, i32 1, i1 false)
+  call void @f3(%s* sret %c, %s* byval %c)
+  ret void
+}
+
+; Lint should not complain about passing %c to both arguments even if one is
+; noalias, since the other one is byval, effectively copying the data to the
+; stack instead of passing the pointer itself.
+; CHECK-NOT: Unusual: noalias argument aliases another argument
+; CHECK-NOT: call void @f3(%s* sret %c, %s* %c)
+
+attributes #0 = { argmemonly nounwind }