[PATCH] D157737: [Lint] Permit aliasing noalias and readnone arguments

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

There are test failures.



================
Comment at: llvm/lib/Analysis/Lint.cpp:240
+            // dereferenced anyway.
+            if (F->hasParamAttribute(ArgNo, Attribute::ReadNone))
+              continue;
----------------
bjope wrote:
> bjope wrote:
> > bjope wrote:
> > > Maybe we need to check for NoCapture as well (that would be a bit more defensive at least)?
> > 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?
> 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.
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157737