[OpenCL] Override supported OpenCL extensions with -cl-ext option

[asavonic]

Summary

This patch adds a command line option '-cl-ext' to control a set of
supported OpenCL extensions. Option accepts a comma-separated list
of extensions prefixed with '+' or '-'.

It can be used together with a target triple to override support for some
extensions:

// spir target supports all extensions, but we want to disable fp64
clang -cc1 -triple spir-unknown-unknown -cl-ext=-cl_khr_fp64

Special 'all' extension allows to enable or disable all possible
extensions:

// only fp64 will be supported
clang -cc1 -triple spir-unknown-unknown -cl-ext=-all,+cl_khr_fp64

[asavonic]

• Remove unused test case

[Anastasia]

What would be the use case to override the supported extensions for the end user?

The change to set the right extensions based on the target compiled for was to avoid mis-compilations. But adding a user flag to control that could lead to the old problem to reoccur.

[asavonic]

In D23712#520818, @Anastasia wrote:

What would be the use case to override the supported extensions for the end user?

Some extensions may be supported by the platform in general, but not by the
specific version of the OpenCL runtime.

For example, when the platform supports fp64, the OpenCL runtime may not have a
fp64 built-ins implementation. In this case it is better to disable fp64 by the
compiler, rather than get a link error.

Actually, the use case is similar to the -target-feature option, which allows to
control supported CPU features, e.g enable or disable some instruction set.

[Anastasia]

In D23712#524021, @asavonic wrote:

In D23712#520818, @Anastasia wrote:

What would be the use case to override the supported extensions for the end user?

Some extensions may be supported by the platform in general, but not by the
specific version of the OpenCL runtime.

Would triple not be suitable in this case, because they include OS/RT information?

[asavonic]

Hi @Anastasia,

Sorry for my late reply.

What would be the use case to override the supported extensions for the end user?

Some extensions may be supported by the platform in general, but not by the
specific version of the OpenCL runtime.

Would triple not be suitable in this case, because they include OS/RT information?

This can be achieved with a custom triple, yes, but it is not a flexible solution.
We need to create a triple for every combination of extensions. If we want to change it, we need to modify the clang source code and recompile it.

I think it is better to have a default set of extensions (from target triple) and allow user to tweak it without creating one more triple.

So instead of:

clang -cc1 -triple spir-unknown-intel-skl-nofp64-nofp16 <...>

we could write:

clang -cc1 -triple spir-unknown-unknown -cl-ext=-cl_khr_fp64,-cl_khr_fp16 <...>

Both have the same result, but the latter one is more flexible and it can be changed without changing the clang code.

[yaxunl]

I think we need two more tests for concatenating and overriding the option, e.g

-cl-ext=-cl_khr_fp64 -cl-ext=+cl_khr_fp64

and

-cl-ext=-cl_khr_fp64,+cl_khr_fp64

include/clang/Basic/OpenCLOptions.h

35

nm = Enable;

39

It seems Enable should be a local variable.

59

nm = Enable;

include/clang/Driver/Options.td

395

maybe something like

... Enable or disable specific OpenCL extensions separated by comma. Use 'all' for all extensions.

[asavonic]

Add more test cases and fix minor issues

[asavonic]

include/clang/Basic/OpenCLOptions.h

39

This argument is used when Ext is not prefixed by '+' or '-'.

[yaxunl]

I think we need two more tests for concatenating and overriding the option, e.g

-cl-ext=-cl_khr_fp64 -cl-ext=+cl_khr_fp64

and

-cl-ext=-cl_khr_fp64,+cl_khr_fp64

include/clang/Basic/OpenCLOptions.h

39

Better add a comments for this function about its semantics, i.e., if Ext does not starts with +/-, it is enabled/disabled by \p Enable, otherwise +/- overrides \p Enable, since this is not obvious.

[Anastasia]

include/clang/Basic/OpenCLOptions.h

39

Indeed, generally it would be nice to add a comment explaining the purpose of this functions. I don't think the name is descriptive enough.

include/clang/Driver/Options.td

394

I would see it as cc1 option instead to avoid confusions on its intension.

395

