sidorovd added inline comments.

================
Comment at: lib/Headers/opencl-c.h:26
+#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+#ifndef cl_intel_planar_yuv
+#define cl_intel_planar_yuv
----------------
Anastasia wrote:
> Anastasia wrote:
> > @yaxunl, do you think we need to add some kind of architecture guard for 
> > such things? Like it should only be added if the architecture supports the 
> > extension? But I guess `-cl-ext=+cl_intel_planar_yuv` trick might not work 
> > here because it's not a Clang known extension?
> > 
> > So may be the right solution here is to introduce a target specific header? 
> > For now it can be explicitly included but we could think of a target hook 
> > to preload a target specific header...
> @sidorovd, I am wondering if we could instead extend 
> '-cl-ext=+cl_intel_planar_yuv' to work with non-builtin extensions? Would 
> that be an acceptable solution for vendor extensions to be added to the 
> common header?
Sorry for a long response.
I have covered a little study on how to enable extensions via adding them in 
clang, common header or without any changes (yeap, that is also a case) and how 
adding -cl-ext+ in the RUN string affects the test's (introduced in this patch, 
but with removed versioning) result. And found out that I don't understand how 
it works or it's supposed to work. Here is a result of my study: {F7312300}
                                 

|  Changes      | cl-ext+ | cl-ext- | no cl-ext |  
| opencl-c.h        ver  >=1.2 | defined, known, supported     | defined, 
known, supported                | defined, known, supported            |
| opencl-c.h       ver < 1.2   | undefined, known, supported | undefined, 
unknown                          | undefined, unknown                      |  
| OpenCLExtensions.def  ver  >=1.2 | defined, known, supported         |   
undefined, known, unsupported   |  defined, known, supported          |
| OpenCLExtensions.def  ver < 1.2   | undefined, known, unsupported | 
undefined, known, unsupported     |    undefined, known, unsupported |
| No changes  ver  >=1.2 | undefined, known, supported    | undefined, unknown  
                      | undefined, unknown                     |
| No changes  ver < 1.2    | undefined, known, supported   | undefined, unknown 
                        | undefined, unknown                    |

So from my perspective there shouldn't be any difference between the result 
depending on an approach to use to enable an extension. But that is not how 
it's happening in reality, as an example see how values put in column #3 
differs for changes in opencl-c.h and OpenCLExtensions.def.
Or am I wrong?






https://reviews.llvm.org/D51402



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

Reply via email to