Pierre Moreau <pierre.mor...@free.fr> writes:
> Signed-off-by: Pierre Moreau <pierre.mor...@free.fr>
> ---
> src/gallium/state_trackers/clover/api/dispatch.hpp | 4 ++
> src/gallium/state_trackers/clover/api/program.cpp | 29 ++++++++-
> src/gallium/state_trackers/clover/core/program.cpp | 68
> +++++++++++++++++++++-
> src/gallium/state_trackers/clover/core/program.hpp | 14 +++++
> 4 files changed, 110 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/api/dispatch.hpp
> b/src/gallium/state_trackers/clover/api/dispatch.hpp
> index 0910e19422..21bd379fe6 100644
> --- a/src/gallium/state_trackers/clover/api/dispatch.hpp
> +++ b/src/gallium/state_trackers/clover/api/dispatch.hpp
> @@ -900,6 +900,10 @@ namespace clover {
> cl_int
> IcdGetPlatformIDsKHR(cl_uint num_entries, cl_platform_id *rd_platforms,
> cl_uint *rnum_platforms);
> +
> + cl_program
> + CreateProgramWithILKHR(cl_context d_ctx, const void *il,
> + const size_t length, cl_int *r_errcode);
Make the the length argument non-const to match the spec's type
signature.
> }
>
> #endif
> diff --git a/src/gallium/state_trackers/clover/api/program.cpp
> b/src/gallium/state_trackers/clover/api/program.cpp
> index 6ec87ad128..ed3b679c7c 100644
> --- a/src/gallium/state_trackers/clover/api/program.cpp
> +++ b/src/gallium/state_trackers/clover/api/program.cpp
> @@ -22,6 +22,7 @@
>
> #include "api/util.hpp"
> #include "core/program.hpp"
> +#include "spirv/invocation.hpp"
> #include "util/u_debug.h"
>
> #include <sstream>
> @@ -128,6 +129,30 @@ clCreateProgramWithBinary(cl_context d_ctx, cl_uint n,
> return NULL;
> }
>
> +cl_program
> +clover::CreateProgramWithILKHR(cl_context d_ctx, const void *il,
> + const size_t length, cl_int *r_errcode) try {
> + auto &ctx = obj(d_ctx);
> +
> + if (!il || !length)
> + throw error(CL_INVALID_VALUE);
> +
> + uint32_t type = UINT32_MAX;
Since you're using pipe_shader_ir enumerants to represent the IL format
it would be a good idea to declare the IL type variables as such here
and below.
> + if (spirv::is_binary_spirv(reinterpret_cast<const char*>(il)))
> + type = PIPE_SHADER_IR_SPIRV;
> +
> + if (type == UINT32_MAX)
> + throw error(CL_INVALID_VALUE);
> +
> + // initialize a program object with it.
Capitalize the comment.
> + ret_error(r_errcode, CL_SUCCESS);
> + return new program(ctx, il, length, type);
> +
> +} catch (error &e) {
> + ret_error(r_errcode, e);
> + return NULL;
> +}
> +
> CLOVER_API cl_program
> clCreateProgramWithBuiltInKernels(cl_context d_ctx, cl_uint n,
> const cl_device_id *d_devs,
> @@ -183,7 +208,7 @@ clBuildProgram(cl_program d_prog, cl_uint num_devs,
>
> validate_build_common(prog, num_devs, d_devs, pfn_notify, user_data);
>
> - if (prog.has_source) {
> + if (prog.has_source || prog.has_il) {
> prog.compile(devs, opts);
> prog.link(devs, opts, { prog });
> } else if (any_of([&](const device &dev){
> @@ -221,7 +246,7 @@ clCompileProgram(cl_program d_prog, cl_uint num_devs,
> if (bool(num_headers) != bool(header_names))
> throw error(CL_INVALID_VALUE);
>
> - if (!prog.has_source)
> + if (!prog.has_source && !prog.has_il)
> throw error(CL_INVALID_OPERATION);
>
> for_each([&](const char *name, const program &header) {
> diff --git a/src/gallium/state_trackers/clover/core/program.cpp
> b/src/gallium/state_trackers/clover/core/program.cpp
> index 976213ef95..b9ba38d4e6 100644
> --- a/src/gallium/state_trackers/clover/core/program.cpp
> +++ b/src/gallium/state_trackers/clover/core/program.cpp
> @@ -24,24 +24,51 @@
> #include "llvm/invocation.hpp"
> #include "spirv/invocation.hpp"
> #include "tgsi/invocation.hpp"
> +#include "spirv/invocation.hpp"
> +
> +#include <cstring>
>
> using namespace clover;
>
> program::program(clover::context &ctx, const std::string &source) :
> - has_source(true), context(ctx), _source(source), _kernel_ref_counter(0) {
> + has_source(true), has_il(false), il_type(UINT32_MAX), context(ctx),
> + _source(source), _kernel_ref_counter(0), _il(nullptr), _length(0) {
> }
>
> program::program(clover::context &ctx,
> const ref_vector<device> &devs,
> const std::vector<module> &binaries) :
> - has_source(false), context(ctx),
> - _devices(devs), _kernel_ref_counter(0) {
> + has_source(false), has_il(false), il_type(UINT32_MAX), context(ctx),
> + _devices(devs), _kernel_ref_counter(0), _il(nullptr), _length(0) {
> for_each([&](device &dev, const module &bin) {
> _builds[&dev] = { bin };
> },
> devs, binaries);
> }
>
> +program::program(clover::context &ctx, const void *il, const size_t length,
il/length (and their program member counterparts) look like an
open-coded std::vector.
> + const uint32_t type) :
> + has_source(false), has_il(true), il_type(type), context(ctx),
> + _kernel_ref_counter(0), _il(nullptr), _length(length) {
> + switch (il_type) {
> + case PIPE_SHADER_IR_SPIRV: {
> + const char *c_il = reinterpret_cast<const char *>(il);
> + _il = spirv::spirv_to_cpu(c_il, length);
Why isn't the endianness conversion an implementation detail of
clover::spirv::process_program()? It seems like it could be made a
private function to invocation.cpp and simplify the interface.
> + }
> + break;
> + }
> +}
> +
> +program::~program() {
> + if (has_il) {
> + switch (il_type) {
> + case PIPE_SHADER_IR_SPIRV:
> + delete[] reinterpret_cast<const char *>(_il);
> + break;
> + }
> + }
> +}
> +
This can go away if you switch to a memory-safe data structure.
> void
> program::compile(const ref_vector<device> &devs, const std::string &opts,
> const header_map &headers) {
> @@ -66,6 +93,31 @@ program::compile(const ref_vector<device> &devs, const
> std::string &opts,
> throw;
> }
> }
> + } else if (has_il) {
> + _devices = devs;
> +
> + for (auto &dev : devs) {
> + std::string log;
> +
> + try {
> + switch (il_type) {
> + case PIPE_SHADER_IR_SPIRV:
> + if (!dev.supports_ir(PIPE_SHADER_IR_SPIRV)) {
> + log = "Device does not support SPIR-V as IL\n";
> + throw error(CL_INVALID_BINARY);
> + }
> + _builds[&dev] = { spirv::process_program(_il, _length, dev,
> log),
> + opts, log };
> + break;
> + default:
> + log = "Only SPIR-V is supported as IL by clover for now\n";
> + throw error(CL_INVALID_BINARY);
> + }
> + } catch (const error &) {
> + _builds[&dev] = { module(), opts, log };
> + throw;
> + }
> + }
This is duplicating most of the previous block. It would make more
sense to make the previous block execute if (has_source || has_il), and
add a demuxing 'module compile_program(...)' function which will call
the right compile function based on the IR format (c.f. suggestion for
PATCH 13).
> }
> }
>
> @@ -104,6 +156,16 @@ program::link(const ref_vector<device> &devs, const
> std::string &opts,
> }
> }
>
> +const void *
> +program::il() const {
> + return _il;
> +}
> +
> +size_t
> +program::length() const {
> + return _length;
> +}
> +
> const std::string &
> program::source() const {
> return _source;
> diff --git a/src/gallium/state_trackers/clover/core/program.hpp
> b/src/gallium/state_trackers/clover/core/program.hpp
> index 05964e78a7..77a79c5d94 100644
> --- a/src/gallium/state_trackers/clover/core/program.hpp
> +++ b/src/gallium/state_trackers/clover/core/program.hpp
> @@ -43,11 +43,17 @@ namespace clover {
> program(clover::context &ctx,
> const ref_vector<device> &devs = {},
> const std::vector<module> &binaries = {});
> + program(clover::context &ctx,
> + const void *il,
> + const size_t length,
> + const uint32_t type);
>
> program(const program &prog) = delete;
> program &
> operator=(const program &prog) = delete;
>
> + ~program();
> +
> void compile(const ref_vector<device> &devs, const std::string &opts,
> const header_map &headers = {});
> void link(const ref_vector<device> &devs, const std::string &opts,
> @@ -56,6 +62,12 @@ namespace clover {
> const bool has_source;
> const std::string &source() const;
>
> + const bool has_il;
> + const uint32_t il_type;
> + const void *il() const;
> +
> + size_t length() const;
> +
> device_range devices() const;
>
> struct build {
> @@ -85,6 +97,8 @@ namespace clover {
> std::map<const device *, struct build> _builds;
> std::string _source;
> ref_counter _kernel_ref_counter;
> + const void *_il;
> + size_t _length;
> };
> }
>
> --
> 2.16.0
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
