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

Tue Jan 14 16:12:24 PST 2020

jdoerfert added a comment.

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.

>> 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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72666