Thank you for the prompt response, Matt. On Jan 20, 2014, at 3:03 PM, Matt Arsenault <Matthew.Arsenault at amd.com> wrote:> Since 3.4 you need to use the addrspacecast instruction to cast between address spaces. It's possible there are still some places left that haven't been fixed yet to use it instead of creating bit casts.Are you saying you think this is a bug and it should in fact be generating an addrspacecast? I will be looking into making this change, and if it would be accepted as a bug fix, I’d be happy to submit it.>> 2. Is there a way to create methods that can be called with pointers to different address spaces? It seems there is no way to declare these in C++ using GNU attributes, and addrspace() seems to not be supported by C++11 attribute syntax, which could possibly express this. > You can use __attribute__((address_space(N))I have already been using that syntax to annotate pointer declarations. However I don’t think it can be applied to methods the way I was thinking. At least it doesn’t actually change them: struct Foo { long x; __attribute__((address_space(7))) void bar(long y) { printf("%ld %ld\n", x, y); } }; Generates this declaration still: define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo* %this, i64 %y) ssp uwtable align 2 Though I think desired behavior would be: define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo addrspace(7)* %this, i64 %y) ssp uwtable align 2 (actually I think it’s more tricky because the address_space attribute could be applying to the return type. I’d think the correct way to specify the attribute on “this” would be to put it where “const” goes, after the parentheses) -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140120/d644c0b0/attachment.html>
----- Original Message -----> From: "Brandon Holt" <bholt at cs.washington.edu> > To: "Matt Arsenault" <Matthew.Arsenault at amd.com> > Cc: "LLVM Developers Mailing List" <llvmdev at cs.uiuc.edu> > Sent: Monday, January 20, 2014 5:19:27 PM > Subject: Re: [LLVMdev] Methods on addrspace pointers > > > > Thank you for the prompt response, Matt. > > > On Jan 20, 2014, at 3:03 PM, Matt Arsenault < > Matthew.Arsenault at amd.com > wrote: > > > Since 3.4 you need to use the addrspacecast instruction to cast > between address spaces. It's possible there are still some places > left that haven't been fixed yet to use it instead of creating bit > casts. > > > > > Are you saying you think this is a bug and it should in fact be > generating an addrspacecast? I will be looking into making this > change, and if it would be accepted as a bug fix, I’d be happy to > submit it.Looks like a bug (regardless, it certainly shouldn't crash like that).> > > > > > > 2. Is there a way to create methods that can be called with pointers > to different address spaces? It seems there is no way to declare > these in C++ using GNU attributes, and addrspace() seems to not be > supported by C++11 attribute syntax, which could possibly express > this. You can use __attribute__((address_space(N)) > > > > I have already been using that syntax to annotate pointer > declarations. However I don’t think it can be applied to methods the > way I was thinking. At least it doesn’t actually change them: > > > > struct Foo { > long x; > > __attribute__((address_space(7))) void bar(long y) { > printf("%ld %ld\n", x, y); > } > }; > > > Generates this declaration still: > > > define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo* %this, i64 %y) > ssp uwtable align 2 > > > Though I think desired behavior would be: > > > define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo addrspace(7) * > %this, i64 %y) ssp uwtable align 2 > > > (actually I think it’s more tricky because the address_space > attribute could be applying to the return type. I’d think the > correct way to specify the attribute on “this” would be to put it > where “const” goes, after the parentheses)Does it make sense for 'this' to be in one address space on some functions and in a different address space in other functions? If so, should the address space of 'this' contribute to overload resolution? -Hal> > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory
On 01/20/2014 03:29 PM, Hal Finkel wrote:> > 2. Is there a way to create methods that can be called with pointers > to different address spaces? It seems there is no way to declare > these in C++ using GNU attributes, and addrspace() seems to not be > supported by C++11 attribute syntax, which could possibly express > this. You can use __attribute__((address_space(N)) > > > > I have already been using that syntax to annotate pointer > declarations. However I don’t think it can be applied to methods the > way I was thinking. At least it doesn’t actually change them: > > > > struct Foo { > long x; > > __attribute__((address_space(7))) void bar(long y) { > printf("%ld %ld\n", x, y); > } > }; > > > Generates this declaration still: > > > define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo* %this, i64 %y) > ssp uwtable align 2 > > > Though I think desired behavior would be: > > > define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo addrspace(7) * > %this, i64 %y) ssp uwtable align 2 > > > (actually I think it’s more tricky because the address_space > attribute could be applying to the return type. I’d think the > correct way to specify the attribute on “this” would be to put it > where “const” goes, after the parentheses) > Does it make sense for 'this' to be in one address space on some functions and in a different address space in other functions? If so, should the address space of 'this' contribute to overload resolution? > > -HalYes it would. The struct itself has no specified address space, but the instances of it would. __attribute__((address_space(1)) Foo f1; __attribute__((address_space(2)) Foo f2; f1.bar() and f2.bar() would need to call functions with different address spaced this pointers. Support for this would be required to support the OpenCL static C++ extension> >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >>
I have since found many more places in Clangs CodeGen that doesnt take into account AddrSpaceCast when implicitly converting operands with BitCast. Is a better solution to instead just make BitCast check if the address spaces are different and apply an AddrSpaceCast instead? I made such a change in `llvm/lib/IR/Instructions.cpp` (see attached patch). This seems like it misses the point of adding the AddrSpaceCast, and breaks assumptions because when you insert a BitCast you may or may not insert an AddrSpaceCast instead. Perhaps the correct solution is to find all the cases in Clangs CodeGen where this may come up and ensure that we handle them all correctly... On Jan 25, 2014, at 2:55 PM, Brandon Holt <bholt at cs.washington.edu> wrote:> Bringing this to the attention of the Clang list because thats actually where my problems exist. > > Im working on using the address_space attribute to delineate special pointers in a pass Im working on. This lets me intercept uses of these pointers and plug in calls to my runtime wherever theres a load/store or other use. However, Im having problems trying to call methods on pointers with this attribute. > > I ran into a CodeGen problem where CodeGenFunction::EmitCall tried to `bitcast`an addrspace(N)* and failed an assertion. The attached patch is one possible fix, which is to use `addrspacecast` if the address spaces are different, otherwise just use `bitcast` as before. This has allowed me to then write my own pass to replace these instances with things from my runtime. > > However, I have a new problem now: the Clang type checker seems to choke converting between `__attribute__((address_space(N))` annotated types and const types, so that, though it allows me to call a non-const method on an addrspace pointer, it issues a compile error when I try to invoke a const method: > > struct Foo { > void bar() const { /* */ } > }; > > __attribute__((address_space(N)) Foo f; > > void main() { > f.bar(); > } > > Results in the following error message: > > error: cannot initialize object parameter of type 'const Foo' with an expression of type '__attribute__((address_space(200))) Foo' > f.bar(0); > > It seems to me that const/non-const is orthogonal to address space attributes, so if one method invocation works, the other should as well. So my question is: which behavior is intended to be correct? Should Clang allow methods on arbitrary address spaces? (if not, how would one write a method to be used on the address space version?) It seems like this question must come up in the OpenCL/CUDA extensions, but I am not familiar with those so dont know how this is handled there. > > Thanks, > -Brandon > > > > Begin forwarded message: > >> From: Matt Arsenault <Matthew.Arsenault at amd.com> >> Subject: Re: [LLVMdev] Methods on addrspace pointers >> Date: January 20, 2014 at 3:41:07 PM PST >> To: Hal Finkel <hfinkel at anl.gov>, Brandon Holt <bholt at cs.washington.edu> >> Cc: LLVM Developers Mailing List <llvmdev at cs.uiuc.edu> >> >> On 01/20/2014 03:29 PM, Hal Finkel wrote: >>> >>> 2. Is there a way to create methods that can be called with pointers >>> to different address spaces? It seems there is no way to declare >>> these in C++ using GNU attributes, and addrspace() seems to not be >>> supported by C++11 attribute syntax, which could possibly express >>> this. You can use __attribute__((address_space(N)) >>> >>> >>> >>> I have already been using that syntax to annotate pointer >>> declarations. However I dont think it can be applied to methods the >>> way I was thinking. At least it doesnt actually change them: >>> >>> >>> >>> struct Foo { >>> long x; >>> >>> __attribute__((address_space(7))) void bar(long y) { >>> printf("%ld %ld\n", x, y); >>> } >>> }; >>> >>> >>> Generates this declaration still: >>> >>> >>> define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo* %this, i64 %y) >>> ssp uwtable align 2 >>> >>> >>> Though I think desired behavior would be: >>> >>> >>> define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo addrspace(7) * >>> %this, i64 %y) ssp uwtable align 2 >>> >>> >>> (actually I think its more tricky because the address_space >>> attribute could be applying to the return type. Id think the >>> correct way to specify the attribute on this would be to put it >>> where const goes, after the parentheses) >>> Does it make sense for 'this' to be in one address space on some functions and in a different address space in other functions? If so, should the address space of 'this' contribute to overload resolution? >>> >>> -Hal >> >> Yes it would. The struct itself has no specified address space, but the instances of it would. >> >> __attribute__((address_space(1)) Foo f1; >> __attribute__((address_space(2)) Foo f2; >> >> f1.bar() and f2.bar() would need to call functions with different address spaced this pointers. Support for this would be required to support the OpenCL static C++ extension >> >>> >>>> >>>> >>>> _______________________________________________ >>>> 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/20140128/b469a5bb/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: implicit_addrspacecast.diff Type: application/octet-stream Size: 1127 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140128/b469a5bb/attachment.obj> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140128/b469a5bb/attachment-0001.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: fix_addrspace_call.diff Type: application/octet-stream Size: 1267 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140128/b469a5bb/attachment-0001.obj> -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140128/b469a5bb/attachment-0002.html>
On 01/28/2014 11:21 AM, Brandon Holt wrote:> I have since found many more places in Clangs CodeGen that doesnt > take into account AddrSpaceCast when implicitly converting operands > with BitCast. Is a better solution to instead just make BitCast check > if the address spaces are different and apply an AddrSpaceCast > instead? I made such a change in `llvm/lib/IR/Instructions.cpp` (see > attached patch).I've thought of trying that, but I think that will just hide many bugs. Most everywhere I've encountered that's trying to bitcast between address spaces is a bug from using the default.> > This seems like it misses the point of adding the AddrSpaceCast, and > breaks assumptions because when you insert a BitCast you may or may > not insert an AddrSpaceCast instead. Perhaps the correct solution is > to find all the cases in Clangs CodeGen where this may come up and > ensure that we handle them all correctly...Yes. I haven't done as thorough of a search through the uses in clang as I have in core LLVM for this.> > > On Jan 25, 2014, at 2:55 PM, Brandon Holt <bholt at cs.washington.edu > <mailto:bholt at cs.washington.edu>> wrote: > >> Bringing this to the attention of the Clang list because thats >> actually where my problems exist. >> >> Im working on using the address_space attribute to delineate >> special pointers in a pass Im working on. This lets me intercept >> uses of these pointers and plug in calls to my runtime wherever >> theres a load/store or other use. However, Im having problems >> trying to call methods on pointers with this attribute. >> >> I ran into a CodeGen problem where CodeGenFunction::EmitCall tried to >> `bitcast`an addrspace(N)* and failed an assertion. The attached patch >> is one possible fix, which is to use `addrspacecast` if the address >> spaces are different, otherwise just use `bitcast` as before. This >> has allowed me to then write my own pass to replace these instances >> with things from my runtime. >> >> However, I have a new problem now: the Clang type checker seems to >> choke converting between `__attribute__((address_space(N))` annotated >> types and const types, so that, though it allows me to call a >> non-const method on an addrspace pointer, it issues a compile error >> when I try to invoke a const method: >> >> struct Foo { >> void bar() const { /* */ } >> }; >> >> __attribute__((address_space(N)) Foo f; >> >> void main() { >> f.bar(); >> } >> >> Results in the following error message: >> >> error: cannot initialize object parameter of type 'const Foo' with an >> expression of type '__attribute__((address_space(200))) Foo' >> f.bar(0); >> >> It seems to me that const/non-const is orthogonal to address space >> attributes, so if one method invocation works, the other should as >> well. So my question is: which behavior is intended to be correct? >> Should Clang allow methods on arbitrary address spaces? (if not, how >> would one write a method to be used on the address space version?) It >> seems like this question must come up in the OpenCL/CUDA extensions, >> but I am not familiar with those so dont know how this is handled there. >> >> Thanks, >> -Brandon >> >> >> Begin forwarded message: >> >>> *From: *Matt Arsenault <Matthew.Arsenault at amd.com >>> <mailto:Matthew.Arsenault at amd.com>> >>> *Subject: **Re: [LLVMdev] Methods on addrspace pointers* >>> *Date: *January 20, 2014 at 3:41:07 PM PST >>> *To: *Hal Finkel <hfinkel at anl.gov <mailto:hfinkel at anl.gov>>, Brandon >>> Holt <bholt at cs.washington.edu <mailto:bholt at cs.washington.edu>> >>> *Cc: *LLVM Developers Mailing List <llvmdev at cs.uiuc.edu >>> <mailto:llvmdev at cs.uiuc.edu>> >>> >>> On 01/20/2014 03:29 PM, Hal Finkel wrote: >>>> >>>> 2. Is there a way to create methods that can be called with pointers >>>> to different address spaces? It seems there is no way to declare >>>> these in C++ using GNU attributes, and addrspace() seems to not be >>>> supported by C++11 attribute syntax, which could possibly express >>>> this. You can use __attribute__((address_space(N)) >>>> >>>> >>>> >>>> I have already been using that syntax to annotate pointer >>>> declarations. However I dont think it can be applied to methods the >>>> way I was thinking. At least it doesnt actually change them: >>>> >>>> >>>> >>>> struct Foo { >>>> long x; >>>> >>>> __attribute__((address_space(7))) void bar(long y) { >>>> printf("%ld %ld\n", x, y); >>>> } >>>> }; >>>> >>>> >>>> Generates this declaration still: >>>> >>>> >>>> define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo* %this, i64 %y) >>>> ssp uwtable align 2 >>>> >>>> >>>> Though I think desired behavior would be: >>>> >>>> >>>> define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo addrspace(7) * >>>> %this, i64 %y) ssp uwtable align 2 >>>> >>>> >>>> (actually I think its more tricky because the address_space >>>> attribute could be applying to the return type. Id think the >>>> correct way to specify the attribute on this would be to put it >>>> where const goes, after the parentheses) >>>> Does it make sense for 'this' to be in one address space on some >>>> functions and in a different address space in other functions? If >>>> so, should the address space of 'this' contribute to overload >>>> resolution? >>>> >>>> -Hal >>> >>> Yes it would. The struct itself has no specified address space, but >>> the instances of it would. >>> >>> __attribute__((address_space(1)) Foo f1; >>> __attribute__((address_space(2)) Foo f2; >>> >>> f1.bar() and f2.bar() would need to call functions with different >>> address spaced this pointers. Support for this would be required to >>> support the OpenCL static C++ extension >>> >>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> 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/20140128/9c994d17/attachment.html>