[PATCH] D72666: [IR] ArgMemOnly functions with WriteOnly ptr args do not read memory.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D72666#1820906 <https://reviews.llvm.org/D72666#1820906>, @jdoerfert wrote:

> In D72666#1820862 <https://reviews.llvm.org/D72666#1820862>, @fhahn wrote:
>
> > In D72666#1818532 <https://reviews.llvm.org/D72666#1818532>, @jdoerfert wrote:
> >
> > > In D72666#1818407 <https://reviews.llvm.org/D72666#1818407>, @efriedma wrote:
> > >
> > > > I'd prefer to just add writeonly markings in places where they're missing.  For llvm.memset in particular, can we mark it IntrWriteMem?
> > >
> >
> >
> > Yes we should probably just mark llvm.memset as IntrWriteMem.
>
>
> Agreed.


Posted D72789 <https://reviews.llvm.org/D72789>.

>>> We could "deduce" it, based on this logic, and then add `writeonly`. That would work for non intrinsics as well.
>> 
>> Yep I think it might be beneficial to use the logic in the patch to set the write-only function attribute at the time attributes are added to a call/function. I am not sure how common this is (argmemonly + writeOnly/readOnly argument attributes), but that it was missing for an intrinsic like memset seems to indicate how easy things like that can be missed. Alternatively we could use the logic to warn/fail on missing attributes. WDYT?
> 
> I dislike the warn/fail idea and I would still argue for deduction instead:
> 
> 1. We will hopefully soon have a way for users to add attributes like `argmemonly` and `writeonly`, so if they miss one we can just infer it.
> 2. It is also a "shortcut" if we can infer it if after partial code specialization eliminated the read arguments or the non-argument accesses.

I'll check how to best do it at call/declare construction time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72666