[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: lib/CodeGen/CGCall.cpp:80
+    // used with the same version of generated operators.
+    RecTy = Context.getAddrSpaceQualType(RecTy, LangAS::opencl_generic);
+
----------------
rjmccall wrote:
> Anastasia wrote:
> > rjmccall wrote:
> > > I would suggest taking this opportunity to set up the AST to support declaring methods in an arbitrary address space, so that you can just ask a `CXXMethodDecl` what address space it's in.  You don't have to actually add language support for that — OpenCL C++ would simply change the it to the generic address space instead of the default — but I think that's the right technical approach for implementing this, as opposed to adding a bunch of OpenCL C++ -specific logic all over the compiler that just hardcodes a different address space.
> > I quite like this idea. Apart from providing more clean implementation, it opens opportunities for solving several problems that I am trying to understand how to address. Specifically I am trying to find a way to 'overload' methods based on the address space of the object.
> > 
> > For example, if an object is created in the address space 1 then programmers should be able to provide a method to be used for objects in such address space for efficiency or even correctness issue.
> > 
> > The reasons I am looking at it is that currently C++ doesn't make much sense for address spaces, because we are removing them to generate just one implementation with generic/default address space. However,
> > - Not all address spaces can be converted to generic/default address space. Example in OpenCL is constant AS that can't be converted to any other.
> > - Higher performance can be achieved on some HW when using specific address spaces instead of default.
> > 
> > I was wondering if a method qualifier is a good language solution for this? For example in OpenCL we could write something like:
> > 
> >   class foo
> >   {
> >   public:
> >     void bar() __private; // implies bar(__private foo*)
> >     void bar() __constant; // implies bar(__constant foo*)
> >   };
> > 
> > I guess in C++ it can be done similarly:
> > 
> >   class foo
> >   {
> >   public:
> >     void bar() __attribute__((address_space(1)));
> >     void bar() __attribute__((address_space(2)));
> >   };
> > 
> > I would quite like to solve this generically, not just for OpenCL. I think a lot of implementation can be unified/reused then.
> > 
> > Without this address spaces seem pretty useless with C++ because they are just cast away to generic/default and no specific address space ends up at the AST level at all. This means implementation will have to rely on the optimizers to recover/deduce address spaces. But I would quite like to provide a way for the developers to manually tune the code for address spaces, just as it was done for OpenCL C.
> > 
> > Let me know if you have any thought/suggestions.
> > I was wondering if a method qualifier is a good language solution for this? For example in OpenCL we could write something like:
> 
> Yes, I think that's a very natural extension of C++'s method-qualification rules for `const` and `volatile`.  Overloads would then be resolved based on which address space was the best match.
> 
> Now, to briefly take a holistic perspective on the language design, this feature would *strongly* benefit from a way to make a method templated over the address space of `this`.  Unfortunately, I don't think that's reasonable to solve in a language extension; it's really something that needs core language work.  That would be a pretty big leap in scope; that said, if you're interested in pursuing it, I'd be happy to share some thoughts on how it'd look, and I think there are several people in the Clang community who could help you with putting a proposal before the committee.
Thanks! Yes, I believe some template-based approach could make this a lot more usable. I would be really interested to learn more about your ideas. I would also quite like to see this properly in the spec. I will follow up with you in a separate thread.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54862