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