[PATCH] D100980: [OpenCL] Allow use of double type without extension pragma

Anastasia Stulova via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 11 02:31:28 PDT 2021


Anastasia added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10006
   "use of %select{type|declaration}0 %1 requires %2 support">;
+def ext_opencl_double_without_pragma : Extension<
+  "Clang permits use of type 'double' regardless pragma if 'cl_khr_fp64' is supported">;
----------------
azabaznov wrote:
> Anastasia wrote:
> > azabaznov wrote:
> > > nit: this can be extended to use arbitrary type and extension for other patches which will eliminate pragmas for types
> > Well, actually I am not sure we should use it elsewhere because I don't think it really applies universally.
> > The situation for `doubles` seems to be that the older specs were intructing to use the pragma explicitly.
> > 
> > For the future work we should just avoid adding or using pragma at all unless it brings beneficial functionality.  
> > Well, actually I am not sure we should use it elsewhere because I don't think it really applies universally.
> > The situation for doubles seems to be that the older specs were intructing to use the pragma explicitly.
> 
> The same applies for atomic types, for example (https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_C.html#_footnoteref_55):
> 
> //If the device address space is 64-bits, the data types atomic_intptr_t, atomic_uintptr_t, atomic_size_t and atomic_ptrdiff_t are supported if the cl_khr_int64_base_atomics and cl_khr_int64_extended_atomics **extensions are supported and have been enabled**.//
> 
> It seems for me that simplifying the implementation in a way that enabling pragma is not necessary is fine if such warning is diagnosed for atomic types and half type as well. Alternatively, maybe the spec should be fixed by adding `__opencl_c_fp16` and etc. as optional core features?
Ok, this is a different issues in my view:
a. It doesn't explicitly mention the pragma although I assume it is implied.
b. It assumes the pragmas could load and unload the extensions but it has not been implemented correctly. As a matter of fact I am removing the requirements for the pragma on atomics completely in https://reviews.llvm.org/D100976 because I see no value in implementing it half way ending up in incorrect and inconveninent functionality.

Double support is different to atomics because `double` is actually a reserved identifier so disabling it actually works as expected.


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

https://reviews.llvm.org/D100980



More information about the cfe-commits mailing list