azabaznov added a comment. > Ok, addressing in a separate patch is reasonable, but why do you think that > we will break backward compatibility? My current worry is that the > implementation is so messy and inconsistent that it will take us longer time > if we do the incremental steps. Also, we risk introducing the regressions > since it is so hard to make sense out of it.
As regarding backward compatibility -- they may be kernels which already rely on pragmas. So maybe their users already expect to see something like: //some_type requires some_ext to be enabled//. If we remove check for pragma their will be no diagnostic at all > This issue has been reported 2 years ago but there was very little progress > made since then. I see :( Maybe we should introduce diagnostic, something like: //pragma enable is deprecated: there is no need to enable extension to use optional functionally//. But anyway it seems really impactful as current specification allows using pragma... ================ Comment at: clang/lib/Sema/Sema.cpp:339 + setOpenCLExtensionForType(AtomicDoubleT, "cl_khr_fp64"); + Atomic64BitTypes.push_back(AtomicDoubleT); + } ---------------- Anastasia wrote: > Side comment: I don't see why would `atomic_double` have anything to do with > `cl_khr_int64_base_atomics` or `cl_khr_int64_extended_atomics`? If anything > the extensions should have been named differently... Yeah... With the fact given that `atomic_int` for example by default. Intuitively I expected to see only `cl_khr_fp64` required. OpenCL C 3.0 spec, 6.15.12.6. Atomic integer and floating-point types: ... atomic_double [54] ... [54]: //The atomic_double type is only supported if double precision is supported and the cl_khr_int64_base_atomics and cl_khr_int64_extended_atomics extensions are supported and have been enabled. If this is the case then an OpenCL C 3.0 compiler must also define the __opencl_c_fp64 feature//. Does it seem like spec issue/inaccuracy? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97058/new/ https://reviews.llvm.org/D97058 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
