[OpenCL] Adding cl_intel_planar_yuv extension

[sidorovd]

Summary

Just adding a preprocessor #define for the extension.

Patch by Alexey Sotkin

[Anastasia]

Will this extension require any Clang changes that are not possible to add using this approach for vendor extensions:
https://clang.llvm.org/docs/UsersManual.html#opencl-extensions

If not I would suggest to add this to the header file instead.

[sidorovd]

I'm not sure if I put pragmas in an appropriate place (I mean in the very beginning of the header).

[Anastasia]

lib/Headers/opencl-c.h

26 ↗

(On Diff #164157)

@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...

[Anastasia]

lib/Headers/opencl-c.h

26 ↗

(On Diff #164157)

@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?

[sidorovd]

lib/Headers/opencl-c.h

26 ↗

(On Diff #164157)

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:

extensions.xlsx10 KBDownload

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?

[sidorovd]

@Anastasia , since there is a problem I described, wouldn't you mind if I stay with the first iteration of the patch (adding the extension to OpenCLExtensions.def) and after we'll investigate what is wrong with -cl-ext approach?

[Anastasia]

In D51402#1252619, @sidorovd wrote:

@Anastasia , since there is a problem I described, wouldn't you mind if I stay with the first iteration of the patch (adding the extension to OpenCLExtensions.def) and after we'll investigate what is wrong with -cl-ext approach?

I would prefer not to add it to Clang directly unless absolutely necessary. I think it's ok to add this in the header for now as is, but then investigate how we can add an architecture guard later. Please, could you just move the testing into test/Headers/opencl-c-header.cl, because test/SemaOpenCL/extension-version.cl is for Clang built in extensions.

[sidorovd]

Moved the test to test/Headers/opencl-c-header.cl

[sidorovd]

• Forgot to add clarification comments on conditional branching. Added.

[sidorovd]

Ping

[Anastasia]

LGTM!

[asavonic]

Committed revision 345044

include/clang/Basic/OpenCLExtensions.def

 
 // AMD OpenCL extensions
 OPENCLEXT_INTERNAL(cl_amd_media_ops, 100, ~0U)
 OPENCLEXT_INTERNAL(cl_amd_media_ops2, 100, ~0U)
 
 // Intel OpenCL extensions
 OPENCLEXT_INTERNAL(cl_intel_subgroups, 120, ~0U)
 OPENCLEXT_INTERNAL(cl_intel_subgroups_short, 120, ~0U)
+OPENCLEXT_INTERNAL(cl_intel_planar_yuv, 120, ~0U)
 
 #undef OPENCLEXT_INTERNAL
 
 #ifdef OPENCLEXT
 #undef OPENCLEXT
 #endif

test/SemaOpenCL/extension-version.cl

 #ifndef cl_intel_subgroups_short
 #error "Missing cl_intel_subgroups_short define"
 #endif
 #else
 // expected-warning@+2{{unsupported OpenCL extension 'cl_intel_subgroups_short' - ignoring}}
 #endif
 #pragma OPENCL EXTENSION cl_intel_subgroups_short : enable
 
+#if (__OPENCL_C_VERSION__ >= 120)
+#ifndef cl_intel_planar_yuv
+#error "Missing cl_intel_planar_yuv define"
+#endif
+#else
+// expected-warning@+2{{unsupported OpenCL extension 'cl_intel_planar_yuv' - ignoring}}
+#endif
+#pragma OPENCL EXTENSION cl_intel_planar_yuv : enable