sidorovd marked an inline comment as done. sidorovd added inline comments.
================ Comment at: test/SemaOpenCL/extension-begin.cl:26 + #ifndef IMPLICIT_INCLUDE #include "extension-begin.h" ---------------- Anastasia wrote: > sidorovd wrote: > > Anastasia wrote: > > > sidorovd wrote: > > > > Anastasia wrote: > > > > > Can we also test that macro `my_ext` is not defined here but defined > > > > > above? > > > > > > > > > > It seems we are not testing anything like this... > > > > #pragma OPENCL EXTENSION my_ext : begin doesn't define an appropriate > > > > macro. And so cl-ext=+my_ext. > > > But don't you need to expose the definition of it? > > Certainly I need, but now the only proper way to do so is by adding an > > extension via adding it in OpenCLExtensions.def. Previously we decided to > > avoid adding an extension directly into clang, so with a new approach I'd > > prefer not to add a definition of the macro in the header but define it > > somewhere else, otherwise the macro becomes defined where it's not > > supposed to be (even for ARM and AMD =) ). > However, my understanding is that you should define the macro when you define > the extension itself. > > > ``` > #pragma OPENCL EXTENSION my_ext : begin > #define my_ext > ... > #pragma OPENCL EXTENSION my_ext : end > ``` > > does it not work for you? ``` #pragma OPENCL EXTENSION my_ext : begin #define my_ext ... #pragma OPENCL EXTENSION my_ext : end ``` ^ I It defines the macro regardless begin : end range, so one can consider that it's the same code as: ``` #define my_ext #pragma OPENCL EXTENSION my_ext : begin ... #pragma OPENCL EXTENSION my_ext : end ``` I agree, that it's a better way to define a macro with defining an appropriate extension itself, but it makes it be defined where it's not supposed to be (at least from my perspective). To sum up: 1. #pragma OPENCL EXTENSION my_ext : begin ... end - makes an extension known for clang if the header included and if the extension enabled; doesn't affect a macro definition anyhow; 2. OPENCLEXT_INTERNAL(my_ext, *version*, ~0U) - makes an extension known for clang and defines an appropriate macro if the extension enabled. I believe we are okay not to define cl_intel_planar_yuv macro in the header - just make the extension known for clang. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58666/new/ https://reviews.llvm.org/D58666 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
