[RFC][OpenCL] Add new diagnostic for extension pragma with no effect

[Anastasia]

Summary

All OpenCL extensions implemented in clang were adding an extension pragma. However, the spec has never described what the pragma was needed for. There is now an issue open to the spec to clarify the behavior of pragma https://github.com/KhronosGroup/OpenCL-Docs/issues/82.

The majory of extension pragmas are simply ignored by clang. This is for example the case for all extensions from the list II in RFC: http://lists.llvm.org/pipermail/cfe-dev/2020-September/066911.html. The pragmas are simply ignored because the only functionality needed is extension macro definition.

In https://reviews.llvm.org/D91531 I proposed alternative mechanisms to define the extension macro. But that patch completely removed the pragma for the extension that was accepted by the parser originally. This implies that the existing kernels that used the pragma will be compiled with a warning for the new clang releases:

unknown OpenCL extension 'cl_khr_depth_images' - ignoring

Even though it is not uncommon that the new warnings occur for the existing code with new compiler releases, it has been discussed in https://reviews.llvm.org/D89372 that this might be unacceptable for the OpenCL developers. To prevent this warning the suggestion is to introduce a new warning that is TURNED OFF by default with the following wording:

OpenCL extension pragma 'cl_khr_depth_images' has no effect - ignoring

This design favors the existing kernels that might have used the pragma. This is highly unlikely because the pragma never did anything but it is not impossible. The developer of new kernels that wish to avoid redundant pragma will be able to pass -Wpedantic-redundant-pragma flag to clang which will then provide the warning with the wording below if such pragmas are found.

Note that the same behavior has been implemented for the extension pragma that become core functionality of the specification. See -Wpedantic-core-features for more details. The proposed approach is therefore built on top of the existing concept.

[Anastasia]

Added full diff.

clang/docs/DiagnosticsReference.rst

 ------------------------
 **Diagnostic text:**
 
 +----------------------------------------------------------------------------------------------------------------------------------------------------------------+
 |:warning:`warning:` |nbsp| :diagtext:`OpenCL extension` |nbsp| :placeholder:`A` |nbsp| :diagtext:`is core feature or supported optional core feature - ignoring`|
 +----------------------------------------------------------------------------------------------------------------------------------------------------------------+
 
 
+-Wpedantic-redundant-pragma
+------------------------
+**Diagnostic text:**
+
++----------------------------------------------------------------------------------------------------------------------------------------------------------------+
+|:warning:`warning:` |nbsp| :diagtext:`OpenCL extension pragma` |nbsp| :placeholder:`A` |nbsp| :diagtext:`has no effect - ignoring`|
++----------------------------------------------------------------------------------------------------------------------------------------------------------------+
+
+
 -Wpessimizing-move
 ------------------
 **Diagnostic text:**
 
 +--------------------------------------------------------------------------------------+
 |:warning:`warning:` |nbsp| :diagtext:`moving a temporary object prevents copy elision`|
 +--------------------------------------------------------------------------------------+
 

clang/include/clang/Basic/DiagnosticParseKinds.td

 // OpenCL EXTENSION pragma (OpenCL 1.1 [9.1])
 def warn_pragma_expected_colon : Warning<
   "missing ':' after %0 - ignoring">, InGroup<IgnoredPragmas>;
 def warn_pragma_expected_predicate : Warning<
   "expected %select{'enable', 'disable', 'begin' or 'end'|'disable'}0 - ignoring">, InGroup<IgnoredPragmas>;
 def warn_pragma_begin_end_mismatch : Warning<
   "OpenCL extension end directive mismatches begin directive - ignoring">, InGroup<IgnoredPragmas>;
 def warn_pragma_unknown_extension : Warning<
-  "unknown OpenCL extension %0 - ignoring">, InGroup<IgnoredPragmas>;
+  "unknown OpenCL extension pragma %0 - ignoring">, InGroup<IgnoredPragmas>;
+def warn_pragma_deprecated_extension : Warning<
+  "OpenCL extension pragma %0 has no effect - ignoring">, InGroup<DiagGroup<"pedantic-redundant-pragma">>, DefaultIgnore;
 def warn_pragma_unsupported_extension : Warning<
   "unsupported OpenCL extension %0 - ignoring">, InGroup<IgnoredPragmas>;
 def warn_pragma_extension_is_core : Warning<
   "OpenCL extension %0 is core feature or supported optional core feature - ignoring">, InGroup<DiagGroup<"pedantic-core-features">>, DefaultIgnore;
 
 // OpenCL errors.
 def err_opencl_taking_function_address_parser : Error<
   "taking address of function is not allowed">;

clang/lib/Parse/ParsePragma.cpp

