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

[Mikael Nilsson via Phabricator via cfe-commits]

mikael marked 3 inline comments as done.
mikael added inline comments.


================
Comment at: lib/AST/Type.cpp:2950
+   FunctionTypeBits.HasExtQuals = 0;
+  }
 }
----------------
rjmccall wrote:
> The indentation here is messed up.
> 
> You seem to be mixing up "fast qualifiers" with "CVR qualifiers" in a few places.  That's currently correct, in the sense that the fast qualifiers are currently defined to be the CVR qualifiers, but the point of the distinction is that we might want to change that (and we have in fact explored that in the past).  I think you should make `FunctionTypeBits` only claim to store the fast qualifiers, which mostly just means updating a few field / accessor names and changing things like the `getCVRQualifiers()` call right above this to be `getFastQualifiers()`.
> 
> If there are places where it's convenient to assume that the CVR qualifiers are exactly the fast qualifiers, it's okay to static_assert that, but you should make sure to static_assert it in each place you assume it.
I'll change it, but I do have a question related to this:

Can we make any assumptions with the "fast qualifiers"? I'd like it if we can assume that they fit in 3 bits. Otherwise I think I also need an assertion here.


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:2593
+      getOrCreateInstanceMethodType(
+          CXXMethodDecl::getThisType(FPT, Ty->getMostRecentCXXRecordDecl()),
+          FPT, U),
----------------
rjmccall wrote:
> Why is this `getMostRecentCXXRecordDecl` instead of `getClass`?
Your initial comment suggested to send in a record rather than a type. But I see now that it may make more sense to use the type directly instead. Will update it.


================
Comment at: lib/CodeGen/CGDeclCXX.cpp:32
+       (D.hasLocalStorage() && CGF.getContext().getLangOpts().OpenCLCPlusPlus)) &&
+      "VarDecl must have global or local (in the case of OpenCL) storage!");
   assert(!D.getType()->isReferenceType() &&
----------------
rjmccall wrote:
> What causes OpenCL C++ declarations to go down this path?
It goes there when we defined an object  in __local address space.  From the spec "The constructors of a objects in local memory should only be initialized once".  This is the same case as with C++ 11 static local variables.


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

https://reviews.llvm.org/D54862