[PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
This revision was automatically updated to reflect the committed changes. Closed by commit rL285700: [OpenCL] Override supported OpenCL extensions with -cl-ext option (authored by bader). Changed prior to commit: https://reviews.llvm.org/D23712?vs=75568=76571#toc Repository: rL LLVM https://reviews.llvm.org/D23712 Files: cfe/trunk/include/clang/Basic/OpenCLOptions.h cfe/trunk/include/clang/Basic/TargetInfo.h cfe/trunk/include/clang/Basic/TargetOptions.h cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/test/SemaOpenCL/extensions.cl Index: cfe/trunk/include/clang/Driver/CC1Options.td === --- cfe/trunk/include/clang/Driver/CC1Options.td +++ cfe/trunk/include/clang/Driver/CC1Options.td @@ -688,6 +688,13 @@ HelpText<"include a detailed record of preprocessing actions">; //===--===// +// OpenCL Options +//===--===// + +def cl_ext_EQ : CommaJoined<["-"], "cl-ext=">, + HelpText<"OpenCL only. Enable or disable OpenCL extensions. The argument is a comma-separated sequence of one or more extension names, each prefixed by '+' or '-'.">; + +//===--===// // CUDA Options //===--===// Index: cfe/trunk/include/clang/Basic/TargetInfo.h === --- cfe/trunk/include/clang/Basic/TargetInfo.h +++ cfe/trunk/include/clang/Basic/TargetInfo.h @@ -995,6 +995,13 @@ /// \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 : getTargetOpts().OpenCLExtensionsAsWritten) { + getTargetOpts().SupportedOpenCLOptions.set(Ext); +} + } + /// \brief Get supported OpenCL extensions and optional core features. OpenCLOptions () { return getTargetOpts().SupportedOpenCLOptions; Index: cfe/trunk/include/clang/Basic/TargetOptions.h === --- cfe/trunk/include/clang/Basic/TargetOptions.h +++ cfe/trunk/include/clang/Basic/TargetOptions.h @@ -58,6 +58,10 @@ /// 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 OpenCLExtensionsAsWritten; }; } // end namespace clang Index: cfe/trunk/include/clang/Basic/OpenCLOptions.h === --- cfe/trunk/include/clang/Basic/OpenCLOptions.h +++ cfe/trunk/include/clang/Basic/OpenCLOptions.h @@ -15,6 +15,8 @@ #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 @@ -28,9 +30,39 @@ #include "clang/Basic/OpenCLExtensions.def" } - // Enable all options. - void setAll() { -#define OPENCLEXT(nm) nm = 1; + // Enable or disable all options. + void setAll(bool Enable = true) { +#define OPENCLEXT(nm) nm = Enable; +#include "clang/Basic/OpenCLExtensions.def" + } + + /// \brief Enable or disable support for OpenCL extensions + /// \param Ext name of the extension optionally prefixed with + ///'+' or '-' + /// \param Enable used when \p Ext is not prefixed by '+' or '-' + 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); + return; +} + +#define OPENCLEXT(nm) \ +if (Ext.equals(#nm)) { \ + nm = Enable; \ +} #include "clang/Basic/OpenCLExtensions.def" } Index: cfe/trunk/test/SemaOpenCL/extensions.cl === --- cfe/trunk/test/SemaOpenCL/extensions.cl +++ cfe/trunk/test/SemaOpenCL/extensions.cl @@ -2,7 +2,26 @@ // 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
[PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
asavonic updated this revision to Diff 75568. asavonic added a comment. - Fix comments and code formatting https://reviews.llvm.org/D23712 Files: include/clang/Basic/OpenCLOptions.h include/clang/Basic/TargetInfo.h include/clang/Basic/TargetOptions.h include/clang/Driver/CC1Options.td lib/Basic/Targets.cpp lib/Frontend/CompilerInvocation.cpp test/SemaOpenCL/extensions.cl Index: test/SemaOpenCL/extensions.cl === --- test/SemaOpenCL/extensions.cl +++ test/SemaOpenCL/extensions.cl @@ -2,7 +2,26 @@ // 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 +// +// Concatenating +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-cl_khr_fp64 -cl-ext=+cl_khr_fp64 -DNOFP16 +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-cl_khr_fp64,+cl_khr_fp64 -DNOFP16 +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64 -cl-ext=+cl_khr_fp16 -cl-ext=-cl_khr_fp64 -DNOFP64 +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64,-cl_khr_fp64,+cl_khr_fp16 -DNOFP64 + + void f1(double da) { // expected-error {{type 'double' requires cl_khr_fp64 extension}} double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}} @@ -14,6 +33,11 @@ // 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 Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -2349,6 +2349,7 @@ // 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 , Index: lib/Basic/Targets.cpp === --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -8676,6 +8676,7 @@ return nullptr; Target->setSupportedOpenCLOpts(); + Target->setOpenCLExtensionOpts(); if (!Target->validateTarget(Diags)) return nullptr; Index: include/clang/Driver/CC1Options.td === --- include/clang/Driver/CC1Options.td +++ include/clang/Driver/CC1Options.td @@ -685,6 +685,13 @@ HelpText<"include a detailed record of preprocessing actions">; //===--===// +// OpenCL Options +//===--===// + +def cl_ext_EQ : CommaJoined<["-"], "cl-ext=">, + HelpText<"OpenCL only. Enable or disable OpenCL extensions. The argument is a comma-separated sequence of one or more extension names, each prefixed by '+' or '-'.">; + +//===--===// // CUDA Options //===--===// Index: include/clang/Basic/TargetOptions.h === --- include/clang/Basic/TargetOptions.h +++ include/clang/Basic/TargetOptions.h @@ -58,6 +58,10 @@ /// 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
[PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
joey added a comment. Two minor comments, but otherwise LGTM! Comment at: include/clang/Basic/OpenCLOptions.h:33 // Enable all options. + void setAll(bool Enable = true) { This comment needs to be changed, to reflect that they are now all enabled or disabled. Comment at: include/clang/Basic/TargetInfo.h:992 + virtual void setOpenCLExtensionOpts() { +for (const auto :getTargetOpts().OpenCLExtensionsAsWritten) { + getTargetOpts().SupportedOpenCLOptions.set(Ext); Can you put a space around the ':'. https://reviews.llvm.org/D23712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
yaxunl accepted this revision. yaxunl added a comment. This revision is now accepted and ready to land. LGTM. Thanks! https://reviews.llvm.org/D23712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
asavonic updated this revision to Diff 73443. asavonic added a comment. - 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 https://reviews.llvm.org/D23712 Files: include/clang/Basic/OpenCLOptions.h include/clang/Basic/TargetInfo.h include/clang/Basic/TargetOptions.h include/clang/Driver/CC1Options.td lib/Basic/Targets.cpp lib/Frontend/CompilerInvocation.cpp test/SemaOpenCL/extensions.cl Index: test/SemaOpenCL/extensions.cl === --- test/SemaOpenCL/extensions.cl +++ test/SemaOpenCL/extensions.cl @@ -2,7 +2,26 @@ // 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 +// +// Concatenating +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-cl_khr_fp64 -cl-ext=+cl_khr_fp64 -DNOFP16 +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-cl_khr_fp64,+cl_khr_fp64 -DNOFP16 +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64 -cl-ext=+cl_khr_fp16 -cl-ext=-cl_khr_fp64 -DNOFP64 +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64,-cl_khr_fp64,+cl_khr_fp16 -DNOFP64 + + void f1(double da) { // expected-error {{type 'double' requires cl_khr_fp64 extension}} double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}} @@ -14,6 +33,11 @@ // 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 Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -2349,6 +2349,7 @@ // 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 , Index: lib/Basic/Targets.cpp === --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -8676,6 +8676,7 @@ return nullptr; Target->setSupportedOpenCLOpts(); + Target->setOpenCLExtensionOpts(); if (!Target->validateTarget(Diags)) return nullptr; Index: include/clang/Driver/CC1Options.td === --- include/clang/Driver/CC1Options.td +++ include/clang/Driver/CC1Options.td @@ -685,6 +685,13 @@ HelpText<"include a detailed record of preprocessing actions">; //===--===// +// OpenCL Options +//===--===// + +def cl_ext_EQ : CommaJoined<["-"], "cl-ext=">, + HelpText<"OpenCL only. Enable or disable OpenCL extensions. The argument is a comma-separated sequence of one or more extension names, each prefixed by '+' or '-'.">; + +//===--===// // CUDA Options //===--===// Index: include/clang/Basic/TargetOptions.h === --- include/clang/Basic/TargetOptions.h +++ include/clang/Basic/TargetOptions.h @@ -58,6 +58,10 @@ /// Supported OpenCL extensions and optional core features. OpenCLOptions
Re: [PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
Anastasia added inline comments. Comment at: include/clang/Basic/OpenCLOptions.h:39 @@ +38,3 @@ + + void set(llvm::StringRef Ext, bool Enable = true) { +assert(!Ext.empty() && "Extension is empty."); yaxunl wrote: > 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. 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. Comment at: include/clang/Driver/Options.td:394 @@ -393,1 +393,3 @@ 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, Flags<[CC1Option]>, + HelpText<"OpenCL only. Enable or disable specific OpenCL extensions separated by comma. Use 'all' for all extensions.">; I would see it as cc1 option instead to avoid confusions on its intension. Comment at: include/clang/Driver/Options.td:395 @@ -394,1 +394,3 @@ +def cl_ext_EQ : CommaJoined<["-"], "cl-ext=">, Group, Flags<[CC1Option]>, + HelpText<"OpenCL only. Enable or disable specific OpenCL extensions separated by comma. Use 'all' for all extensions.">; def client__name : JoinedOrSeparate<["-"], "client_name">; Could we also add a short statement, that +/- are used to turn the extesions on and off. Comment at: lib/Basic/Targets.cpp:1882 @@ -1881,1 +1881,3 @@ + +setOpenCLExtensionOpts(); } Is this really target specific? I feel this should rather go into common code. https://reviews.llvm.org/D23712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
yaxunl added a comment. 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 Comment at: include/clang/Basic/OpenCLOptions.h:39 @@ +38,3 @@ + + void set(llvm::StringRef Ext, bool Enable = true) { +assert(!Ext.empty() && "Extension is empty."); 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. https://reviews.llvm.org/D23712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
asavonic marked 3 inline comments as done. Comment at: include/clang/Basic/OpenCLOptions.h:39 @@ +38,3 @@ + + void set(llvm::StringRef Ext, bool Enable = true) { +assert(!Ext.empty() && "Extension is empty."); yaxunl wrote: > It seems Enable should be a local variable. This argument is used when `Ext` is not prefixed by '+' or '-'. https://reviews.llvm.org/D23712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
asavonic updated this revision to Diff 72143. asavonic added a comment. Herald added a subscriber: yaxunl. Add more test cases and fix minor issues https://reviews.llvm.org/D23712 Files: include/clang/Basic/OpenCLOptions.h include/clang/Basic/TargetInfo.h include/clang/Basic/TargetOptions.h include/clang/Driver/Options.td lib/Basic/Targets.cpp lib/Frontend/CompilerInvocation.cpp test/SemaOpenCL/extensions.cl Index: test/SemaOpenCL/extensions.cl === --- test/SemaOpenCL/extensions.cl +++ test/SemaOpenCL/extensions.cl @@ -2,7 +2,24 @@ // 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 +// +// Concatenating +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64 -cl-ext=+cl_khr_fp16 -cl-ext=-cl_khr_fp64 -DNOFP64 +// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic -fsyntax-only -cl-ext=-all -cl-ext=+cl_khr_fp64,-cl_khr_fp64,+cl_khr_fp16 -DNOFP64 + + void f1(double da) { // expected-error {{type 'double' requires cl_khr_fp64 extension}} double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}} @@ -14,6 +31,11 @@ // 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 Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -2349,6 +2349,7 @@ // 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 , Index: lib/Basic/Targets.cpp === --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -1878,6 +1878,8 @@ 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(); } }; @@ -2163,6 +2165,7 @@ Opts.cl_amd_media_ops = 1; Opts.cl_amd_media_ops2 = 1; } +setOpenCLExtensionOpts(); } LangAS::ID getOpenCLImageAddrSpace() const override { @@ -2855,6 +2858,7 @@ void setSupportedOpenCLOpts() override { getSupportedOpenCLOpts().setAll(); +setOpenCLExtensionOpts(); } }; @@ -8029,6 +8033,7 @@ // Assume all OpenCL extensions and optional core features are supported // for SPIR since it is a generic target. getSupportedOpenCLOpts().setAll(); +setOpenCLExtensionOpts(); } }; Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -391,6 +391,8 @@ 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, 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, Flags<[CC1Option]>, + HelpText<"OpenCL only. Enable or disable specific OpenCL extensions separated by comma. Use 'all' for all extensions.">; def client__name : JoinedOrSeparate<["-"], "client_name">; def combine : Flag<["-", "--"], "combine">, Flags<[DriverOption, Unsupported]>; def compatibility__version : JoinedOrSeparate<["-"],
Re: [PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
yaxunl added a comment. 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 Comment at: include/clang/Basic/OpenCLOptions.h:35 @@ +34,3 @@ + void setAll(bool Enable = true) { +#define OPENCLEXT(nm) nm = Enable ? 1 : 0; +#include "clang/Basic/OpenCLExtensions.def" nm = Enable; Comment at: include/clang/Basic/OpenCLOptions.h:39 @@ +38,3 @@ + + void set(llvm::StringRef Ext, bool Enable = true) { +assert(!Ext.empty() && "Extension is empty."); It seems Enable should be a local variable. Comment at: include/clang/Basic/OpenCLOptions.h:59 @@ +58,3 @@ +if (Ext.equals(#nm)) { \ + nm = Enable ? 1 : 0; \ +} nm = Enable; Comment at: include/clang/Driver/Options.td:395 @@ -394,1 +394,3 @@ +def cl_ext_EQ : CommaJoined<["-"], "cl-ext=">, Group, Flags<[CC1Option]>, + HelpText<"OpenCL only. Enable or disable specific OpenCL extensions.">; def client__name : JoinedOrSeparate<["-"], "client_name">; maybe something like ... Enable or disable specific OpenCL extensions separated by comma. Use 'all' for all extensions. https://reviews.llvm.org/D23712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
asavonic added a comment. 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. https://reviews.llvm.org/D23712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
Anastasia added a comment. In https://reviews.llvm.org/D23712#524021, @asavonic wrote: > In https://reviews.llvm.org/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? https://reviews.llvm.org/D23712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
asavonic added a comment. In https://reviews.llvm.org/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. https://reviews.llvm.org/D23712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
Anastasia added a comment. 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. https://reviews.llvm.org/D23712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
asavonic updated this revision to Diff 68677. asavonic added a comment. - Remove unused test case https://reviews.llvm.org/D23712 Files: include/clang/Basic/OpenCLOptions.h include/clang/Basic/TargetInfo.h include/clang/Basic/TargetOptions.h include/clang/Driver/Options.td lib/Basic/Targets.cpp lib/Frontend/CompilerInvocation.cpp test/SemaOpenCL/extensions.cl Index: test/SemaOpenCL/extensions.cl === --- test/SemaOpenCL/extensions.cl +++ test/SemaOpenCL/extensions.cl @@ -2,7 +2,20 @@ // 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}} @@ -14,6 +27,11 @@ // 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 Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -2316,6 +2316,7 @@ // 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 , Index: lib/Basic/Targets.cpp === --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -1874,6 +1874,8 @@ 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(); } }; @@ -2157,6 +2159,7 @@ Opts.cl_amd_media_ops = 1; Opts.cl_amd_media_ops2 = 1; } +setOpenCLExtensionOpts(); } LangAS::ID getOpenCLImageAddrSpace() const override { @@ -2848,6 +2851,7 @@ void setSupportedOpenCLOpts() override { getSupportedOpenCLOpts().setAll(); +setOpenCLExtensionOpts(); } }; @@ -8007,6 +8011,7 @@ // Assume all OpenCL extensions and optional core features are supported // for SPIR since it is a generic target. getSupportedOpenCLOpts().setAll(); +setOpenCLExtensionOpts(); } }; Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -391,6 +391,8 @@ 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, 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, 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">; Index: include/clang/Basic/TargetOptions.h === --- include/clang/Basic/TargetOptions.h +++ include/clang/Basic/TargetOptions.h @@ -58,6 +58,10 @@ /// 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. +
[PATCH] D23712: [OpenCL] Override supported OpenCL extensions with -cl-ext option
asavonic created this revision. asavonic added a reviewer: Anastasia. asavonic added a subscriber: cfe-commits. 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 https://reviews.llvm.org/D23712 Files: include/clang/Basic/OpenCLOptions.h include/clang/Basic/TargetInfo.h include/clang/Basic/TargetOptions.h include/clang/Driver/Options.td lib/Basic/Targets.cpp lib/Frontend/CompilerInvocation.cpp test/SemaOpenCL/extensions.cl Index: test/SemaOpenCL/extensions.cl === --- test/SemaOpenCL/extensions.cl +++ test/SemaOpenCL/extensions.cl @@ -2,7 +2,20 @@ // 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}} @@ -14,6 +27,11 @@ // 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 @@ -34,3 +52,13 @@ 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 Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -2316,6 +2316,7 @@ // 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 , Index: lib/Basic/Targets.cpp === --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -1874,6 +1874,8 @@ 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(); } }; @@ -2157,6 +2159,7 @@ Opts.cl_amd_media_ops = 1; Opts.cl_amd_media_ops2 = 1; } +setOpenCLExtensionOpts(); } LangAS::ID getOpenCLImageAddrSpace() const override { @@ -2848,6 +2851,7 @@ void setSupportedOpenCLOpts() override { getSupportedOpenCLOpts().setAll(); +setOpenCLExtensionOpts(); } }; @@ -8007,6 +8011,7 @@ // Assume all OpenCL extensions and optional core features are supported // for SPIR since it is a generic target. getSupportedOpenCLOpts().setAll(); +setOpenCLExtensionOpts(); } }; Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -391,6 +391,8 @@ HelpText<"OpenCL only. Allow denormals to be flushed to zero.">;