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

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Nov 25 23:41:26 PST 2018


rjmccall 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);
+
----------------
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.


================
Comment at: lib/CodeGen/CGCall.cpp:4035
+          V = Builder.CreatePointerBitCastOrAddrSpaceCast(V, DestTy);
+        }
 
----------------
Always use the `performAddrSpaceConversion` target hook if there's a semantic address-space conversion required.  But really, this doesn't seem like the right place to be doing this; it ought to happen higher up when we're adding the `this` argument to the call, either explicitly in IRGen or implicitly by expecting the object expression to already yield a value in the right address space.

I could definitely believe that we don't currently create `CastExpr`s for simple qualification conversions of the object argument of a C++ method call, since (ignoring these address-space conversions) they're always trivial.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54862





More information about the cfe-commits mailing list