Could we also add a short statement, that +/- are used to turn the extesions on and off.

lib/Basic/Targets.cpp

1878

Is this really target specific? I feel this should rather go into common code.

[asavonic]

• Describe OpenCLOptions::set() function
• Move -cl-ext option to cc1
• Reword -cl-ext option help
• Move -cl-ext handling out of target-specific code
• Add two more test cases regarding -cl-ext option

[yaxunl]

LGTM. Thanks!

[joey]

Two minor comments, but otherwise LGTM!

include/clang/Basic/OpenCLOptions.h

33–34

This comment needs to be changed, to reflect that they are now all enabled or disabled.

include/clang/Basic/TargetInfo.h

986

Can you put a space around the ':'.

[asavonic]

• Fix comments and code formatting

include/clang/Basic/OpenCLOptions.h

 /// \file
 /// \brief Defines the clang::OpenCLOptions class.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_OPENCLOPTIONS_H
 #define LLVM_CLANG_BASIC_OPENCLOPTIONS_H
 
+#include "llvm/ADT/StringRef.h"
+
 namespace clang {
 
 /// \brief OpenCL supported extensions and optional core features
 class OpenCLOptions {
 public:
 #define OPENCLEXT(nm) unsigned nm : 1;
 #include "clang/Basic/OpenCLExtensions.def"
 
   OpenCLOptions() {
 #define OPENCLEXT(nm)   nm = 0;
 #include "clang/Basic/OpenCLExtensions.def"
   }
 
   // Enable all options.
-  void setAll() {
-#define OPENCLEXT(nm)   nm = 1;
+  void setAll(bool Enable = true) {
+#define OPENCLEXT(nm)   nm = Enable ? 1 : 0;
+#include "clang/Basic/OpenCLExtensions.def"
+  }
+
+  void set(llvm::StringRef Ext, bool Enable = true) {
+    assert(!Ext.empty() && "Extension is empty.");
+
+    switch (Ext[0]) {
+    case '+':
+      Enable = true;
+      Ext = Ext.drop_front();
+      break;
+    case '-':
+      Enable = false;
+      Ext = Ext.drop_front();
+      break;
+    }
+
+    if (Ext.equals("all")) {
+      setAll(Enable);
+    }
+
+#define OPENCLEXT(nm)       \
+    if (Ext.equals(#nm)) {  \
+      nm = Enable ? 1 : 0;  \
+    }
 #include "clang/Basic/OpenCLExtensions.def"
   }
 
   // Is supported with OpenCL version \p OCLVer.
 #define OPENCLEXT_INTERNAL(Ext, Avail, ...) \
   bool is_##Ext##_supported(unsigned OCLVer) const { \
     return Ext && OCLVer >= Avail; \
   }

include/clang/Basic/TargetInfo.h

public:
   }
 
   /// \brief Whether target allows to overalign ABI-specified prefered alignment
   virtual bool allowsLargerPreferedTypeAlignment() const { return true; }
 
   /// \brief Set supported OpenCL extensions and optional core features.
   virtual void setSupportedOpenCLOpts() {}
 
+  /// \brief Set supported OpenCL extensions as written on command line
+  virtual void setOpenCLExtensionOpts() {
+    for (const auto &Ext:getTargetOpts().OpenCLExtensionsAsWritten) {
+      getTargetOpts().SupportedOpenCLOptions.set(Ext);
+    }
+  }
+
   /// \brief Get supported OpenCL extensions and optional core features.
   OpenCLOptions &getSupportedOpenCLOpts() {
     return getTargetOpts().SupportedOpenCLOptions;
   }
 
   /// \brief Get const supported OpenCL extensions and optional core features.
   const OpenCLOptions &getSupportedOpenCLOpts() const {
       return getTargetOpts().SupportedOpenCLOptions;

include/clang/Basic/TargetOptions.h

public:
   /// The list of target specific features to enable or disable -- this should
   /// be a list of strings starting with by '+' or '-'.
   std::vector<std::string> Features;
 
   std::vector<std::string> Reciprocals;
 
   /// Supported OpenCL extensions and optional core features.
   OpenCLOptions SupportedOpenCLOptions;
+
+  /// \brief The list of OpenCL extensions to enable or disable, as written on
+  /// the command line.
+  std::vector<std::string> OpenCLExtensionsAsWritten;
 };
 
 }  // end namespace clang
 
 #endif

include/clang/Driver/Options.td

 def cl_no_signed_zeros : Flag<["-"], "cl-no-signed-zeros">, Group<opencl_Group>, Flags<[CC1Option]>,
   HelpText<"OpenCL only. Allow use of less precise no signed zeros computations in the generated binary.">;
 def cl_std_EQ : Joined<["-"], "cl-std=">, Group<opencl_Group>, Flags<[CC1Option]>,
   HelpText<"OpenCL language standard to compile for.">;
 def cl_denorms_are_zero : Flag<["-"], "cl-denorms-are-zero">, Group<opencl_Group>, Flags<[CC1Option]>,
   HelpText<"OpenCL only. Allow denormals to be flushed to zero.">;
 def cl_fp32_correctly_rounded_divide_sqrt : Flag<["-"], "cl-fp32-correctly-rounded-divide-sqrt">, Group<opencl_Group>, Flags<[CC1Option]>,
   HelpText<"OpenCL only. Specify that single precision floating-point divide and sqrt used in the program source are correctly rounded.">;
+def cl_ext_EQ : CommaJoined<["-"], "cl-ext=">, Group<opencl_Group>, Flags<[CC1Option]>,
+  HelpText<"OpenCL only. Enable or disable specific OpenCL extensions.">;
 def client__name : JoinedOrSeparate<["-"], "client_name">;
 def combine : Flag<["-", "--"], "combine">, Flags<[DriverOption, Unsupported]>;
 def compatibility__version : JoinedOrSeparate<["-"], "compatibility_version">;
 def coverage : Flag<["-", "--"], "coverage">;
 def cpp_precomp : Flag<["-"], "cpp-precomp">, Group<clang_ignored_f_Group>;
 def current__version : JoinedOrSeparate<["-"], "current_version">;
 def cxx_isystem : JoinedOrSeparate<["-"], "cxx-isystem">, Group<clang_i_Group>,
   HelpText<"Add directory to the C++ SYSTEM include search path">, Flags<[CC1Option]>,

lib/Basic/Targets.cpp

void setSupportedOpenCLOpts() override {
     Opts.cl_khr_icd = 1;
 
     Opts.cl_khr_fp64 = 1;
     Opts.cl_khr_byte_addressable_store = 1;
     Opts.cl_khr_global_int32_base_atomics = 1;
     Opts.cl_khr_global_int32_extended_atomics = 1;
     Opts.cl_khr_local_int32_base_atomics = 1;
     Opts.cl_khr_local_int32_extended_atomics = 1;
+
+    setOpenCLExtensionOpts();
   }
 };
 
 const Builtin::Info NVPTXTargetInfo::BuiltinInfo[] = {
 #define BUILTIN(ID, TYPE, ATTRS)                                               \
   { #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr },
 #define LIBBUILTIN(ID, TYPE, ATTRS, HEADER)                                    \
   { #ID, TYPE, ATTRS, HEADER, ALL_LANGUAGES, nullptr },
if (GPU >= GK_SOUTHERN_ISLANDS) {
       Opts.cl_khr_int64_base_atomics = 1;
       Opts.cl_khr_int64_extended_atomics = 1;
       Opts.cl_khr_mipmap_image = 1;
       Opts.cl_khr_subgroups = 1;
       Opts.cl_khr_3d_image_writes = 1;
       Opts.cl_amd_media_ops = 1;
       Opts.cl_amd_media_ops2 = 1;
     }
+    setOpenCLExtensionOpts();
   }
 
   LangAS::ID getOpenCLImageAddrSpace() const override {
     return LangAS::opencl_constant;
   }
 
   CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
     switch (CC) {
public:
   }
 
   bool hasSjLjLowering() const override {
     return true;
   }
 
   void setSupportedOpenCLOpts() override {
     getSupportedOpenCLOpts().setAll();
+    setOpenCLExtensionOpts();
   }
 };
 
 bool X86TargetInfo::setFPMath(StringRef Name) {
   if (Name == "387") {
     FPMath = FP_387;
     return true;
   }
public:
   CallingConv getDefaultCallingConv(CallingConvMethodType MT) const override {
     return CC_SpirFunction;
   }
 
   void setSupportedOpenCLOpts() override {
     // Assume all OpenCL extensions and optional core features are supported
     // for SPIR since it is a generic target.
     getSupportedOpenCLOpts().setAll();
+    setOpenCLExtensionOpts();
   }
 };
 
 class SPIR32TargetInfo : public SPIRTargetInfo {
 public:
   SPIR32TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : SPIRTargetInfo(Triple, Opts) {
     PointerWidth = PointerAlign = 32;

lib/Frontend/CompilerInvocation.cpp

static void ParseTargetArgs(TargetOptions &Opts, ArgList &Args,
   Opts.FPMath = Args.getLastArgValue(OPT_mfpmath);
   Opts.FeaturesAsWritten = Args.getAllArgValues(OPT_target_feature);
   Opts.LinkerVersion = Args.getLastArgValue(OPT_target_linker_version);
   Opts.Triple = llvm::Triple::normalize(Args.getLastArgValue(OPT_triple));
   Opts.Reciprocals = Args.getAllArgValues(OPT_mrecip_EQ);
   // Use the default target triple if unspecified.
   if (Opts.Triple.empty())
     Opts.Triple = llvm::sys::getDefaultTargetTriple();
+  Opts.OpenCLExtensionsAsWritten = Args.getAllArgValues(OPT_cl_ext_EQ);
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
                                         const char *const *ArgBegin,
                                         const char *const *ArgEnd,
                                         DiagnosticsEngine &Diags) {
   bool Success = true;
 

test/SemaOpenCL/extensions.cl

 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-std=CL1.1
 
 // Test with a target not supporting fp64.
-// RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -DNOFP64
+// RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -DNOFP64 -DNOFP16
+
+// Test with some extensions enabled or disabled by cmd-line args
+//
+// Target does not support fp64 and fp16 - override it
+// RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+cl_khr_fp64,+cl_khr_fp16
+//
+// Disable or enable all extensions
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -DNOFP64 -DNOFP16
+// RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+all
+// RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=+all,-cl_khr_fp64 -DNOFP64
+// RUN: %clang_cc1 %s -triple r600-unknown-unknown -target-cpu r600 -verify -pedantic -fsyntax-only -cl-ext=-all,+cl_khr_fp64 -DNOFP16
+
+
 
 void f1(double da) { // expected-error {{type 'double' requires cl_khr_fp64 extension}}
   double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
   (void) 1.0; // expected-warning {{double precision constant requires cl_khr_fp64}}
 }
 
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable
 #ifdef NOFP64
 // expected-warning@-2{{unsupported OpenCL extension 'cl_khr_fp64' - ignoring}}
 #endif
 
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+#ifdef NOFP16
+// expected-warning@-2{{unsupported OpenCL extension 'cl_khr_fp16' - ignoring}}
+#endif
+
 void f2(void) {
   double d;
 #ifdef NOFP64
 // expected-error@-2{{use of type 'double' requires cl_khr_fp64 extension to be enabled}}
 #endif
 
   (void) 1.0;
 #ifdef NOFP64
 // expected-warning@-2{{double precision constant requires cl_khr_fp64, casting to single precision}}
 #endif
 }
 
 #pragma OPENCL EXTENSION cl_khr_fp64 : disable
 #ifdef NOFP64
 // expected-warning@-2{{unsupported OpenCL extension 'cl_khr_fp64' - ignoring}}
 #endif
 
 void f3(void) {
   double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
 }
+
+#ifdef CL_EXT_OPT
+#pragma OPENCL EXTENSION cl_khr_fp64 : enable // expected-warning{{unsupported OpenCL extension 'cl_khr_fp64' - ignoring}}
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : disable
+void f4(__global half* h) {
+  *h = 4.h; // expected-error{{half precision constant requires cl_khr_fp16}}
+}
+#endif