[Lint] Permit aliasing noalias and readnone arguments

[bjope]

Summary

If an argument is readnone we know that it isn't dereferenced. Then
it should be OK if that argument alias with a noalias argument.

[bjope]

llvm/lib/Analysis/Lint.cpp

240

Maybe we need to check for NoCapture as well (that would be a bit more defensive at least)?

[bjope]

For reference, here is an example that made me look at this: https://godbolt.org/z/xY4hE9Kvb
The caller is doing

call void @foo(ptr nonnull sret(%struct.A) align 8 %3, ptr noundef nonnull align 8 %1)

and the callee is

define dso_local void @foo(ptr noalias nocapture writeonly sret(%struct.A) align 8 %0, ptr nocapture noundef readnone align 8 %1) local_unnamed_addr #0

And then memcpyopt will rewrite the call to use same ptr for both arguments, which then result in complaints from lint. But I think that should be OK here.

[bjope]

llvm/lib/Analysis/Lint.cpp

240

Given the example from https://godbolt.org/z/xY4hE9Kvb

If I remove NoCapture on the second argument then MemCpyOpt no longer rewrite the call. So then I actually think it would make sense to also check NoCapture here. Right?

[bjope]

llvm/lib/Analysis/Lint.cpp

240

I looked a bit more a MemCpyOptPass::performCallSlotOptzn to understand the logic for when it may introduce the IR when passing the ptr in multiple arguments (with some being noalias).
IIUC that transform is based on the several conditions. For example that the pointer is pointing to an uninitialized alloca.
And it also need to check that the pointer isn't captured between the call and the memcpy. For that check it uses PointerMayBeCapturedBefore, and I think that is why the NoCapture attribute makes a difference.
Another check is to verify that the call "does not sneakily access dest". For that check it use AA and callCapturesBefore. Which would detect readnone to derive that the pointer isn't dereferenced via that argument.

Anyway. Not sure if the MemCpyOpt behavior is that important here. The question is when it is allowed to pass a pointer that is used in a noalias argument in another argument.

[nikic]

There are test failures.

llvm/lib/Analysis/Lint.cpp

240

Should use I.doesNotAccessMemory(ArgNo) to take into account both the call-site and declaration attributes.

I don't think NoCapture is relevant here. noalias is a pure provenance concept, so it only cares about memory accesses. It also applies only for the duration of the call, so it does not matter if the pointer is captured and accessed later.

[bjope]

Updated to use I.doesNotAccessMemory.

[nikic]

LGTM

llvm/lib/Analysis/Lint.cpp

for (; AI != AE; ++AI) {
           for (auto *BI = I.arg_begin(); BI != AE; ++BI, ++ArgNo) {
             // 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.hasParamAttr(ArgNo, Attribute::ByVal))
               continue;
             // If both arguments are readonly, they have no dependence.
             if (Formal->onlyReadsMemory() && I.onlyReadsMemory(ArgNo))
               continue;
+            // Skip readnone arguments since those are guaranteed not to be
+            // dereferenced anyway.
+            if (I.doesNotAccessMemory(ArgNo))
+              continue;
             if (AI != BI && (*BI)->getType()->isPointerTy()) {
               AliasResult Result = AA->alias(*AI, *BI);
               Check(Result != AliasResult::MustAlias &&
                         Result != AliasResult::PartialAlias,
                     "Unusual: noalias argument aliases another argument", &I);
             }
           }
         }

llvm/test/Analysis/Lint/noalias-byval.ll

 ; RUN: opt < %s -passes=lint -disable-output 2>&1 | FileCheck %s
 
 %s = type { i8 }
 
 ; Function Attrs: argmemonly nounwind
 declare void @llvm.memcpy.p0.p0.i32(ptr nocapture writeonly, ptr nocapture readonly, i32, i1) #0
 
 ; Function Attrs: argmemonly nounwind
 declare void @llvm.memset.p0.i32(ptr nocapture writeonly, i8, i32, i1) #0
 
-declare void @f1(ptr noalias nocapture sret(%s), ptr nocapture readnone)
+declare void @f1(ptr noalias nocapture sret(%s), ptr nocapture)
 
 define void @f2() {
 entry:
   %c = alloca %s
   %tmp = alloca %s
   call void @llvm.memset.p0.i32(ptr %c, i8 0, i32 1, i1 false)
   call void @f1(ptr sret(%s) %c, ptr %c)
   ret void

llvm/test/Analysis/Lint/noalias-readnone.ll

+; RUN: opt < %s -passes=lint -disable-output 2>&1 | FileCheck --allow-empty %s
+
+declare void @foo1(ptr noalias, ptr readnone)
+
+define void @test1(ptr %a) {
+entry:
+  call void @foo1(ptr %a, ptr %a)
+  ret void
+}
+
+; Lint should not complain about passing %a to both arguments even if one is
+; noalias, since the second argument is readnone.
+; CHECK-NOT: Unusual: noalias argument aliases another argument
+; CHECK-NOT: call void @foo1(ptr %a, ptr %a)