[PATCH] D95776: [OpenCL] Add macro definitions of OpenCL C 3.0 features

Sven van Haastregt via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 2 04:58:38 PST 2021


svenvh added inline comments.


================
Comment at: clang/lib/Headers/opencl-c.h:17161
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ == 200)
+#undef __opencl_c_pipes
+#undef __opencl_c_generic_address_space
----------------
Anastasia wrote:
> Looping in @svenvh  - I don't mind if we define those macros in headers for OpenCL 2.0. The only concern is that if we `undef` them here we will end up with different behavior between `-finclude-default-header` and `-fdeclare-opencl-builtins`. I would suggest not to `undef` them because it is better if we have coherency. Alternatively we could also add a third header with undefs that can be included at the end for both but it seems to make things even more complicated.
> 
> FYI `__opencl_c_int64` is already added for all OpenCL versions.
> The only concern is that if we undef them here we will end up with different behavior between -finclude-default-header and -fdeclare-opencl-builtins

This is a valid point.  Doing the undefs only in `opencl-c.h` will lead to a problem similar to https://reviews.llvm.org/D91429 .

A third header just for the undefs sounds like a bit of an overkill indeed, although having duplication isn't great either.  Not sure what's best to be honest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95776



More information about the cfe-commits mailing list