[PATCH] D47154: Try to make builtin address space declarations not useless

Anastasia Stulova via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 22 10:26:08 PDT 2018

Anastasia added inline comments.

Comment at: include/clang/Basic/BuiltinsAMDGPU.def:49
+// FIXME: Need to disallow constant address space.
 BUILTIN(__builtin_amdgcn_div_scale, "dddbb*", "n")
Do you plan to provide the support for it later? Or if else perhaps we should elaborate more what's to be done.

Comment at: include/clang/Basic/TargetInfo.h:1009
+  virtual LangAS getOpenCLBuiltinAddressSpace(unsigned AS) const {
+    return getLangASFromTargetAS(AS);
Can you add a comment please to explain what the function is for?

Comment at: lib/AST/ASTContext.cpp:9093
       unsigned AddrSpace = strtoul(Str, &End, 10);
-      if (End != Str && AddrSpace != 0) {
-        Type = Context.getAddrSpaceQualType(Type,
-                                            getLangASFromTargetAS(AddrSpace));
+      if (End != Str) {
+        // Note AddrSpace == 0 is not the same as an unspecified address space.
Could we check against LangAS::Default instead of removing this completely.

Comment at: lib/CodeGen/CGBuiltin.cpp:3500
+        if (auto *PtrTy = dyn_cast<llvm::PointerType>(PTy)) {
+          if (PtrTy->getAddressSpace() !=
+              ArgValue->getType()->getPointerAddressSpace()) {
Would this be correct for OpenCL? Should we use  `isAddressSpaceSupersetOf` helper instead? Would it also sort the issue with constant AS (at least for OpenCL)? 

Comment at: test/CodeGenOpenCL/numbered-address-space.cl:36
+#if 0
+// XXX: Should this compile?
+void test_generic_as_to_builtin_parameter_explicit_cast_numeric(__attribute__((address_space(3))) int *local_ptr, float src) {
`__attribute__((address_space(N)))` is not an OpenCL feature and I think it's not specified in C either? But I think generally non matching address spaces don't compile in Clang. So it might be useful to disallow this?


More information about the cfe-commits mailing list