azabaznov added inline comments.

================
Comment at: clang/lib/Basic/Targets.cpp:743
+  // Assume compiling for FULL profile
+  Builder.defineMacro("__opencl_c_int64");
 }
----------------
Anastasia wrote:
> Btw we could add the other feature macros for earlier versions too but I 
> guess it makes code more complicated?
Yes, this will complicate things: we decided to generate a warning if any of 
core features is unsupported, right? So making OpenCL C 3.0 features as core in 
OpenCL C 2.0 will result in this kind of warning; distinguishing these features 
among the set of core functionality may require workarounds in clang, so let's 
keep them in headers only for OpenCL C 2.0.


================
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
----------------
svenvh wrote:
> 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.
My idea was just to temporary define features for built-ins and enums 
definitions as this is the only place where these macros may be useful for 
OpenCL C 2.0. However, I don't have any strong concerns if these macros will 
remain defined.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95776

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to