Ayal added a comment. In D143849#4121348 <https://reviews.llvm.org/D143849#4121348>, @Anastasia wrote:
> I feel that originally pointers were disallowed because they create the same > issue as `size_t` and etc as their size is implementation depended but the > same logic applies to images and other types that are even more > implementation depended. Overall this bit of the spec is very inconsistent so > I don't mind if we change the behavior to be whatever we find more helpful. > However I would encourage to submit an issue to OpenCL-Docs to point out this > inconsistency. Yeah, pointer sizes need to match, pointers should be passed to clSetKernelExecInfo, and SVM should be employed, which the spec should clarify. In any case the quoted restriction "s6.9.p" of https://registry.khronos.org/OpenCL/sdk/1.2/docs/man/xhtml/restrictions.html justifying the SemA enforcement is clearly absent in https://registry.khronos.org/OpenCL/sdk/2.0/docs/man/xhtml/restrictions.html. ================ Comment at: clang/lib/Sema/SemaDecl.cpp:9493 + // union. This restriction was lifted in OpenCL v2.0 with the introduction + // of SVM. + if (S.getLangOpts().getOpenCLCompatibleVersion() > 120 && ---------------- Anastasia wrote: > To be honest I feel like it was a bug fix? Do you happen to have any record > of the fix? Not sure what was the alleged bug nor its fix, but in any case I have no record thereof.. ================ Comment at: clang/lib/Sema/SemaDecl.cpp:9494 + // of SVM. + if (S.getLangOpts().getOpenCLCompatibleVersion() > 120 && + (ParamType == PtrKernelParam || ParamType == PtrPtrKernelParam)) ---------------- Anastasia wrote: > I think it should be possible to merge this with `if` below to avoid > condition duplication. > > Sure, but that trades one duplication for another, rather than clearly separating the early-continue case early? ``` if (ParamType == PtrKernelParam || ParamType == PtrPtrKernelParam) { if (S.getLangOpts().getOpenCLCompatibleVersion() > 120) continue; S.Diag(Param->getLocation(), diag::err_record_with_pointers_kernel_param) << PT->isUnionType() << PT; } else if (ParamType == InvalidAddrSpacePtrKernelParam) { S.Diag(Param->getLocation(), diag::err_record_with_pointers_kernel_param) << PT->isUnionType() << PT; } else { S.Diag(Param->getLocation(), diag::err_bad_kernel_param_type) << PT; ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143849/new/ https://reviews.llvm.org/D143849 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
