Re: [Mesa-dev] [PATCH 4/5 v6] clover/llvm: Add get_[cl|language]_version, validation and some helpers

[Pierre Moreau]

On 2018-03-01 — 20:02, Aaron Watry wrote:
> Used to calculate the default CLC language version based on the --cl-std in 
> build args
> and the device capabilities.
> 
> According to section 5.8.4.5 of the 2.0 spec, the CL C version is chosen by:
>  1) If you have -cl-std=CL1.1+ use the version specified
>  2) If not, use the highest 1.x version that the device supports
> 
> Curiously, there is no valid value for -cl-std=CL1.0
> 
> Validates requested cl-std against device_clc_version
> 
> Signed-off-by: Aaron Watry 
> Cc: Pierre Moreau 
> 
> v6: (Pierre) Add more const and fix some whitespace
> 
> v5: (Aaron) Use a collection of cl versions instead of switch cases
> Consolidates the string, numeric version, and clc langstandard::kind
> 
> v4: (Pierre) Split get_language_version addition and use into separate patches
> Squash patches that add the helpers and validate the language standard
> 
> v3: Change device_version to device_clc_version
> 
> v2: (Pierre) Move create_compiler_instance changes to correct patch
> to prevent temporary build breakage.
> Convert version_str into unsigned and use it to find language version
> Add build_error for unknown language version string
> Whitespace fixes
> ---
>  .../state_trackers/clover/llvm/invocation.cpp  | 63 
> ++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp 
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index 0bc06e..0f854b9049 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -63,6 +63,23 @@ using ::llvm::Module;
>  using ::llvm::raw_string_ostream;
>  
>  namespace {
> +
> +   struct cl_version {
> +  std::string version_str; // CL Version
> +  unsigned version_number; // Numeric CL Version
> +  clang::LangStandard::Kind clc_lang_standard; // lang standard for 
> version
> +   };
> +
> +   static const unsigned ANY_VERSION = 999;
> +   const cl_version cl_versions[] = {
> +  { "1.0", 100, clang::LangStandard::lang_opencl10},
> +  { "1.1", 110, clang::LangStandard::lang_opencl11},
> +  { "1.2", 120, clang::LangStandard::lang_opencl12},
> +  { "2.0", 200, clang::LangStandard::lang_opencl20},
> +  { "2.1", 210, clang::LangStandard::lang_unspecified}, //2.1 doesn't 
> exist
> +  { "2.2", 220, clang::LangStandard::lang_unspecified}, //2.2 doesn't 
> exist
> +   };

Given that OpenCL C version are no longer a 1:1 mapping to OpenCL API version,
and with the introduction of OpenCL C++ 1.0 with OpenCL 2.2, I would go with
two different tables:
* one matching strings to the integer version;
* one matching integer or string version, to the OpenCL C LangStandard
and later we can have a third one for matching integer/string to OpenCL C++
LangStandard.

> +
> void
> init_targets() {
>static bool targets_initialized = false;
> @@ -93,6 +110,52 @@ namespace {
>return ctx;
> }
>  
> +   const struct cl_version

I think you forgot the reference there, otherwise if you want to return a copy,
you can remove the `const` keyword as it has no impact on copies.

> +   get_cl_version(const std::string _str,
> +  unsigned max = ANY_VERSION) {
> +  for (const struct cl_version version : cl_versions) {

Please make `version` into a reference: no need to create a copy each time.

> + if (version.version_number == max || version.version_str == 
> version_str) {
> +return version;
> + }
> +  }
> +  throw build_error("Unknown/Unsupported language version");
> +   }
> +
> +   clang::LangStandard::Kind
> +   get_lang_standard_from_version_str(const std::string _str,
> +  bool is_build_opt = false) {
> +   /**
> +   * Per CL 2.0 spec, section 5.8.4.5:
> +   * If it's an option, use the value directly.
> +   * If it's a device version, clamp to max 1.x version, a.k.a. 1.2
> +   */

Please switch to regular C++-style comments.

> +  const struct cl_version version = get_cl_version(version_str,
> +  is_build_opt ? ANY_VERSION : 120);
> +  return version.clc_lang_standard;
> +   }
> +
> +   clang::LangStandard::Kind
> +   get_language_version(const std::vector ,
> +const std::string _version) {
> +
> +  const std::string search = "-cl-std=CL";
> +
> +  for (auto opt: opts) {

opt could be a constant reference, to avoid making copies of strings every
time.

With those modifications, this is
Reviewed-by: Pierre Moreau 

> + auto pos = opt.find(search);
> + if (pos == 0){
> +const auto ver = opt.substr(pos + search.size());
> +const auto device_ver = get_cl_version(device_version);
> +const auto requested = get_cl_version(ver);

Re: [Mesa-dev] [PATCH 4/5 v6] clover/llvm: Add get_[cl|language]_version, validation and some helpers

[Pierre Moreau]

Sorry, partially forgot about it. I’ll look at it over the weekend, as I don’t
have time before. :-/

Pierre


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 4/5 v6] clover/llvm: Add get_[cl|language]_version, validation and some helpers

[Aaron Watry]

ping.

--Aaron

On Thu, Mar 1, 2018 at 8:02 PM, Aaron Watry  wrote:
> Used to calculate the default CLC language version based on the --cl-std in 
> build args
> and the device capabilities.
>
> According to section 5.8.4.5 of the 2.0 spec, the CL C version is chosen by:
>  1) If you have -cl-std=CL1.1+ use the version specified
>  2) If not, use the highest 1.x version that the device supports
>
> Curiously, there is no valid value for -cl-std=CL1.0
>
> Validates requested cl-std against device_clc_version
>
> Signed-off-by: Aaron Watry 
> Cc: Pierre Moreau 
>
> v6: (Pierre) Add more const and fix some whitespace
>
> v5: (Aaron) Use a collection of cl versions instead of switch cases
> Consolidates the string, numeric version, and clc langstandard::kind
>
> v4: (Pierre) Split get_language_version addition and use into separate patches
> Squash patches that add the helpers and validate the language standard
>
> v3: Change device_version to device_clc_version
>
> v2: (Pierre) Move create_compiler_instance changes to correct patch
> to prevent temporary build breakage.
> Convert version_str into unsigned and use it to find language version
> Add build_error for unknown language version string
> Whitespace fixes
> ---
>  .../state_trackers/clover/llvm/invocation.cpp  | 63 
> ++
>  1 file changed, 63 insertions(+)
>
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp 
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index 0bc06e..0f854b9049 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -63,6 +63,23 @@ using ::llvm::Module;
>  using ::llvm::raw_string_ostream;
>
>  namespace {
> +
> +   struct cl_version {
> +  std::string version_str; // CL Version
> +  unsigned version_number; // Numeric CL Version
> +  clang::LangStandard::Kind clc_lang_standard; // lang standard for 
> version
> +   };
> +
> +   static const unsigned ANY_VERSION = 999;
> +   const cl_version cl_versions[] = {
> +  { "1.0", 100, clang::LangStandard::lang_opencl10},
> +  { "1.1", 110, clang::LangStandard::lang_opencl11},
> +  { "1.2", 120, clang::LangStandard::lang_opencl12},
> +  { "2.0", 200, clang::LangStandard::lang_opencl20},
> +  { "2.1", 210, clang::LangStandard::lang_unspecified}, //2.1 doesn't 
> exist
> +  { "2.2", 220, clang::LangStandard::lang_unspecified}, //2.2 doesn't 
> exist
> +   };
> +
> void
> init_targets() {
>static bool targets_initialized = false;
> @@ -93,6 +110,52 @@ namespace {
>return ctx;
> }
>
> +   const struct cl_version
> +   get_cl_version(const std::string _str,
> +  unsigned max = ANY_VERSION) {
> +  for (const struct cl_version version : cl_versions) {
> + if (version.version_number == max || version.version_str == 
> version_str) {
> +return version;
> + }
> +  }
> +  throw build_error("Unknown/Unsupported language version");
> +   }
> +
> +   clang::LangStandard::Kind
> +   get_lang_standard_from_version_str(const std::string _str,
> +  bool is_build_opt = false) {
> +   /**
> +   * Per CL 2.0 spec, section 5.8.4.5:
> +   * If it's an option, use the value directly.
> +   * If it's a device version, clamp to max 1.x version, a.k.a. 1.2
> +   */
> +  const struct cl_version version = get_cl_version(version_str,
> +  is_build_opt ? ANY_VERSION : 120);
> +  return version.clc_lang_standard;
> +   }
> +
> +   clang::LangStandard::Kind
> +   get_language_version(const std::vector ,
> +const std::string _version) {
> +
> +  const std::string search = "-cl-std=CL";
> +
> +  for (auto opt: opts) {
> + auto pos = opt.find(search);
> + if (pos == 0){
> +const auto ver = opt.substr(pos + search.size());
> +const auto device_ver = get_cl_version(device_version);
> +const auto requested = get_cl_version(ver);
> +if (requested.version_number > device_ver.version_number) {
> +   throw build_error();
> +}
> +return get_lang_standard_from_version_str(ver, true);
> + }
> +  }
> +
> +  return get_lang_standard_from_version_str(device_version);
> +   }
> +
> std::unique_ptr
> create_compiler_instance(const device ,
>  const std::vector ,
> --
> 2.14.1
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 4/5 v6] clover/llvm: Add get_[cl|language]_version, validation and some helpers

[Aaron Watry]

Used to calculate the default CLC language version based on the --cl-std in 
build args
and the device capabilities.

According to section 5.8.4.5 of the 2.0 spec, the CL C version is chosen by:
 1) If you have -cl-std=CL1.1+ use the version specified
 2) If not, use the highest 1.x version that the device supports

Curiously, there is no valid value for -cl-std=CL1.0

Validates requested cl-std against device_clc_version

Signed-off-by: Aaron Watry 
Cc: Pierre Moreau 

v6: (Pierre) Add more const and fix some whitespace

v5: (Aaron) Use a collection of cl versions instead of switch cases
Consolidates the string, numeric version, and clc langstandard::kind

v4: (Pierre) Split get_language_version addition and use into separate patches
Squash patches that add the helpers and validate the language standard

v3: Change device_version to device_clc_version

v2: (Pierre) Move create_compiler_instance changes to correct patch
to prevent temporary build breakage.
Convert version_str into unsigned and use it to find language version
Add build_error for unknown language version string
Whitespace fixes
---
 .../state_trackers/clover/llvm/invocation.cpp  | 63 ++
 1 file changed, 63 insertions(+)

diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp 
b/src/gallium/state_trackers/clover/llvm/invocation.cpp
index 0bc06e..0f854b9049 100644
--- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
+++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
@@ -63,6 +63,23 @@ using ::llvm::Module;
 using ::llvm::raw_string_ostream;
 
 namespace {
+
+   struct cl_version {
+  std::string version_str; // CL Version
+  unsigned version_number; // Numeric CL Version
+  clang::LangStandard::Kind clc_lang_standard; // lang standard for version
+   };
+
+   static const unsigned ANY_VERSION = 999;
+   const cl_version cl_versions[] = {
+  { "1.0", 100, clang::LangStandard::lang_opencl10},
+  { "1.1", 110, clang::LangStandard::lang_opencl11},
+  { "1.2", 120, clang::LangStandard::lang_opencl12},
+  { "2.0", 200, clang::LangStandard::lang_opencl20},
+  { "2.1", 210, clang::LangStandard::lang_unspecified}, //2.1 doesn't exist
+  { "2.2", 220, clang::LangStandard::lang_unspecified}, //2.2 doesn't exist
+   };
+
void
init_targets() {
   static bool targets_initialized = false;
@@ -93,6 +110,52 @@ namespace {
   return ctx;
}
 
+   const struct cl_version
+   get_cl_version(const std::string _str,
+  unsigned max = ANY_VERSION) {
+  for (const struct cl_version version : cl_versions) {
+ if (version.version_number == max || version.version_str == 
version_str) {
+return version;
+ }
+  }
+  throw build_error("Unknown/Unsupported language version");
+   }
+
+   clang::LangStandard::Kind
+   get_lang_standard_from_version_str(const std::string _str,
+  bool is_build_opt = false) {
+   /**
+   * Per CL 2.0 spec, section 5.8.4.5:
+   * If it's an option, use the value directly.
+   * If it's a device version, clamp to max 1.x version, a.k.a. 1.2
+   */
+  const struct cl_version version = get_cl_version(version_str,
+  is_build_opt ? ANY_VERSION : 120);
+  return version.clc_lang_standard;
+   }
+
+   clang::LangStandard::Kind
+   get_language_version(const std::vector ,
+const std::string _version) {
+
+  const std::string search = "-cl-std=CL";
+
+  for (auto opt: opts) {
+ auto pos = opt.find(search);
+ if (pos == 0){
+const auto ver = opt.substr(pos + search.size());
+const auto device_ver = get_cl_version(device_version);
+const auto requested = get_cl_version(ver);
+if (requested.version_number > device_ver.version_number) {
+   throw build_error();
+}
+return get_lang_standard_from_version_str(ver, true);
+ }
+  }
+
+  return get_lang_standard_from_version_str(device_version);
+   }
+
std::unique_ptr
create_compiler_instance(const device ,
 const std::vector ,
-- 
2.14.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev