[PATCH] D51402: [OpenCL] Adding cl_intel_planar_yuv extension

[Dmitry Sidorov via Phabricator via cfe-commits]

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