[Mesa-dev] [PATCH 06/15] clover/spirv: Add functions for validating SPIR-V binaries
Francisco Jerez
currojerez at riseup.net
Sat May 11 20:55:05 UTC 2019
Karol Herbst <kherbst at redhat.com> writes:
> From: Pierre Moreau <pierre.morrow at free.fr>
>
> Changes since:
> * v12: remove autotools (Karol Herbst)
> * v11: Fix compilation error introduced in v11.
> * v10:
> - Reuse format_validation_msg in is_valid_spirv.
> - Remove LVL2STR macro in format_validation_msg.
> * v9: Add `clover_cpp_std` to the overrides of the `libclspirv` target
> in Meson.
> * v7: Add DEFINES to libclspirv and libclover, in autotools, as they
> would otherwise never know whether CLOVER_ALLOW_SPIRV has been
> defined (Dave Airlie)
> * v6: Update the dependency name (meson) and the libs variable
> (Makefile) due to the replacement of llvm-spirv to the new
> official SPIRV-LLVM-Translator.
> * v5: Changed to match the updated “clover/llvm: Allow translating from
> SPIR-V to LLVM IR” in the v6.
>
> Reviewed-by: Karol Herbst <kherbst at redhat.com>
> ---
> .../state_trackers/clover/Makefile.sources | 4 +
> src/gallium/state_trackers/clover/meson.build | 11 +-
> .../clover/spirv/invocation.cpp | 129 ++++++++++++++++++
> .../clover/spirv/invocation.hpp | 47 +++++++
> 4 files changed, 190 insertions(+), 1 deletion(-)
> create mode 100644 src/gallium/state_trackers/clover/spirv/invocation.cpp
> create mode 100644 src/gallium/state_trackers/clover/spirv/invocation.hpp
>
> diff --git a/src/gallium/state_trackers/clover/Makefile.sources b/src/gallium/state_trackers/clover/Makefile.sources
> index 5167ca75af4..38f94981fb6 100644
> --- a/src/gallium/state_trackers/clover/Makefile.sources
> +++ b/src/gallium/state_trackers/clover/Makefile.sources
> @@ -62,3 +62,7 @@ LLVM_SOURCES := \
> llvm/invocation.hpp \
> llvm/metadata.hpp \
> llvm/util.hpp
> +
> +SPIRV_SOURCES := \
> + spirv/invocation.cpp \
> + spirv/invocation.hpp
> diff --git a/src/gallium/state_trackers/clover/meson.build b/src/gallium/state_trackers/clover/meson.build
> index 311dcb69a6b..461c69f54c0 100644
> --- a/src/gallium/state_trackers/clover/meson.build
> +++ b/src/gallium/state_trackers/clover/meson.build
> @@ -57,6 +57,15 @@ libclllvm = static_library(
> override_options : clover_cpp_std,
> )
>
> +libclspirv = static_library(
> + 'clspirv',
> + files('spirv/invocation.cpp', 'spirv/invocation.hpp'),
> + include_directories : clover_incs,
> + cpp_args : [clover_spirv_cpp_args, cpp_vis_args],
> + dependencies : [dep_spirv_tools],
> + override_options : clover_cpp_std,
> +)
> +
> clover_files = files(
> 'api/context.cpp',
> 'api/device.cpp',
> @@ -117,6 +126,6 @@ libclover = static_library(
> [clover_files, sha1_h],
> include_directories : clover_incs,
> cpp_args : [clover_spirv_cpp_args, clover_cpp_args, cpp_vis_args],
> - link_with : [libclllvm],
> + link_with : [libclllvm, libclspirv],
> override_options : clover_cpp_std,
> )
> diff --git a/src/gallium/state_trackers/clover/spirv/invocation.cpp b/src/gallium/state_trackers/clover/spirv/invocation.cpp
> new file mode 100644
> index 00000000000..b874f2f061c
> --- /dev/null
> +++ b/src/gallium/state_trackers/clover/spirv/invocation.cpp
> @@ -0,0 +1,129 @@
> +//
> +// Copyright 2018 Pierre Moreau
> +//
> +// 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.
> +//
> +
> +#include "invocation.hpp"
> +
> +#ifdef CLOVER_ALLOW_SPIRV
> +#include <spirv-tools/libspirv.hpp>
> +#endif
> +
> +#include "util/u_math.h"
> +
> +#include "compiler/spirv/spirv.h"
> +
> +using namespace clover;
> +
> +namespace {
> +
> +#ifdef CLOVER_ALLOW_SPIRV
> + std::string
> + format_validator_msg(spv_message_level_t level, const char * /* source */,
> + const spv_position_t &position, const char *message) {
> + auto const level_to_string = [](spv_message_level_t level){
"const auto" would be more idiosyncratic.
> + switch (level) {
> + case SPV_MSG_FATAL:
> + return std::string("Fatal");
> + case SPV_MSG_INTERNAL_ERROR:
> + return std::string("Internal error");
> + case SPV_MSG_ERROR:
> + return std::string("Error");
> + case SPV_MSG_WARNING:
> + return std::string("Warning");
> + case SPV_MSG_INFO:
> + return std::string("Info");
> + case SPV_MSG_DEBUG:
> + return std::string("Debug");
> + }
> + return std::string();
> + };
You could just use a 'level == X ? "Y" : ...' ladder here instead since
the 'level' argument is already known at the point the level_to_string
function is defined.
> + return "[" + level_to_string(level) + "] At word No." +
> + std::to_string(position.index) + ": \"" + message + "\"\n";
> + }
> +
> + spv_target_env
> + convert_opencl_str_to_target_env(const std::string &opencl_version) {
> + if (opencl_version == "2.2") {
> + return SPV_ENV_OPENCL_2_2;
> + } else if (opencl_version == "2.1") {
> + return SPV_ENV_OPENCL_2_1;
> + } else if (opencl_version == "2.0") {
> + return SPV_ENV_OPENCL_2_0;
> + } else if (opencl_version == "1.2" ||
> + opencl_version == "1.1" ||
> + opencl_version == "1.0") {
> + // SPIR-V is only defined for OpenCL >= 1.2, however some drivers
> + // might use it with OpenCL 1.0 and 1.1.
> + return SPV_ENV_OPENCL_1_2;
> + } else {
> + throw build_error("Invalid OpenCL version");
> + }
> + }
> +#endif
> +
> +}
> +
> +bool
> +clover::spirv::is_binary_spirv(const char *il, size_t length)
> +{
> + const uint32_t *binary = reinterpret_cast<const uint32_t*>(il);
> +
> + // A SPIR-V binary is at the very least 5 32-bit words, which represent the
> + // SPIR-V header.
> + if (length < 20u)
> + return false;
> +
> + const uint32_t first_word = binary[0u];
> + return (first_word == SpvMagicNumber) ||
> + (util_bswap32(first_word) == SpvMagicNumber);
> +}
> +
This function seems like dead code. Maybe drop it? Then you'll be able
to use a single #ifdef preprocessor conditional to guard the whole file.
> +#ifdef CLOVER_ALLOW_SPIRV
> +bool
> +clover::spirv::is_valid_spirv(const uint32_t *binary, size_t length,
> + const std::string &opencl_version,
> + const context::notify_action ¬ify) {
We don't need an extra level of call-backs here, you can pass a
'std::string &r_log' reference and append the errors there, just like
the LLVM codepaths do too return error strings.
Other than that looks good:
Reviewed-by: Francisco Jerez <currojerez at riseup.net>
> + auto const validator_consumer =
> + [¬ify](spv_message_level_t level, const char *source,
> + const spv_position_t &position, const char *message) {
> + if (!notify)
> + return;
> +
> + const std::string log = format_validator_msg(level, source, position,
> + message);
> + notify(log.c_str());
> + };
> +
> + const spv_target_env target_env =
> + convert_opencl_str_to_target_env(opencl_version);
> + spvtools::SpirvTools spvTool(target_env);
> + spvTool.SetMessageConsumer(validator_consumer);
> +
> + return spvTool.Validate(binary, length);
> +}
> +#else
> +bool
> +clover::spirv::is_valid_spirv(const uint32_t * /*binary*/, size_t /*length*/,
> + const std::string &/*opencl_version*/,
> + const context::notify_action &/*notify*/) {
> + return false;
> +}
> +#endif
> diff --git a/src/gallium/state_trackers/clover/spirv/invocation.hpp b/src/gallium/state_trackers/clover/spirv/invocation.hpp
> new file mode 100644
> index 00000000000..92255a68a8e
> --- /dev/null
> +++ b/src/gallium/state_trackers/clover/spirv/invocation.hpp
> @@ -0,0 +1,47 @@
> +//
> +// Copyright 2018 Pierre Moreau
> +//
> +// 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_SPIRV_INVOCATION_HPP
> +#define CLOVER_SPIRV_INVOCATION_HPP
> +
> +#include "core/context.hpp"
> +
> +namespace clover {
> + namespace spirv {
> + // Returns whether the binary starts with the SPIR-V magic word.
> + //
> + // The first word is interpreted as little endian and big endian, but
> + // only one of them has to match.
> + bool is_binary_spirv(const char *binary, size_t length);
> +
> + // Returns whether the given binary is considered valid for the given
> + // OpenCL version.
> + //
> + // It uses SPIRV-Tools validator to do the validation, and potential
> + // warnings and errors are forwarded to the application via |notify|.
> + bool is_valid_spirv(const uint32_t *binary, size_t length,
> + const std::string &opencl_version,
> + const context::notify_action ¬ify);
> + }
> +}
> +
> +#endif
> --
> 2.21.0
>
> _______________________________________________
> 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: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190511/0f0d521e/attachment.sig>
More information about the mesa-dev
mailing list