[Mesa-dev] [PATCH v2] clover: Allow OpenCL version override
Francisco Jerez
currojerez at riseup.net
Sun Oct 30 20:53:16 UTC 2016
Vedran Miletić <vedran at miletic.net> writes:
> CLOVER_CL_VERSION_OVERRIDE allows overriding default OpenCL version
> supported by Clover, analogous to MESA_GL_VERSION_OVERRIDE for OpenGL.
> CLOVER_CL_C_VERSION_OVERRIDE allows overridng default OpenCL C version.
>
> v2:
> - move version getters to version.hpp, simplify
> - set -cl-std= to CLOVER_CL_C_VERSION_OVERRIDE
> - set __OPENCL_VERSION__ to CLOVER_CL_VERSION_OVERRIDE
> ---
> docs/envvars.html | 12 ++++++
> src/gallium/state_trackers/clover/Makefile.sources | 3 +-
> src/gallium/state_trackers/clover/api/device.cpp | 5 ++-
> src/gallium/state_trackers/clover/api/platform.cpp | 3 +-
> .../state_trackers/clover/llvm/invocation.cpp | 30 ++++++++++++--
> src/gallium/state_trackers/clover/llvm/util.hpp | 4 +-
> src/gallium/state_trackers/clover/util/version.hpp | 48 ++++++++++++++++++++++
> 7 files changed, 95 insertions(+), 10 deletions(-)
> create mode 100644 src/gallium/state_trackers/clover/util/version.hpp
>
> diff --git a/docs/envvars.html b/docs/envvars.html
> index cf57ca5..f76827b 100644
> --- a/docs/envvars.html
> +++ b/docs/envvars.html
> @@ -235,6 +235,18 @@ Setting to "tgsi", for example, will print all the TGSI shaders.
> See src/mesa/state_tracker/st_debug.c for other options.
> </ul>
>
> +<h3>Clover state tracker environment variables</h3>
> +
> +<ul>
> +<li>CLOVER_CL_VERSION_OVERRIDE - allows overriding OpenCL version returned by
> + clGetPlatformInfo(CL_PLATFORM_VERSION) and
> + clGetDeviceInfo(CL_DEVICE_VERSION). Note that the setting sets the version
> + of the platform and all the devices to the same value.
I'd change this to something along the lines of "allows overriding the
OpenCL API version reported by the OpenCL platform and devices, as
reported by clGetPlatformInfo(CL_PLATFORM_VERSION) and
clGetDeviceInfo(CL_DEVICE_VERSION)".
> +<li>CLOVER_CL_C_VERSION_OVERRIDE - allows overriding OpenCL C version reported
> + by clGetDeviceInfo(CL_DEVICE_OPENCL_C_VERSION) and the value of the
> + __OPENCL_VERSION__ macro in the OpenCL compiler.
I don't think this is accurate, AFAIK __OPENCL_VERSION__ is supposed to
expand to the OpenCL API version (i.e. the value of
CLOVER_CL_VERSION_OVERRIDE), not the OpenCL C source language version.
Maybe use something along the lines of "allows overriding the source
language version version used by the OpenCL C compiler, as reported by
clGetDeviceInfo(CL_DEVICE_OPENCL_C_VERSION)".
You also probably need to rebase this on top of your previous commit.
> +</ul>
> +
> <h3>Softpipe driver environment variables</h3>
> <ul>
> <li>SOFTPIPE_DUMP_FS - if set, the softpipe driver will print fragment shaders
> diff --git a/src/gallium/state_trackers/clover/Makefile.sources b/src/gallium/state_trackers/clover/Makefile.sources
> index e9828b1..37f61b0 100644
> --- a/src/gallium/state_trackers/clover/Makefile.sources
> +++ b/src/gallium/state_trackers/clover/Makefile.sources
> @@ -50,7 +50,8 @@ CPP_SOURCES := \
> util/lazy.hpp \
> util/pointer.hpp \
> util/range.hpp \
> - util/tuple.hpp
> + util/tuple.hpp \
> + util/version.hpp
>
I suggest you put this in api/util.hpp if you want to make them
stand-alone functions, since the clover/util library is largely
API-agnostic, and there's already some other CL version-related stuff in
api/util.hpp. Though I have the impression that your first approach of
adding clover::platform and ::device queries is somewhat more
future-looking, since it will enable us to make the version computation
device-dependent instead of it being basically a constant.
> LLVM_SOURCES := \
> llvm/codegen/bitcode.cpp \
> diff --git a/src/gallium/state_trackers/clover/api/device.cpp b/src/gallium/state_trackers/clover/api/device.cpp
> index f7bd61b..a759e0e 100644
> --- a/src/gallium/state_trackers/clover/api/device.cpp
> +++ b/src/gallium/state_trackers/clover/api/device.cpp
> @@ -23,6 +23,7 @@
> #include "api/util.hpp"
> #include "core/platform.hpp"
> #include "core/device.hpp"
> +#include "util/version.hpp"
> #include "git_sha1.h"
>
> using namespace clover;
> @@ -301,7 +302,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param,
> break;
>
> case CL_DEVICE_VERSION:
> - buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
> + buf.as_string() = "OpenCL " + get_opencl_version() + " Mesa " PACKAGE_VERSION
> #ifdef MESA_GIT_SHA1
> " (" MESA_GIT_SHA1 ")"
> #endif
> @@ -355,7 +356,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param,
> break;
>
> case CL_DEVICE_OPENCL_C_VERSION:
> - buf.as_string() = "OpenCL C 1.1 ";
> + buf.as_string() = "OpenCL C " + get_opencl_c_version() + " ";
> break;
>
> case CL_DEVICE_PARENT_DEVICE:
> diff --git a/src/gallium/state_trackers/clover/api/platform.cpp b/src/gallium/state_trackers/clover/api/platform.cpp
> index b1b1fdf..f3360fe 100644
> --- a/src/gallium/state_trackers/clover/api/platform.cpp
> +++ b/src/gallium/state_trackers/clover/api/platform.cpp
> @@ -22,6 +22,7 @@
>
> #include "api/util.hpp"
> #include "core/platform.hpp"
> +#include "util/version.hpp"
> #include "git_sha1.h"
>
> using namespace clover;
> @@ -58,7 +59,7 @@ clover::GetPlatformInfo(cl_platform_id d_platform, cl_platform_info param,
> break;
>
> case CL_PLATFORM_VERSION:
> - buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
> + buf.as_string() = "OpenCL " + get_opencl_version() + " Mesa " PACKAGE_VERSION
> #ifdef MESA_GIT_SHA1
> " (" MESA_GIT_SHA1 ")"
> #endif
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index b5e8b52..6886dad 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -52,7 +52,7 @@
> #include "llvm/metadata.hpp"
> #include "llvm/util.hpp"
> #include "util/algorithm.hpp"
> -
> +#include "util/version.hpp"
>
> using namespace clover;
> using namespace clover::llvm;
> @@ -120,10 +120,23 @@ namespace {
> // http://www.llvm.org/bugs/show_bug.cgi?id=19735
> c->getDiagnosticOpts().ShowCarets = false;
>
> + const std::string opencl_c_version = get_opencl_c_version();
> + clang::LangStandard::Kind lang_kind;
> + if (opencl_c_version == "1.0")
> + lang_kind = clang::LangStandard::lang_opencl;
> + else if (opencl_c_version == "1.1")
> + lang_kind = clang::LangStandard::lang_opencl11;
> + else if (opencl_c_version == "1.2")
> + lang_kind = clang::LangStandard::lang_opencl12;
> + else if (opencl_c_version == "2.0")
> + lang_kind = clang::LangStandard::lang_opencl20;
> + else
> + invalid_build_options_error("Invalid OpenCL version string: " +
> + opencl_c_version);
> +
This is creating a build options error but not throwing it :). In order
to eliminate the coupling between the API and compiler layers I think it
would make sense to use the "opts" argument of compile_program() and
link_program() for clover::program to specify what the default language
version should be (e.g. by using the regular cl-std option). This will
allow more interesting default language version selection logic than a
global environment variable, e.g. based on the capabilities of the
device.
> compat::set_lang_defaults(c->getInvocation(), c->getLangOpts(),
> clang::IK_OpenCL, ::llvm::Triple(target.triple),
> - c->getPreprocessorOpts(),
> - clang::LangStandard::lang_opencl11);
> + c->getPreprocessorOpts(), lang_kind);
>
> c->createDiagnostics(new clang::TextDiagnosticPrinter(
> *new raw_string_ostream(r_log),
> @@ -154,7 +167,16 @@ namespace {
> c.getPreprocessorOpts().Includes.push_back("clc/clc.h");
>
> // Add definition for the OpenCL version
> - c.getPreprocessorOpts().addMacroDef("__OPENCL_VERSION__=110");
> + const std::string opencl_version = get_opencl_version();
> + auto ocl_version_major_minor = tokenize(opencl_version, '.');
> + if (ocl_version_major_minor.size() != 2) {
> + invalid_build_options_error("Invalid OpenCL version string: " + opencl_version);
> + }
> + int ocl_version_major = stoi(ocl_version_major_minor[0]);
> + int ocl_version_minor = stoi(ocl_version_major_minor[1]);
> + int ocl_version_number = ocl_version_major * 100 + ocl_version_minor * 10;
> + c.getPreprocessorOpts().addMacroDef("__OPENCL_VERSION__=" +
> + std::to_string(ocl_version_number));
>
Same here. We could pass this as a '-D' compiler option to
compile_program() in order to make sure that Clover is in control of the
reported OpenCL API version.
> // clc.h requires that this macro be defined:
> c.getPreprocessorOpts().addMacroDef("cl_clang_storage_class_specifiers");
> diff --git a/src/gallium/state_trackers/clover/llvm/util.hpp b/src/gallium/state_trackers/clover/llvm/util.hpp
> index 8db6f20..fd47d3a 100644
> --- a/src/gallium/state_trackers/clover/llvm/util.hpp
> +++ b/src/gallium/state_trackers/clover/llvm/util.hpp
> @@ -40,12 +40,12 @@ namespace clover {
> }
>
> inline std::vector<std::string>
> - tokenize(const std::string &s) {
> + tokenize(const std::string &s, const char sep = ' ') {
> std::vector<std::string> ss;
> std::istringstream iss(s);
> std::string t;
>
> - while (getline(iss, t, ' '))
> + while (getline(iss, t, sep))
> ss.push_back(t);
>
> return ss;
> diff --git a/src/gallium/state_trackers/clover/util/version.hpp b/src/gallium/state_trackers/clover/util/version.hpp
> new file mode 100644
> index 0000000..d94e00f
> --- /dev/null
> +++ b/src/gallium/state_trackers/clover/util/version.hpp
> @@ -0,0 +1,48 @@
> +//
> +// Copyright 2016 Vedran Miletić
> +//
> +// Permission is hereby granted, free of charge, to any person obtaining a
> +// copy of this software and associated documentation files (the "Software"),
> +// to deal in the Software without restriction, including without limitation
> +// the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +// and/or sell copies of the Software, and to permit persons to whom the
> +// Software is furnished to do so, subject to the following conditions:
> +//
> +// The above copyright notice and this permission notice shall be included in
> +// all copies or substantial portions of the Software.
> +//
> +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> +// THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> +// OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> +// ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> +// OTHER DEALINGS IN THE SOFTWARE.
> +//
> +
> +#ifndef CLOVER_UTIL_VERSION_HPP
> +#define CLOVER_UTIL_VERSION_HPP
> +
> +#include <string>
> +
> +#include "util/u_debug.h"
> +
> +namespace clover {
> + ///
> + /// Return the OpenCL version supported by Clover.
> + ///
> + inline std::string
> + get_opencl_version() {
> + return debug_get_option("CLOVER_CL_VERSION_OVERRIDE", "1.1");
> + }
> +
> + ///
> + /// Return the OpenCL C version supported by Clover.
> + ///
> + inline std::string
> + get_opencl_c_version() {
> + return debug_get_option("CLOVER_CL_C_VERSION_OVERRIDE", "1.1");
> + }
> +}
> +
> +#endif
> --
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161030/1259a336/attachment-0001.sig>
More information about the mesa-dev
mailing list