[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

Headers/opencl-c-header.cl

-// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s| FileCheck %s
-// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -cl-std=CL1.1| FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify | FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.1| FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.2| FileCheck %s
 
 // Test including the default header as a module.
 // The module should be compiled only once and loaded from cache afterwards.
 // Change the directory mode to read only to make sure no new modules are created.
 // Check time report to make sure module is used.
 // Check that some builtins occur in the generated IR when called.
 
 // ===

 
 // CHECK20: _Z12write_imagef14ocl_image3d_wo
 #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 void test_image3dwo(write_only image3d_t img) {
   write_imagef(img, (0), (0.0f));
 }
 #endif //__OPENCL_C_VERSION__
 
+// Verify that non-builtin cl_intel_planar_yuv extension is defined from
+// OpenCL 1.2 onwards.
+#if (__OPENCL_C_VERSION__ >= CL_VERSION_1_2)
+// expected-no-diagnostics
+#ifndef cl_intel_planar_yuv
+#error "Missing cl_intel_planar_yuv define"
+#endif
+#else //__OPENCL_C_VERSION__
+// expected-warning@+2{{unknown OpenCL extension 'cl_intel_planar_yuv' - ignoring}}
+#endif //__OPENCL_C_VERSION__
+#pragma OPENCL EXTENSION cl_intel_planar_yuv : enable
+
 // CHECK-MOD: Reading modules

Headers/opencl-c.h

 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
 
 #if __OPENCL_C_VERSION__ < CL_VERSION_2_0
 #ifdef cl_khr_3d_image_writes
 #pragma OPENCL EXTENSION cl_khr_3d_image_writes : enable
 #endif //cl_khr_3d_image_writes
 #endif //__OPENCL_C_VERSION__ < CL_VERSION_2_0
 
+#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+#ifndef cl_intel_planar_yuv
+#define cl_intel_planar_yuv
+#endif // cl_intel_planar_yuv
+#pragma OPENCL EXTENSION cl_intel_planar_yuv : begin
+#pragma OPENCL EXTENSION cl_intel_planar_yuv : end
+#endif // __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+
 #define __ovld __attribute__((overloadable))
 #define __conv __attribute__((convergent))
 
 // Optimizations
 #define __purefn __attribute__((pure))
 #define __cnfn __attribute__((const))
 
 // built-in scalar data types: