[RFC][OpenCL] Provide mechanisms for defining extension macros

[Anastasia]

Summary

Background:
Extensions in OpenCL allow adding extra functionality to the core standards. The analogy to them in C/C++ are libraries. They are not to be confused with language extensions in Clang that modify the standard language behavior. Apart from very few exceptions that add extra language features.

OpenCL differs to the standard C/C++ because the application code doesn't need to include library headers or to link to the library functions.

There are a number of extensions that are added to Clang without any need for changes in parsing (see list II in RFC: http://lists.llvm.org/pipermail/cfe-dev/2020-September/066911.html). Such extensions are implemented only in the libraries. Clang now supports internal headers that can be included implicitly and provide declarations for the majority of builtin library like functionality. Without the internal header, the functionality of the stand-alone frontend is incomplete. Therefore it makes more sense to move the extensions that add functionality using regular libraries mechanisms outside of clang. The benefit of such refactoring are:

1. Simplify the frontend code
2. Allow adding extensions flexibly without the dependency on clang source code
3. Make user interface more coherent i.e. the extensions macro are now only available when the declarations of functionality from the extensions are available too - via implicit headers.

Design:

The design is illustrated on cl_khr_depth_images. This extension has only been added to guard the declaration of corresponding functions in the headers by the extension macro. The use of extension pragma has no effect and setting the extension by targets doesn't change anything but the definition of the macro. This, therefore, shows that the only reason why the extension is added to clang is to define the macro. Moreover, the definition of this macro has also been modified in opencl-c.h. Please note that presumably the depth image types could be made available in the earlier OpenCL versions via this extension but however it has never been implemented. Therefore, it is assumed there is no interest in this in the community.

A new hook to define macros for OpenCL extensions has been added to TargetInfo that can be overridden to define the macros as needed by the targets and additionally it is shown that macros can be defined in the header files completely outside of the clang source code. The preference (at least for SPIR target) is to define the macros in the implicit headers.

Examples:

1. In the following invocation of clang the macro cl_khr_depth_images is not defined.

clang -cc1 -finclude-default-header -triple=spir -cl-std=CL1.1 test.cl 
clang -cc1 -finclude-default-header -triple=ppc -cl-std=CL1.1 test.cl 
clang -cc1 -finclude-default-header -triple=ppc -cl-std=CL1.2 test.cl

2. In the following invocation of clang the macro cl_khr_depth_images is defined.

clang -cc1 -finclude-default-header -triple=spir -cl-std=CL1.2 test.cl
clang -cc1 -finclude-default-header -triple=spir -cl-std=CL2.0 test.cl 
clang -cc1 -finclude-default-header -triple=ppc -cl-std=CL2.0 test.cl

3. Defining __undef_cl_khr_depth_images can alter the default behavior of the predefined macro. This is equivalent to passing -cl-ext=-cl_khr_depth_images.

• In the following invocation of clang the macro cl_khr_depth_images is not defined.

clang -cc1 -finclude-default-header -triple=spir -cl-std=CL1.2 test.cl -D__undef_cl_khr_depth_images

• cl_khr_depth_images is a core functionality of CL2.0 and thefore defining __undef_cl_khr_depth_images doesn't modify the default behavior.

clang -cc1 -finclude-default-header -triple=spir -cl-std=CL1.2 test.cl -D__undef_cl_khr_depth_images

NOTE
The refactoring of extension pragma and tablegen header (enabled via -fdeclare-opencl-builtins) are going to be covered in a separate RFC patches.

[Anastasia]

• Added full diffs.

[azabaznov]

Yes, in general this approach looks good to me conceptually. I have two suggestions:

1. As we discussed, the term core functionality should be revisited here. There's no clear meaning about that in the spec and I think interpreting it as supported by default is a little dangerous. So core (AFAIK) means that it was just promoted to a core specification thus is still remains optional by targets.
2. Sort of a implementation suggestion. I understand that double-scored identifiers are reserved for any use, but still, can defining such macro as __undef_cl_khr_depth_images be avoided? We could use Preproceccor class for the things that you are proposing to do. I was trying to do something similar when implementing features and I tried something like (Preprocessor::appendDefMacroDirective already exists):

UndefMacroDirective *Preprocessor::appendUndefMacroDirective(IdentifierInfo *II,
                               SourceLocation Loc) {
   if (!II->hasMacroDefinition())
     return nullptr;
   UndefMacroDirective *UD = AllocateUndefMacroDirective(Loc);
   appendMacroDirective(II, UD);
   return UD;
}

I tried to handle some special pragma in this way and it worked. So maybe this can be reused without binding to any specific SourceLocation? But maybe there is an other strong concern why Preprocessor::appendUndefMacroDirective still doesn't exist...

[Anastasia]

In D91531#2406390, @azabaznov wrote:

Yes, in general this approach looks good to me conceptually. I have two suggestions:

1. As we discussed, the term core functionality should be revisited here. There's no clear meaning about that in the spec and I think interpreting it as supported by default is a little dangerous. So core (AFAIK) means that it was just promoted to a core specification thus is still remains optional by targets.

Btw I think there is a core and an optional core extension too? I believe that the definition that you are providing applies to the optional core extension but not core extension. However I have created this issue as I think the spec should be explicit about all of these: https://github.com/KhronosGroup/OpenCL-Docs/issues/500.

1. Sort of a implementation suggestion. I understand that double-scored identifiers are reserved for any use, but still, can defining such macro as __undef_cl_khr_depth_images be avoided? We could use Preproceccor class for the things that you are proposing to do. I was trying to do something similar when implementing features and I tried something like (Preprocessor::appendDefMacroDirective already exists):

UndefMacroDirective *Preprocessor::appendUndefMacroDirective(IdentifierInfo *II,
                               SourceLocation Loc) {
   if (!II->hasMacroDefinition())
     return nullptr;
   UndefMacroDirective *UD = AllocateUndefMacroDirective(Loc);
   appendMacroDirective(II, UD);
   return UD;
}

I tried to handle some special pragma in this way and it worked. So maybe this can be reused without binding to any specific SourceLocation? But maybe there is an other strong concern why Preprocessor::appendUndefMacroDirective still doesn't exist...

As far as I can see Preprocessor::appendDefMacroDirective is mainly used for the extension https://gcc.gnu.org/onlinedocs/gcc/Push_002fPop-Macro-Pragmas.html. It seems interesting but I am not sure yet if it can help. So what do you plan to do with Preprocessor::appendUndefMacroDirective? Perhaps if you give me an example it would help to understand.

I agree that __undef macro is a bit messy. We could of course make it a bit prettier. For example,

(1.) we could add a helper macro

#define DEFINE_EXTENSION_MACRO(NAME) \
#if !defined(NAME) && !defined(__undef_ ## NAME)\
#define NAME \
#endif

Then we could make a definition of cl_khr_depth_images simpler

#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0) || \
    (__OPENCL_C_VERSION__ >= CL_VERSION_1_2  && defined(__SPIR__) )
DEFINE_EXTENSION_MACRO(cl_khr_depth_images)
#endif

or (2.) just do something like the following at the end of all extension/feature macro setting code.

#if defined(__undef_cl_khr_depth_images)
#undef cl_khr_depth_images
#endif

[azabaznov]

Perhaps if you give me an example it would help to understand

I was meant that this can potentially be used to undefine macros inside clang directly. In this case there will no need to add a lot of conditional preprocessor directives in the header, also the existing interface (-cl-ext=-cl_khr_depth_images) will be preserved. So for example in the header there was specified an extension definition. Can undef directive be allocated and bound to a specific source location right after extension definition if -cl-ext=-cl_khr_depth_images was specifed:

#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0) || \
    (__OPENCL_C_VERSION__ >= CL_VERSION_1_2  && defined(__SPIR__) )
#define cl_khr_depth_images 1
#endif

// Bind undef directive here

I understand that this sounds tricky, but preserving interface sound reasonable for me.

[Anastasia]

In D91531#2411725, @azabaznov wrote:

Perhaps if you give me an example it would help to understand

I was meant that this can potentially be used to undefine macros inside clang directly. In this case there will no need to add a lot of conditional preprocessor directives in the header, also the existing interface (-cl-ext=-cl_khr_depth_images) will be preserved. So for example in the header there was specified an extension definition. Can undef directive be allocated and bound to a specific source location right after extension definition if -cl-ext=-cl_khr_depth_images was specifed:

#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0) || \
    (__OPENCL_C_VERSION__ >= CL_VERSION_1_2  && defined(__SPIR__) )
#define cl_khr_depth_images 1
#endif

// Bind undef directive here

I understand that this sounds tricky, but preserving interface sound reasonable for me.

Yes, preserving the interface is not unreasonable indeed. So if I understand it well you are suggesting to perform a check for every parsed macro definition that would identify whether the macro has a name matching what is provided in -cl-ext=? Then if the name matches it would check whether the macro should be defined or not? I feel this might be a bit too costly to justify the gain. That means that not every user would agree on such a mechanism to be enabled by default i.e. it means we would need to provide an alternative interface anyway.

[azabaznov]

So if I understand it well you are suggesting to perform a check for every parsed macro definition that would identify whether the macro has a name matching what is provided in -cl-ext=? Then if the name matches it would check whether the macro should be defined or not?

No, sorry for confusing. There is no need to check every parsed macro. For example an internal pragma can be used for these purposes (#pragma OPENCL EXTENSION all : undef in example below). So imagine if we can differentiate header-only extensions and features and all of them are defined in header. After the list of definitions the special pragma is used and is a processed in a way that it inserts undef for each macro definition which relates to header-only extension/feature and was turned off with the option (-cl-ext=-cl_khr_depth_images):

#define cl_khr_depth_images 1
#define cl_khr_fp64 1
#define cl_khr_mipmap_image  1
...
#pragma OPENCL EXTENSION all : undef

However, this might be hard to maintain and I'm not sure yet that this is even legally to do in current Preprocessor design, but this is at least more scalable than adding #if defined(__undef_ for each extension in the end of the header. Nevertheless, that's what I meant about preserving the current interface.

Also, I didn't quite get how the proposing hook will allow users to declare own extensions outside the codebase. Are you expecting them to use existing -cl-ext or -Dcl_khr_depth_images? In the later case they won't able to use #pragma OPENCL EXTESNION cl_khr_depth_images : enable (while the specification does not describe for which extensions pragma is exactly needed or not, but still)

[Anastasia]

In D91531#2412532, @azabaznov wrote:

So if I understand it well you are suggesting to perform a check for every parsed macro definition that would identify whether the macro has a name matching what is provided in -cl-ext=? Then if the name matches it would check whether the macro should be defined or not?

No, sorry for confusing. There is no need to check every parsed macro. For example an internal pragma can be used for these purposes (#pragma OPENCL EXTENSION all : undef in example below). So imagine if we can differentiate header-only extensions and features and all of them are defined in header. After the list of definitions the special pragma is used and is a processed in a way that it inserts undef for each macro definition which relates to header-only extension/feature and was turned off with the option (-cl-ext=-cl_khr_depth_images):

#define cl_khr_depth_images 1
#define cl_khr_fp64 1
#define cl_khr_mipmap_image  1
...
#pragma OPENCL EXTENSION all : undef

However, this might be hard to maintain and I'm not sure yet that this is even legally to do in current Preprocessor design, but this is at least more scalable than adding #if defined(__undef_ for each extension in the end of the header. Nevertheless, that's what I meant about preserving the current interface.

Regarding the scalability, I think we can hide the fact that __undef_<ext name> are being added using some mechanisms I have suggested earlier. As for interface, I agree that this will be different. But I don't know whether it is a new problem. For C and C++ it is similar - the functionality is being extended via libraries where possible without modifying the compiler. Adding everything in the compiler is just not going to work. It is just that we ended up doing this more than we should have. Do you see a big value in hooking the extensions to -cl-ext? then we could think of some slight modification in its current behavior but I am worried that this might introduce a place for nasty to debug issues.

Regarding the idea of a new pragma I think this is slightly violating normal flow. Such pragma only provides partial functionality i.e. it is useless without -cl-ext. The flag is a frontend only so there is no way to set it for the regular users. Also this is something that we should hide from the users really. But there are no good mechanisms to do so. We could, of course, think of some ways to refine the pragma and for that, it is probably better to submit a separate RFC with your initial ideas. But I feel this might bring more problems than it would solve. Also in the past, we have reinvented the wheel several times in OpenCL and the community is just not big enough to maintain everything so if we can reuse functionality from C or C++ we should just do it.

Also, I didn't quite get how the proposing hook will allow users to declare own extensions outside the codebase. Are you expecting them to use existing -cl-ext or -Dcl_khr_depth_images? In the later case they won't able to use #pragma OPENCL EXTESNION cl_khr_depth_images : enable (while the specification does not describe for which extensions pragma is exactly needed or not, but still)

Vendor extensions that genuinely need the fronted modifications can be added to OpenCLExtensions.def, as I am not suggesting to deprecate them. However if extensions can be implemented as libraries then the macro can be added using either:

• Internal header
• Command line flag -D
• Target hooks TargetInfo::getOpenCLExtensionDefines

I would recommend using either 1 or 2. It is easier to dial with OpenCL C code than dialing with C++ code that adds OpenCL C code.

Regarding the pragma - that should be added only if it is needed and I think most of the current extensions don't need it. In the future, we should probably change the definition of extensions in OpenCLExtensions.def to allow specifying whether pragma is to be added or not. So I think we should just stop adding the pragma everywhere for no reason.

[mantognini]

When reading the documentation [1] for -cl-ext (which I've never used so far), I've noticed nothing is said about non-standard configurations (such as disabling cl_khr_depth_images with CL2.0). Quickly testing this shows that options can be specified to produce non-standard behaviour, as shown by https://godbolt.org/z/1Yz1Md.

Is it intentional that -cl-ext allows such non-standard behaviour? (This question is not necessarily address to @Anastasia.)
/If so/, then these statements

Defining __undef_cl_khr_depth_images can alter the default behavior of the predefined macro. This is equivalent to passing -cl-ext=-cl_khr_depth_images.

and

cl_khr_depth_images is a core functionality of CL2.0 and thefore defining __undef_cl_khr_depth_images doesn't modify the default behavior.

are slightly contradicting each other: the approach with __undef macros seems to ensure a more conformant behaviour.

I'm mainly asking for clarification in order to know in which direction we want to go, as one could also argue the present documentation doesn't imply non-standard behaviour is desired and that the current implementation of -cl-ext is buggy.

[1] https://clang.llvm.org/docs/UsersManual.html#cmdoption-cl-ext

[Anastasia]

In D91531#2418237, @mantognini wrote:

When reading the documentation [1] for -cl-ext (which I've never used so far), I've noticed nothing is said about non-standard configurations (such as disabling cl_khr_depth_images with CL2.0). Quickly testing this shows that options can be specified to produce non-standard behaviour, as shown by https://godbolt.org/z/1Yz1Md.

Is it intentional that -cl-ext allows such non-standard behaviour? (This question is not necessarily address to @Anastasia.)
/If so/, then these statements

Defining __undef_cl_khr_depth_images can alter the default behavior of the predefined macro. This is equivalent to passing -cl-ext=-cl_khr_depth_images.

and

cl_khr_depth_images is a core functionality of CL2.0 and thefore defining __undef_cl_khr_depth_images doesn't modify the default behavior.

are slightly contradicting each other: the approach with __undef macros seems to ensure a more conformant behaviour.

I'm mainly asking for clarification in order to know in which direction we want to go, as one could also argue the present documentation doesn't imply non-standard behaviour is desired and that the current implementation of -cl-ext is buggy.

[1] https://clang.llvm.org/docs/UsersManual.html#cmdoption-cl-ext

Thanks! This is a very good point. I am not entirely clear of the intended use case. In this patch I have only tried to preserve existing functionality. To my memory the flag was added to simplify life of downstream implementations. However if the intent of it is not clear now perhaps it shouldn't really be part of upstream code base. Otherwise all it does is complicating future development. I suggest we try to clarify the following:

1. How/where -cl-ext is used.
2. Does its use (if any) still justify maintenance and further development?
3. Do we need equivalent functionality for extensions that are not part of clang i.e. library extensions?

clang/include/clang/Basic/OpenCLExtensions.def

 OPENCLEXT_INTERNAL(cl_khr_local_int32_extended_atomics, 100, 110)
 OPENCLEXT_INTERNAL(cl_khr_int64_base_atomics, 100, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_int64_extended_atomics, 100, ~0U)
 
 // EMBEDDED_PROFILE
 OPENCLEXT_INTERNAL(cles_khr_int64, 110, ~0U)
 
 // OpenCL 1.2.
-OPENCLEXT_INTERNAL(cl_khr_depth_images, 120, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_gl_msaa_sharing, 120, ~0U)
 
 // OpenCL 2.0.
 OPENCLEXT_INTERNAL(cl_khr_mipmap_image, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_mipmap_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U)
 OPENCLEXT_INTERNAL(cl_khr_subgroup_extended_types, 200, ~0U)

clang/include/clang/Basic/TargetInfo.h

public:
 
   ///===---- Other target property query methods --------------------------===//
 
   /// Appends the target-specific \#define values for this
   /// target set to the specified buffer.
   virtual void getTargetDefines(const LangOptions &Opts,
                                 MacroBuilder &Builder) const = 0;
 
+  /// Appends the \#define for supported extensions.
+  virtual void getOpenCLExtensionDefines(const LangOptions &Opts,
+                                MacroBuilder &Builder) const;
 
   /// Return information about target-specific builtins for
   /// the current primary target, and info about which builtins are non-portable
   /// across the current set of primary and secondary targets.
   virtual ArrayRef<Builtin::Info> getTargetBuiltins() const = 0;
 
   /// The __builtin_clz* and __builtin_ctz* built-in
   /// functions are specified to have undefined results for zero inputs, but

clang/lib/Basic/Targets.cpp

TargetInfo::CreateTargetInfo(DiagnosticsEngine &Diags,
 
   if (!Target->validateTarget(Diags))
     return nullptr;
 
   Target->CheckFixedPointBits();
 
   return Target.release();
 }
+
+void TargetInfo::getOpenCLExtensionDefines(const LangOptions &Opts,
+                                           MacroBuilder &Builder) const {
+  // Add macros for extension that this target supports.
+  auto defineExtMacro = [&](llvm::StringRef Ext) {
+    if (getSupportedOpenCLOpts().isSupported(Ext, Opts))
+      Builder.defineMacro(Ext);
+  };
+
+#define OPENCLEXT(Ext) defineExtMacro(#Ext);
+#include "clang/Basic/OpenCLExtensions.def"
+
+  // Here we could define macros that are available unconditionally.
+  // Ideally only extensions that are implemented in the frontend or
+  // macros that can't be added in the implicit headers should be
+  // added here.
+  Builder.defineMacro("__opencl_c_int64");
+}

clang/lib/Frontend/InitPreprocessor.cpp

#undef DEFINE_LOCK_FREE_MACRO
   // Define a macro indicating that the source file is being compiled with a
   // SYCL device compiler which doesn't produce host binary.
   if (LangOpts.SYCLIsDevice) {
     Builder.defineMacro("__SYCL_DEVICE_ONLY__", "1");
   }
 
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
-#define OPENCLEXT(Ext)                                                         \
-  if (TI.getSupportedOpenCLOpts().isSupported(#Ext, LangOpts))                 \
-    Builder.defineMacro(#Ext);
-#include "clang/Basic/OpenCLExtensions.def"
+    // Add OpenCL #defines for supported extensions. 
+    TI.getOpenCLExtensionDefines(LangOpts, Builder);
 
     if (TI.getTriple().isSPIR())
       Builder.defineMacro("__IMAGE_SUPPORT__");
   }
 
   if (TI.hasInt128Type() && LangOpts.CPlusPlus && LangOpts.GNUMode) {
     // For each extended integer type, g++ defines a macro mapping the
     // index of the type (0 in this case) in some list of extended types

clang/lib/Headers/opencl-c.h

 //===--- opencl-c.h - OpenCL C language builtin function header -----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _OPENCL_H_
 #define _OPENCL_H_
 
 #include "opencl-c-base.h"
 
-#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0) || \
+    (__OPENCL_C_VERSION__ >= CL_VERSION_1_2 && defined(__SPIR__) && !defined(__undef_cl_khr_depth_images))
 #ifndef cl_khr_depth_images
 #define cl_khr_depth_images
 #endif //cl_khr_depth_images
 #endif //defined(__OPENCL_CPP_VERSION__) || (__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

clang/test/Headers/opencl-c-header.cl

 #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_1_2)
 // expected-no-diagnostics
 #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
+
+// Check predefined extension macros
+
+#if defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ == 200 || \
+     (__OPENCL_C_VERSION__ >= 120 && defined(__SPIR__) && !defined(__undef_cl_khr_depth_images)) 
+#ifndef cl_khr_depth_images
+#error "Missing cl_khr_depth_images define"
+#endif
+#else
+#ifdef cl_khr_depth_images
+#error "Incorrect cl_khr_depth_images define"
+#endif
+#endif

clang/test/SemaOpenCL/extension-version.cl

 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=clc++ %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=clc++ %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200) && !defined(TEST_CORE_FEATURES)
-// expected-no-diagnostics
+// Extensions in all versions
+#ifndef __opencl_c_int64
+#error "Missing __opencl_c_int64 predefined macro"
 #endif
 
-// Extensions in all versions
 #ifndef cl_clang_storage_class_specifiers
 #error "Missing cl_clang_storage_class_specifiers define"
 #endif
 #pragma OPENCL EXTENSION cl_clang_storage_class_specifiers: enable
 
 #ifndef cl_khr_fp16
 #error "Missing cl_khr_fp16 define"
 #endif

 #endif
 #pragma OPENCL EXTENSION cl_amd_media_ops: enable
 
 #ifndef cl_amd_media_ops2
 #error "Missing cl_amd_media_ops2 define"
 #endif
 #pragma OPENCL EXTENSION cl_amd_media_ops2 : enable
 
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_depth_images
-#error "Missing cl_khr_depth_images define"
-#endif
-#else
-#ifdef cl_khr_depth_images
-#error "Incorrect cl_khr_depth_images define"
-#endif
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_depth_images' - ignoring}}
-#endif
+// FIXME: This pragma is meaningless and it doesn't add any functionality.
+// However we will probably need to keep it for backward compatibility.
+// In which case there is no need to guard its use by any version or a target
+// setting as it does absolutely nothing anyway.
+// The warning diagnostic should change however to something like -
+// - pragma has no effect.
 #pragma OPENCL EXTENSION cl_khr_depth_images : enable
+// expected-warning@-1{{unknown OpenCL extension 'cl_khr_depth_images' - ignoring}}
 
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
 #ifndef cl_intel_subgroups
 #error "Missing cl_intel_subgroups define"
 #endif
 #else
 // expected-warning@+2{{unsupported OpenCL extension 'cl_intel_subgroups' - ignoring}}
 #endif