if (Name == "all") {
     if (!Opt.isKnown(Name) || !Opt.isSupported(Name, getLangOpts())) {
       Opt.support(Name);
     }
     Actions.setCurrentOpenCLExtension(Name);
   } else if (State == End) {
     if (Name != Actions.getCurrentOpenCLExtension())
       PP.Diag(NameLoc, diag::warn_pragma_begin_end_mismatch);
     Actions.setCurrentOpenCLExtension("");
-  } else if (!Opt.isKnown(Name))
+  } else if (!Opt.isKnown(Name)) {
+    // The pragmas in this list do nothing but are kept mainly for backward
+    // compatibility. Pragmas from new extensions should not be added here.
+    // If the pragma does nothing it should not be added until its
+    // functionality is determined.
+    StringRef DeprecatedPragmas{"cl_khr_depth_images,"};
+    if (DeprecatedPragmas.find(Name) != StringRef::npos)
+      PP.Diag(NameLoc, diag::warn_pragma_deprecated_extension) << Ident;
+    else
       PP.Diag(NameLoc, diag::warn_pragma_unknown_extension) << Ident;
-  else if (Opt.isSupportedExtension(Name, getLangOpts()))
+  } else if (Opt.isSupportedExtension(Name, getLangOpts()))
     Opt.enable(Name, State == Enable);
   else if (Opt.isSupportedCore(Name, getLangOpts()))
     PP.Diag(NameLoc, diag::warn_pragma_extension_is_core) << Ident;
   else
     PP.Diag(NameLoc, diag::warn_pragma_unsupported_extension) << Ident;
 }
 
 void Parser::HandlePragmaMSPointersToMembers() {

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

 global atomic_int z = ATOMIC_VAR_INIT(99);
 #endif //__OPENCL_C_VERSION__
 
 // Verify that non-builtin cl_intel_planar_yuv extension is defined from
 // OpenCL 1.2 onwards.
 #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}}
+// expected-warning@+2{{unknown OpenCL extension pragma '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 || \

clang/test/Parser/opencl-pragma.cl

 // RUN: %clang_cc1 %s -verify -pedantic -Wno-empty-translation-unit -fsyntax-only -triple spir-unknown-unknown
 
 #pragma OPENCL EXTENSION cl_khr_fp16 : enable
 
-#pragma OPENCL EXTENSION cl_no_such_extension : disable /* expected-warning {{unknown OpenCL extension 'cl_no_such_extension' - ignoring}} */
+#pragma OPENCL EXTENSION cl_no_such_extension : disable /* expected-warning {{unknown OpenCL extension pragma 'cl_no_such_extension' - ignoring}} */
 
 #pragma OPENCL EXTENSION all : disable
 #pragma OPENCL EXTENSION all : enable /* expected-warning {{expected 'disable' - ignoring}} */
 
 #pragma OPENCL EXTENSION cl_khr_fp64 : on /* expected-warning {{expected 'enable', 'disable', 'begin' or 'end' - ignoring}} */
 
 #pragma OPENCL FP_CONTRACT ON
 #pragma OPENCL FP_CONTRACT OFF
 #pragma OPENCL FP_CONTRACT DEFAULT
 #pragma OPENCL FP_CONTRACT FOO // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}

clang/test/SemaOpenCL/extension-begin.cl

 //
 // RUN: rm -rf %t.modules
 // RUN: mkdir -p %t.modules
 //
 // RUN: %clang_cc1 -cl-std=CL2.0 -DIMPLICIT_INCLUDE -include %S/extension-begin.h -triple spir-unknown-unknown -O0 -emit-llvm -o - -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.modules %s -verify -pedantic
 
 #pragma OPENCL EXTENSION my_ext : enable
 #ifndef IMPLICIT_INCLUDE
-// expected-warning@-2 {{unknown OpenCL extension 'my_ext' - ignoring}}
-// expected-warning@+2 {{unknown OpenCL extension 'my_ext' - ignoring}}
+// expected-warning@-2 {{unknown OpenCL extension pragma 'my_ext' - ignoring}}
+// expected-warning@+2 {{unknown OpenCL extension pragma 'my_ext' - ignoring}}
 #endif // IMPLICIT_INCLUDE
 #pragma OPENCL EXTENSION my_ext : disable
 
 #ifndef IMPLICIT_INCLUDE
 #include "extension-begin.h"
 #endif // IMPLICIT_INCLUDE
 #ifndef USE_PCH
 // expected-warning@extension-begin.h:4 {{expected 'disable' - ignoring}}

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
+// RUN: %clang_cc1 -x cl -cl-std=clc++ %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -Wpedantic-redundant-pragma -DTEST_CORE_FEATURES -DTEST_REDUNDANT_PRAGMA
+
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ == 200) && !(defined(TEST_CORE_FEATURES) || defined(TEST_REDUNDANT_PRAGMA))
+// expected-no-diagnostics
+#endif
 
 // Extensions in all versions
 #ifndef __opencl_c_int64
 #error "Missing __opencl_c_int64 predefined macro"
 #endif
 
 #ifndef cl_clang_storage_class_specifiers
 #error "Missing cl_clang_storage_class_specifiers define"

 #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
 
-// FIXME: This pragma is meaningless and it doesn't add any functionality.
+// 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(TEST_REDUNDANT_PRAGMA)
+// expected-warning@-2{{OpenCL extension pragma 'cl_khr_depth_images' has no effect - ignoring}}
+#endif
 
 #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