Philip Reames
2015-Jun-19 17:57 UTC
[LLVMdev] Attribute to mark that function only access memory through it's arguments
On 06/18/2015 09:15 PM, Nick Lewycky wrote:> > Currently in AliasAnalysis we can model ModRef behaviour for > functions which > only access memory through their pointer arguments. However we can't > express this propery as a function attribute. > > For example, for intrinsics we can specify ReadWriteArgMem or > ReadArgMem > attributes in tablegen definitions. But due to the lack of the > related function > attributes on the llvm ir level, this intrinsics would be modelled > as if they > were clobbering arbitrary memory. > > It feels very naturall to add new function attribute which can > cover such cases. > > I have a patch (http://reviews.llvm.org/D10398 > <https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10398&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=Mfk2qtn1LTDThVkh6-oGglNfMADXfJdty4_bhmuhMHA&m=6QD3kOt2shxvhEw0pkmPGL_NzzjWw6s3ZTzBo-rwkUs&s=MJrrF5wf5KOXiTUsfpoJMSDYvyM_nZ3L79ZSsA9iXaA&e=>) > in which I added this > attribute. Currently there is some discussion on how to name it > and how it should > behave when defined together with other fucntion attributes. > > > What does it mean? Can it only touch memory that is directly referred > to by an argument? Or if that argument points to another pointer, can > we follow it?I just want to point out that the notion Igor is introducing as an attribute is not a new one. It's a prexisting notion which is already implemented within LLVM today; it's simply been restricted to intrinsics. Here's the definition from Intrinsics.td: // IntrReadWriteArgMem - This intrinsic reads and writes only from memory that // one of its arguments points to, but may access an unspecified amount. The // reads and writes may be volatile, but except for this it has no other side // effects. def IntrReadWriteArgMem : IntrinsicProperty; I did point out in the review that I think the notion of ReadsArgMem is redundant given the existing notions of ReadWriteArgMem and ReadOnly, but that's somewhat orthogonal. It's true of the existing implementation, not just Igor's patch. It may be worth settling on this to clarify naming (i.e. are we ever going to need an attribute like reads_arg_mem? Or can we be more generic and use accesses_arg_mem + readonly for the same purpose?), but I don't think that really changes the semantics in major ways. Philip -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150619/6501c4b8/attachment.html>
Philip Reames
2015-Jun-26 20:42 UTC
[LLVMdev] Attribute to mark that function only access memory through it's arguments
I haven't heard any objection to the proposed attribute. Given that, I think we can go ahead and move forward. Igor - Can you update the patch to use the name argmemonly (analogous to readonly, readnone)? As discussed, we want to factor out the read/write part and use the existing attributes. Make sure you update the docs to clarify the answer to Nick's question regarding following pointers. Once that's done, I'll take a close look and probably LGTM. We will need to find someone to review the bitcode/IR serialization though since I'm not familiar with those pieces. After this goes in, a good cleanup would be to remove the special casing for intrinsics and have the td files drive the addition of the now standardized attributes. This would be both a good cleanup and ensure test coverage of the newly introduced attribute. As such, I'd like you (Igor) to commit to doing this after the first patch lands. Philip On 06/19/2015 10:57 AM, Philip Reames wrote:> > > On 06/18/2015 09:15 PM, Nick Lewycky wrote: >> >> Currently in AliasAnalysis we can model ModRef behaviour for >> functions which >> only access memory through their pointer arguments. However we can't >> express this propery as a function attribute. >> >> For example, for intrinsics we can specify ReadWriteArgMem or >> ReadArgMem >> attributes in tablegen definitions. But due to the lack of the >> related function >> attributes on the llvm ir level, this intrinsics would be >> modelled as if they >> were clobbering arbitrary memory. >> >> It feels very naturall to add new function attribute which can >> cover such cases. >> >> I have a patch (http://reviews.llvm.org/D10398 >> <https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10398&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=Mfk2qtn1LTDThVkh6-oGglNfMADXfJdty4_bhmuhMHA&m=6QD3kOt2shxvhEw0pkmPGL_NzzjWw6s3ZTzBo-rwkUs&s=MJrrF5wf5KOXiTUsfpoJMSDYvyM_nZ3L79ZSsA9iXaA&e=>) >> in which I added this >> attribute. Currently there is some discussion on how to name it >> and how it should >> behave when defined together with other fucntion attributes. >> >> >> What does it mean? Can it only touch memory that is directly referred >> to by an argument? Or if that argument points to another pointer, can >> we follow it? > I just want to point out that the notion Igor is introducing as an > attribute is not a new one. It's a prexisting notion which is already > implemented within LLVM today; it's simply been restricted to > intrinsics. Here's the definition from Intrinsics.td: > // IntrReadWriteArgMem - This intrinsic reads and writes only from > memory that > // one of its arguments points to, but may access an unspecified > amount. The > // reads and writes may be volatile, but except for this it has no > other side > // effects. > def IntrReadWriteArgMem : IntrinsicProperty; > > I did point out in the review that I think the notion of ReadsArgMem > is redundant given the existing notions of ReadWriteArgMem and > ReadOnly, but that's somewhat orthogonal. It's true of the existing > implementation, not just Igor's patch. It may be worth settling on > this to clarify naming (i.e. are we ever going to need an attribute > like reads_arg_mem? Or can we be more generic and use > accesses_arg_mem + readonly for the same purpose?), but I don't think > that really changes the semantics in major ways. > > Philip > > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150626/cb06fb5e/attachment.html>
Igor Laevsky
2015-Jun-29 18:54 UTC
[LLVMdev] Attribute to mark that function only access memory through it's arguments
After this goes in, a good cleanup would be to remove the special casing for intrinsics and have the td files drive the addition of the now standardized attributes. This would be both a good cleanup and ensure test coverage of the newly introduced attribute. As such, I'd like you (Igor) to commit to doing this after the first patch lands. Sure, I’ll do the follow up cleanups as soon as initial change lands. — Igor On 26 Jun 2015, at 23:42, Philip Reames <listmail at philipreames.com<mailto:listmail at philipreames.com>> wrote: I haven't heard any objection to the proposed attribute. Given that, I think we can go ahead and move forward. Igor - Can you update the patch to use the name argmemonly (analogous to readonly, readnone)? As discussed, we want to factor out the read/write part and use the existing attributes. Make sure you update the docs to clarify the answer to Nick's question regarding following pointers. Once that's done, I'll take a close look and probably LGTM. We will need to find someone to review the bitcode/IR serialization though since I'm not familiar with those pieces. After this goes in, a good cleanup would be to remove the special casing for intrinsics and have the td files drive the addition of the now standardized attributes. This would be both a good cleanup and ensure test coverage of the newly introduced attribute. As such, I'd like you (Igor) to commit to doing this after the first patch lands. Philip On 06/19/2015 10:57 AM, Philip Reames wrote: On 06/18/2015 09:15 PM, Nick Lewycky wrote: Currently in AliasAnalysis we can model ModRef behaviour for functions which only access memory through their pointer arguments. However we can't express this propery as a function attribute. For example, for intrinsics we can specify ReadWriteArgMem or ReadArgMem attributes in tablegen definitions. But due to the lack of the related function attributes on the llvm ir level, this intrinsics would be modelled as if they were clobbering arbitrary memory. It feels very naturall to add new function attribute which can cover such cases. I have a patch (http://reviews.llvm.org/D10398<https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10398&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=Mfk2qtn1LTDThVkh6-oGglNfMADXfJdty4_bhmuhMHA&m=6QD3kOt2shxvhEw0pkmPGL_NzzjWw6s3ZTzBo-rwkUs&s=MJrrF5wf5KOXiTUsfpoJMSDYvyM_nZ3L79ZSsA9iXaA&e=>) in which I added this attribute. Currently there is some discussion on how to name it and how it should behave when defined together with other fucntion attributes. What does it mean? Can it only touch memory that is directly referred to by an argument? Or if that argument points to another pointer, can we follow it? I just want to point out that the notion Igor is introducing as an attribute is not a new one. It's a prexisting notion which is already implemented within LLVM today; it's simply been restricted to intrinsics. Here's the definition from Intrinsics.td: // IntrReadWriteArgMem - This intrinsic reads and writes only from memory that // one of its arguments points to, but may access an unspecified amount. The // reads and writes may be volatile, but except for this it has no other side // effects. def IntrReadWriteArgMem : IntrinsicProperty; I did point out in the review that I think the notion of ReadsArgMem is redundant given the existing notions of ReadWriteArgMem and ReadOnly, but that's somewhat orthogonal. It's true of the existing implementation, not just Igor's patch. It may be worth settling on this to clarify naming (i.e. are we ever going to need an attribute like reads_arg_mem? Or can we be more generic and use accesses_arg_mem + readonly for the same purpose?), but I don't think that really changes the semantics in major ways. Philip _______________________________________________ LLVM Developers mailing list LLVMdev at cs.uiuc.edu<mailto:LLVMdev at cs.uiuc.edu> http://llvm.cs.uiuc.edu<http://llvm.cs.uiuc.edu/> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150629/7b485b9c/attachment.html>