[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh requested changes to this revision.
svenvh added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/lib/Headers/opencl-c-base.h:24
 #define cl_khr_subgroup_clustered_reduce 1
+#define cl_ext_float_atomics
+#ifdef cl_khr_fp16
----------------
Anastasia wrote:
> svenvh wrote:
> > Should this be defined as `1`?
> > 
> > Should this define be tested in `clang/test/Headers/opencl-c-header.cl` too?
> Actually, now that I think more about this, it seems incorrect to add this here without adding the functions to `OpenCLBuiltins.td` because then the feature macro will be present without the feature when the default header is used?
> 
> So I guess we either need to extend `OpenCLBuiltins.td` with new functions or move the new macros into `opencl-c.h`?
Good catch!  Indeed, adding the define in the shared `opencl-c-base.h` without also providing the builtins through `OpenCLBuiltins.td` is incorrect.

The preferred solution would be to add the new builtins to `clang/lib/Sema/OpenCLBuiltins.td` too, to avoid diverging the header and tablegen-driven code paths.


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

https://reviews.llvm.org/D106343