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? https://reviews.llvm.org/D47154 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits