[Mesa-dev] [PATCH] clover: Allow OpenCL version override
Serge Martin
edb+mesa at sigluy.net
Fri Oct 7 10:05:53 UTC 2016
On Thursday 06 October 2016 16:26:21 Vedran Miletić wrote:
> 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.
Thanks.
I've made some comments bellow
> ---
> docs/envvars.html | 12 ++++++++++++
> src/gallium/state_trackers/clover/api/device.cpp | 4 ++--
> src/gallium/state_trackers/clover/api/platform.cpp | 4 ++--
> src/gallium/state_trackers/clover/core/device.cpp | 19
> +++++++++++++++++++ src/gallium/state_trackers/clover/core/device.hpp |
> 4 ++++
> src/gallium/state_trackers/clover/core/platform.cpp | 9 +++++++++
> src/gallium/state_trackers/clover/core/platform.hpp | 3 +++
> src/gallium/state_trackers/clover/core/program.cpp | 4 +++-
> src/gallium/state_trackers/clover/llvm/invocation.cpp | 18
> ++++++++++++++---- src/gallium/state_trackers/clover/llvm/invocation.hpp |
> 1 +
> src/gallium/state_trackers/clover/llvm/util.hpp | 4 ++--
> 11 files changed, 71 insertions(+), 11 deletions(-)
>
> 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.
> +<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 we need CLOVER_CL_C_VERSION_OVERRIDE. CLOVER_CL_VERSION_OVERRIDE
should be enough.
> +</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/api/device.cpp
> b/src/gallium/state_trackers/clover/api/device.cpp index f7bd61b..e23de7a
> 100644
> --- a/src/gallium/state_trackers/clover/api/device.cpp
> +++ b/src/gallium/state_trackers/clover/api/device.cpp
> @@ -301,7 +301,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 " + dev.opencl_version() + " Mesa "
> PACKAGE_VERSION #ifdef MESA_GIT_SHA1
> " (" MESA_GIT_SHA1 ")"
> #endif
> @@ -355,7 +355,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 " + dev.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..f344ec8
> 100644
> --- a/src/gallium/state_trackers/clover/api/platform.cpp
> +++ b/src/gallium/state_trackers/clover/api/platform.cpp
> @@ -50,7 +50,7 @@ clover::GetPlatformInfo(cl_platform_id d_platform,
> cl_platform_info param, size_t size, void *r_buf, size_t *r_size) try {
> property_buffer buf { r_buf, size, r_size };
>
> - obj(d_platform);
> + auto &platform = obj(d_platform);
>
> switch (param) {
> case CL_PLATFORM_PROFILE:
> @@ -58,7 +58,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 " + platform.opencl_version() + " Mesa "
you don't need to add an opencl_version() func to core/plateform,
it won't be used except here.
I prefer if we have an util function that would be used here, in device.cpp
and invocation.cpp because for the moment this is still a global value.
> PACKAGE_VERSION #ifdef MESA_GIT_SHA1
> " (" MESA_GIT_SHA1 ")"
> #endif
> diff --git a/src/gallium/state_trackers/clover/core/device.cpp
> b/src/gallium/state_trackers/clover/core/device.cpp index 8825f99..fce6fb3
> 100644
> --- a/src/gallium/state_trackers/clover/core/device.cpp
> +++ b/src/gallium/state_trackers/clover/core/device.cpp
> @@ -24,6 +24,7 @@
> #include "core/platform.hpp"
> #include "pipe/p_screen.h"
> #include "pipe/p_state.h"
> +#include "util/u_debug.h"
>
> using namespace clover;
>
> @@ -48,6 +49,14 @@ device::device(clover::platform &platform,
> pipe_loader_device *ldev) : pipe->destroy(pipe);
> throw error(CL_INVALID_DEVICE);
> }
> +
> + const std::string cl_version_override =
> + debug_get_option("CLOVER_CL_VERSION_OVERRIDE",
> ""); + ocl_version = !cl_version_override.empty() ? cl_version_override :
> "1.1";
This is what the default value of debug_get_option is for, this is redundant.
You just have to pass "1.1".
Also, if we have a util function that statistically keep the value the extra
device variable and funcs are not needed.
We may need it if we go for CL 2.0 latter with some devices only advertising
CL 1.2 because of unsupported feature, but not for the moment.
> +
> + const std::string clc_version_override =
> + debug_get_option("CLOVER_CLC_VERSION_OVERRIDE",
> ""); + oclc_version = !clc_version_override.empty() ?
> clc_version_override : "1.1"; }
>
> device::~device() {
> @@ -209,6 +218,16 @@ device::vendor_name() const {
> return pipe->get_device_vendor(pipe);
> }
>
> +std::string
> +device::opencl_version() const {
> + return ocl_version;
> +}
> +
> +std::string
> +device::opencl_c_version() const {
> + return oclc_version;
> +}
> +
> enum pipe_shader_ir
> device::ir_format() const {
> return (enum pipe_shader_ir) pipe->get_shader_param(
> diff --git a/src/gallium/state_trackers/clover/core/device.hpp
> b/src/gallium/state_trackers/clover/core/device.hpp index 6cf6c7f..b71cafd
> 100644
> --- a/src/gallium/state_trackers/clover/core/device.hpp
> +++ b/src/gallium/state_trackers/clover/core/device.hpp
> @@ -71,6 +71,8 @@ namespace clover {
> cl_uint address_bits() const;
> std::string device_name() const;
> std::string vendor_name() const;
> + std::string opencl_version() const;
> + std::string opencl_c_version() const;
> enum pipe_shader_ir ir_format() const;
> std::string ir_target() const;
> enum pipe_endian endianness() const;
> @@ -86,6 +88,8 @@ namespace clover {
> private:
> pipe_screen *pipe;
> pipe_loader_device *ldev;
> + std::string ocl_version;
> + std::string oclc_version;
> };
> }
>
> diff --git a/src/gallium/state_trackers/clover/core/platform.cpp
> b/src/gallium/state_trackers/clover/core/platform.cpp index
> 489e8dc..c5d47f0 100644
> --- a/src/gallium/state_trackers/clover/core/platform.cpp
> +++ b/src/gallium/state_trackers/clover/core/platform.cpp
> @@ -21,6 +21,7 @@
> //
>
> #include "core/platform.hpp"
> +#include "util/u_debug.h"
>
> using namespace clover;
>
> @@ -38,4 +39,12 @@ platform::platform() : adaptor_range(evals(), devs) {
> pipe_loader_release(&ldev, 1);
> }
> }
> +
> + const std::string cl_version_override =
> + debug_get_option("CLOVER_CL_VERSION_OVERRIDE",
> ""); + ocl_version = !cl_version_override.empty() ? cl_version_override :
> "1.1"; +}
> +
> +std::string platform::opencl_version() {
> + return ocl_version;
> }
> diff --git a/src/gallium/state_trackers/clover/core/platform.hpp
> b/src/gallium/state_trackers/clover/core/platform.hpp index
> e849645..b283044 100644
> --- a/src/gallium/state_trackers/clover/core/platform.hpp
> +++ b/src/gallium/state_trackers/clover/core/platform.hpp
> @@ -40,8 +40,11 @@ namespace clover {
> platform &
> operator=(const platform &platform) = delete;
>
> + std::string opencl_version();
> +
> protected:
> std::vector<intrusive_ref<device>> devs;
> + std::string ocl_version;
> };
> }
>
> diff --git a/src/gallium/state_trackers/clover/core/program.cpp
> b/src/gallium/state_trackers/clover/core/program.cpp index 79ac851..0cb5e77
> 100644
> --- a/src/gallium/state_trackers/clover/core/program.cpp
> +++ b/src/gallium/state_trackers/clover/core/program.cpp
> @@ -54,7 +54,9 @@ program::compile(const ref_vector<device> &devs, const
> std::string &opts, const module m = (dev.ir_format() == PIPE_SHADER_IR_TGSI
> ? tgsi::compile_program(_source, log) : llvm::compile_program(_source,
> headers, -
> dev.ir_target(), opts, log)); +
> dev.ir_target(), +
> dev.opencl_c_version(), +
> opts, log));
> _builds[&dev] = { m, opts, log };
> } catch (...) {
> _builds[&dev] = { module(), opts, log };
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp index
> b5e8b52..2a47acf 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -24,6 +24,8 @@
> // OTHER DEALINGS IN THE SOFTWARE.
> //
>
> +#include <cassert>
> +
> #include <llvm/IR/DiagnosticPrinter.h>
> #include <llvm/IR/DiagnosticInfo.h>
> #include <llvm/IR/LLVMContext.h>
> @@ -139,7 +141,8 @@ namespace {
> compile(LLVMContext &ctx, clang::CompilerInstance &c,
> const std::string &name, const std::string &source,
> const header_map &headers, const std::string &target,
> - const std::string &opts, std::string &r_log) {
> + const std::string &opencl_version, const std::string &opts,
> + std::string &r_log) {
The version arg is not needed for the moment, all device will advertise the
same version for the moment. Please use an util func
> c.getFrontendOpts().ProgramAction = clang::frontend::EmitLLVMOnly;
> c.getHeaderSearchOpts().UseBuiltinIncludes = true;
> c.getHeaderSearchOpts().UseStandardSystemIncludes = true;
> @@ -154,7 +157,13 @@ namespace {
> c.getPreprocessorOpts().Includes.push_back("clc/clc.h");
>
> // Add definition for the OpenCL version
> - c.getPreprocessorOpts().addMacroDef("__OPENCL_VERSION__=110");
> + auto ocl_version_major_minor = tokenize(opencl_version, '.');
> + assert(ocl_version_major_minor.size() == 2);
I'm not fan of doing assert when we have a way to report error on user.
If the version is wrong, stop the compilation with CL_INVALID_BUILD_OPTION or
some things like that and fill the log.
> + 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));
so this version[0] + version[2] + "0". I think we can go without stoi.
Also you need to change lang_opencl11 that is passed as default unless adding
cl-std=XX (when XX > 1.1) on command line override it.
Serge
>
> // clc.h requires that this macro be defined:
>
> c.getPreprocessorOpts().addMacroDef("cl_clang_storage_class_specifiers");
> @@ -197,6 +206,7 @@ module
> clover::llvm::compile_program(const std::string &source,
> const header_map &headers,
> const std::string &target,
> + const std::string &opencl_version,
> const std::string &opts,
> std::string &r_log) {
> if (has_flag(debug::clc))
> @@ -205,8 +215,8 @@ clover::llvm::compile_program(const std::string &source,
> auto ctx = create_context(r_log);
> auto c = create_compiler_instance(target, tokenize(opts + " input.cl"),
> r_log);
> - auto mod = compile(*ctx, *c, "input.cl", source, headers, target, opts,
> - r_log);
> + auto mod = compile(*ctx, *c, "input.cl", source, headers, target,
> + opencl_version, opts, r_log);
>
> if (has_flag(debug::llvm))
> debug::log(".ll", print_module_bitcode(*mod));
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.hpp
> b/src/gallium/state_trackers/clover/llvm/invocation.hpp index
> 5b3530c..2493cbf 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.hpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.hpp
> @@ -33,6 +33,7 @@ namespace clover {
> module compile_program(const std::string &source,
> const header_map &headers,
> const std::string &target,
> + const std::string &opencl_version,
> const std::string &opts,
> std::string &r_log);
>
> 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;
More information about the mesa-dev
mailing list