In order to validate SPIR-V code we have to fix a little bug that causes us to emit invalid SPIR-V code in some cases. That's technically a bug, so it should be good to fix it in 1.10; unfortunately the way the bug is fixed right now is not ideal, because in some cases it causes many push constants to be wasted, potentially reaching the Vulkan implementation limit, so some application that used to work might stop working. Eventually we should implement a better allocator for push constants, and include an alternative path if the Vulkan implementation doesn't offer enough push constants.
From: Giovanni Mascellani gmascellani@codeweavers.com
Because that's the granularity of Constant Buffer accesses in SM4. This commit requires using more push constants, but without it the generated SPIR-V can be invalid. --- libs/vkd3d/state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libs/vkd3d/state.c b/libs/vkd3d/state.c index 6665a1fd6..148d678bf 100644 --- a/libs/vkd3d/state.c +++ b/libs/vkd3d/state.c @@ -515,7 +515,7 @@ static HRESULT d3d12_root_signature_init_push_constants(struct d3d12_root_signat assert(p->ShaderVisibility <= D3D12_SHADER_VISIBILITY_PIXEL); push_constants[p->ShaderVisibility].stageFlags = use_vk_heaps ? VK_SHADER_STAGE_ALL : stage_flags_from_visibility(p->ShaderVisibility); - push_constants[p->ShaderVisibility].size += p->u.Constants.Num32BitValues * sizeof(uint32_t); + push_constants[p->ShaderVisibility].size += align(p->u.Constants.Num32BitValues, 4) * sizeof(uint32_t); } if (push_constants[D3D12_SHADER_VISIBILITY_ALL].size) { @@ -564,7 +564,7 @@ static HRESULT d3d12_root_signature_init_push_constants(struct d3d12_root_signat
idx = push_constant_count == 1 ? 0 : p->ShaderVisibility; offset = push_constants_offset[idx]; - push_constants_offset[idx] += p->u.Constants.Num32BitValues * sizeof(uint32_t); + push_constants_offset[idx] += align(p->u.Constants.Num32BitValues, 4) * sizeof(uint32_t);
root_signature->parameters[i].parameter_type = p->ParameterType; root_constant->stage_flags = push_constant_count == 1
From: Giovanni Mascellani gmascellani@codeweavers.com
--- libs/vkd3d-shader/spirv.c | 264 +++++++++++++++++++------------------- 1 file changed, 133 insertions(+), 131 deletions(-)
diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index be149a0cf..71bc7538d 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -36,137 +36,7 @@
#ifdef HAVE_SPIRV_TOOLS # include "spirv-tools/libspirv.h" - -static spv_target_env spv_target_env_from_vkd3d(enum vkd3d_shader_spirv_environment environment) -{ - switch (environment) - { - case VKD3D_SHADER_SPIRV_ENVIRONMENT_OPENGL_4_5: - return SPV_ENV_OPENGL_4_5; - case VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0: - return SPV_ENV_VULKAN_1_0; - default: - ERR("Invalid environment %#x.\n", environment); - return SPV_ENV_VULKAN_1_0; - } -} - -static uint32_t get_binary_to_text_options(enum vkd3d_shader_compile_option_formatting_flags formatting) -{ - uint32_t out = 0; - unsigned int i; - - static const struct - { - enum vkd3d_shader_compile_option_formatting_flags vkd3d; - uint32_t spv; - bool invert; - } - valuemap[] = - { - {VKD3D_SHADER_COMPILE_OPTION_FORMATTING_COLOUR, SPV_BINARY_TO_TEXT_OPTION_COLOR }, - {VKD3D_SHADER_COMPILE_OPTION_FORMATTING_INDENT, SPV_BINARY_TO_TEXT_OPTION_INDENT }, - {VKD3D_SHADER_COMPILE_OPTION_FORMATTING_OFFSETS, SPV_BINARY_TO_TEXT_OPTION_SHOW_BYTE_OFFSET}, - {VKD3D_SHADER_COMPILE_OPTION_FORMATTING_HEADER, SPV_BINARY_TO_TEXT_OPTION_NO_HEADER, true}, - {VKD3D_SHADER_COMPILE_OPTION_FORMATTING_RAW_IDS, SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES, true}, - }; - - for (i = 0; i < ARRAY_SIZE(valuemap); ++i) - { - if (valuemap[i].invert == !(formatting & valuemap[i].vkd3d)) - out |= valuemap[i].spv; - } - - return out; -} - -static enum vkd3d_result vkd3d_spirv_binary_to_text(const struct vkd3d_shader_code *spirv, - enum vkd3d_shader_spirv_environment environment, - enum vkd3d_shader_compile_option_formatting_flags formatting, struct vkd3d_shader_code *out) -{ - spv_diagnostic diagnostic = NULL; - spv_text text = NULL; - spv_context context; - spv_result_t spvret; - enum vkd3d_result result = VKD3D_OK; - - context = spvContextCreate(spv_target_env_from_vkd3d(environment)); - - if (!(spvret = spvBinaryToText(context, spirv->code, spirv->size / sizeof(uint32_t), - get_binary_to_text_options(formatting), &text, &diagnostic))) - { - void *code = vkd3d_malloc(text->length); - if (code) - { - memcpy(code, text->str, text->length); - out->size = text->length; - out->code = code; - } - else - result = VKD3D_ERROR_OUT_OF_MEMORY; - } - else - { - FIXME("Failed to convert SPIR-V binary to text, ret %d.\n", spvret); - FIXME("Diagnostic message: %s.\n", debugstr_a(diagnostic->error)); - result = VKD3D_ERROR; - } - - spvTextDestroy(text); - spvDiagnosticDestroy(diagnostic); - spvContextDestroy(context); - - return result; -} - -static void vkd3d_spirv_dump(const struct vkd3d_shader_code *spirv, - enum vkd3d_shader_spirv_environment environment) -{ - static const enum vkd3d_shader_compile_option_formatting_flags formatting - = VKD3D_SHADER_COMPILE_OPTION_FORMATTING_INDENT | VKD3D_SHADER_COMPILE_OPTION_FORMATTING_HEADER; - struct vkd3d_shader_code text; - - if (!vkd3d_spirv_binary_to_text(spirv, environment, formatting, &text)) - { - vkd3d_shader_trace_text(text.code, text.size); - vkd3d_shader_free_shader_code(&text); - } -} - -static void vkd3d_spirv_validate(const struct vkd3d_shader_code *spirv, - enum vkd3d_shader_spirv_environment environment) -{ - spv_diagnostic diagnostic = NULL; - spv_context context; - spv_result_t ret; - - context = spvContextCreate(spv_target_env_from_vkd3d(environment)); - - if ((ret = spvValidateBinary(context, spirv->code, spirv->size / sizeof(uint32_t), - &diagnostic))) - { - FIXME("Failed to validate SPIR-V binary, ret %d.\n", ret); - FIXME("Diagnostic message: %s.\n", debugstr_a(diagnostic->error)); - } - - spvDiagnosticDestroy(diagnostic); - spvContextDestroy(context); -} - -#else - -static enum vkd3d_result vkd3d_spirv_binary_to_text(const struct vkd3d_shader_code *spirv, - enum vkd3d_shader_spirv_environment environment, - enum vkd3d_shader_compile_option_formatting_flags formatting, struct vkd3d_shader_code *out) -{ - return VKD3D_ERROR; -} -static void vkd3d_spirv_dump(const struct vkd3d_shader_code *spirv, - enum vkd3d_shader_spirv_environment environment) {} -static void vkd3d_spirv_validate(const struct vkd3d_shader_code *spirv, - enum vkd3d_shader_spirv_environment environment) {} - -#endif /* HAVE_SPIRV_TOOLS */ +#endif
enum vkd3d_shader_input_sysval_semantic vkd3d_siv_from_sysval_indexed(enum vkd3d_shader_sysval_semantic sysval, unsigned int index) @@ -2743,6 +2613,138 @@ static void VKD3D_PRINTF_FUNC(3, 4) spirv_compiler_warning(struct spirv_compiler va_end(args); }
+#ifdef HAVE_SPIRV_TOOLS +static spv_target_env spv_target_env_from_vkd3d(enum vkd3d_shader_spirv_environment environment) +{ + switch (environment) + { + case VKD3D_SHADER_SPIRV_ENVIRONMENT_OPENGL_4_5: + return SPV_ENV_OPENGL_4_5; + case VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0: + return SPV_ENV_VULKAN_1_0; + default: + ERR("Invalid environment %#x.\n", environment); + return SPV_ENV_VULKAN_1_0; + } +} + +static uint32_t get_binary_to_text_options(enum vkd3d_shader_compile_option_formatting_flags formatting) +{ + uint32_t out = 0; + unsigned int i; + + static const struct + { + enum vkd3d_shader_compile_option_formatting_flags vkd3d; + uint32_t spv; + bool invert; + } + valuemap[] = + { + {VKD3D_SHADER_COMPILE_OPTION_FORMATTING_COLOUR, SPV_BINARY_TO_TEXT_OPTION_COLOR }, + {VKD3D_SHADER_COMPILE_OPTION_FORMATTING_INDENT, SPV_BINARY_TO_TEXT_OPTION_INDENT }, + {VKD3D_SHADER_COMPILE_OPTION_FORMATTING_OFFSETS, SPV_BINARY_TO_TEXT_OPTION_SHOW_BYTE_OFFSET}, + {VKD3D_SHADER_COMPILE_OPTION_FORMATTING_HEADER, SPV_BINARY_TO_TEXT_OPTION_NO_HEADER, true}, + {VKD3D_SHADER_COMPILE_OPTION_FORMATTING_RAW_IDS, SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES, true}, + }; + + for (i = 0; i < ARRAY_SIZE(valuemap); ++i) + { + if (valuemap[i].invert == !(formatting & valuemap[i].vkd3d)) + out |= valuemap[i].spv; + } + + return out; +} + +static enum vkd3d_result vkd3d_spirv_binary_to_text(const struct vkd3d_shader_code *spirv, + enum vkd3d_shader_spirv_environment environment, + enum vkd3d_shader_compile_option_formatting_flags formatting, struct vkd3d_shader_code *out) +{ + spv_diagnostic diagnostic = NULL; + spv_text text = NULL; + spv_context context; + spv_result_t spvret; + enum vkd3d_result result = VKD3D_OK; + + context = spvContextCreate(spv_target_env_from_vkd3d(environment)); + + if (!(spvret = spvBinaryToText(context, spirv->code, spirv->size / sizeof(uint32_t), + get_binary_to_text_options(formatting), &text, &diagnostic))) + { + void *code = vkd3d_malloc(text->length); + if (code) + { + memcpy(code, text->str, text->length); + out->size = text->length; + out->code = code; + } + else + result = VKD3D_ERROR_OUT_OF_MEMORY; + } + else + { + FIXME("Failed to convert SPIR-V binary to text, ret %d.\n", spvret); + FIXME("Diagnostic message: %s.\n", debugstr_a(diagnostic->error)); + result = VKD3D_ERROR; + } + + spvTextDestroy(text); + spvDiagnosticDestroy(diagnostic); + spvContextDestroy(context); + + return result; +} + +static void vkd3d_spirv_dump(const struct vkd3d_shader_code *spirv, + enum vkd3d_shader_spirv_environment environment) +{ + static const enum vkd3d_shader_compile_option_formatting_flags formatting + = VKD3D_SHADER_COMPILE_OPTION_FORMATTING_INDENT | VKD3D_SHADER_COMPILE_OPTION_FORMATTING_HEADER; + struct vkd3d_shader_code text; + + if (!vkd3d_spirv_binary_to_text(spirv, environment, formatting, &text)) + { + vkd3d_shader_trace_text(text.code, text.size); + vkd3d_shader_free_shader_code(&text); + } +} + +static void vkd3d_spirv_validate(const struct vkd3d_shader_code *spirv, + enum vkd3d_shader_spirv_environment environment) +{ + spv_diagnostic diagnostic = NULL; + spv_context context; + spv_result_t ret; + + context = spvContextCreate(spv_target_env_from_vkd3d(environment)); + + if ((ret = spvValidateBinary(context, spirv->code, spirv->size / sizeof(uint32_t), + &diagnostic))) + { + FIXME("Failed to validate SPIR-V binary, ret %d.\n", ret); + FIXME("Diagnostic message: %s.\n", debugstr_a(diagnostic->error)); + } + + spvDiagnosticDestroy(diagnostic); + spvContextDestroy(context); +} + +#else + +static enum vkd3d_result vkd3d_spirv_binary_to_text(const struct vkd3d_shader_code *spirv, + enum vkd3d_shader_spirv_environment environment, + enum vkd3d_shader_compile_option_formatting_flags formatting, struct vkd3d_shader_code *out) +{ + return VKD3D_ERROR; +} +static void vkd3d_spirv_dump(const struct vkd3d_shader_code *spirv, + enum vkd3d_shader_spirv_environment environment) {} +static void vkd3d_spirv_validate(const struct vkd3d_shader_code *spirv, + enum vkd3d_shader_spirv_environment environment) {} + +#endif /* HAVE_SPIRV_TOOLS */ + static struct vkd3d_string_buffer *vkd3d_shader_register_range_string(struct spirv_compiler *compiler, const struct vkd3d_shader_register_range *range) {
From: Giovanni Mascellani gmascellani@codeweavers.com
--- gitlab/build-linux | 2 +- gitlab/build.yml | 4 ++-- gitlab/image.docker | 31 ++++++++++++++++++++++++++++++- 3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/gitlab/build-linux b/gitlab/build-linux index 4a8c793b7..d7e7573f1 100755 --- a/gitlab/build-linux +++ b/gitlab/build-linux @@ -12,7 +12,7 @@ rm -fr build mkdir build cd build export LD_LIBRARY_PATH=/usr/local/lib -if ../configure --enable-demos DXCOMPILER_LIBS="-L/usr/local/lib" CFLAGS="-g -O2 -Werror" CPPFLAGS="-DVKD3D_SHADER_UNSUPPORTED_DXIL" && \ +if ../configure --enable-demos --with-spirv-tools DXCOMPILER_LIBS="-L/usr/local/lib" CFLAGS="-g -O2 -Werror" CPPFLAGS="-DVKD3D_SHADER_UNSUPPORTED_DXIL" && \ make -j$(nproc) ; then make -j$(nproc) check || \ touch ../tests_failed diff --git a/gitlab/build.yml b/gitlab/build.yml index 9695a3042..9a19316b5 100644 --- a/gitlab/build.yml +++ b/gitlab/build.yml @@ -48,7 +48,7 @@ build-radv-32: - amd-gpu variables: VK_LOADER_DRIVERS_SELECT: 'radeon_*' - CC: 'gcc -m32' + CC: 'i686-linux-gnu-gcc' VKD3D_SHADER_CONFIG: 'force_validation'
build-llvmpipe-32: @@ -58,7 +58,7 @@ build-llvmpipe-32: - 2 variables: VK_LOADER_DRIVERS_SELECT: 'lvp_*' - CC: 'gcc -m32' + CC: 'i686-linux-gnu-gcc' VKD3D_SHADER_CONFIG: 'force_validation'
build-crosstest: diff --git a/gitlab/image.docker b/gitlab/image.docker index 9b7b17b09..270c509ba 100644 --- a/gitlab/image.docker +++ b/gitlab/image.docker @@ -2,6 +2,12 @@
WORKDIR /tmp
+# Package spirv-tools from Debian has two problems for us: first, it +# doesn't have shared libraries; second, it's not multiarch. So we +# have to rebuild it with some tweaks. In order to make it multiarch +# we also have to drop the executables, but we don't care about those +# anyway. + RUN export DEBIAN_FRONTEND=noninteractive; \ echo 'path-exclude=/usr/share/doc/*' > /etc/dpkg/dpkg.cfg.d/99-exclude-cruft && \ echo 'path-exclude=/usr/share/locale/*' >> /etc/dpkg/dpkg.cfg.d/99-exclude-cruft && \ @@ -11,9 +17,10 @@ RUN export DEBIAN_FRONTEND=noninteractive; \ echo 'exit 101' >> /usr/sbin/policy-rc.d && \ chmod +x /usr/sbin/policy-rc.d && \ dpkg --add-architecture i386 && \ + sed -i -e 's|Types: deb|Types: deb deb-src|g' /etc/apt/sources.list.d/* && \ apt-get update && \ apt-get dist-upgrade -y && \ - apt-get install -y build-essential pkg-config gcc-multilib gcc-mingw-w64 \ + apt-get install -y build-essential pkg-config gcc-mingw-w64 crossbuild-essential-i386 \ autoconf automake libtool flex bison curl \ git ca-certificates rsync \ doxygen doxygen-latex graphviz \ @@ -27,6 +34,8 @@ RUN export DEBIAN_FRONTEND=noninteractive; \ libxcb-util-dev libxcb-util-dev:i386 \ libxcb-icccm4-dev libxcb-icccm4-dev:i386 \ libxcb-keysyms1-dev libxcb-keysyms1-dev:i386 && \ + apt-get build-dep -y spirv-tools && \ + apt-get build-dep -y -ai386 spirv-tools && \ git clone --depth 1 --branch wine-3.21 https://gitlab.winehq.org/wine/wine.git && \ cd wine && \ mkdir build && \ @@ -36,6 +45,26 @@ RUN export DEBIAN_FRONTEND=noninteractive; \ cp tools/widl/widl /usr/local/bin && \ cd ../.. && \ rm -rf wine && \ + apt-get source spirv-tools && \ + cd spirv-tools-* && \ + sed -i -e 's|-DBUILD_SHARED_LIBS=OFF|-DBUILD_SHARED_LIBS=ON|g' debian/rules && \ + sed -i -e 's|dh_install$|dh_install && rm debian/spirv-tools/usr/bin/*|g' debian/rules && \ + sed -i '/Architecture: any/a Multi-Arch: same' debian/control && \ + dpkg-buildpackage -uc -us && \ + cd .. && \ + rm -f spirv-tools-dbgsym_*.deb && \ + dpkg -i spirv-tools_*.deb && \ + rm -fr * && \ + apt-get source spirv-tools && \ + cd spirv-tools-* && \ + sed -i -e 's|-DBUILD_SHARED_LIBS=OFF|-DBUILD_SHARED_LIBS=ON|g' debian/rules && \ + sed -i -e 's|dh_install$|dh_install && rm debian/spirv-tools/usr/bin/*|g' debian/rules && \ + sed -i '/Architecture: any/a Multi-Arch: same' debian/control && \ + CONFIG_SITE=/etc/dpkg-cross/cross-config.amd64 DEB_BUILD_OPTIONS=nocheck dpkg-buildpackage -ai386 -Pcross,nocheck -uc -us && \ + cd .. && \ + rm -f spirv-tools-dbgsym_*.deb && \ + dpkg -i spirv-tools_*.deb && \ + rm -fr * && \ apt-get clean && \ curl -L -s https://github.com/microsoft/DirectXShaderCompiler/releases/download/v1.7.23... | tar zx -C /usr/local ./lib/libdxcompiler.so ./lib/libdxil.so && \ groupadd host-render -g 800 && \
From: Giovanni Mascellani gmascellani@codeweavers.com
--- libs/vkd3d-shader/spirv.c | 45 +++++++++++++++++------- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 2 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 71bc7538d..daab75058 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -2252,6 +2252,8 @@ struct spirv_compiler
struct ssa_register_info *ssa_register_info; unsigned int ssa_register_count; + + uint64_t config_flags; };
static bool is_in_default_phase(const struct spirv_compiler *compiler) @@ -2307,7 +2309,8 @@ static void spirv_compiler_destroy(struct spirv_compiler *compiler) static struct spirv_compiler *spirv_compiler_create(const struct vkd3d_shader_version *shader_version, struct vkd3d_shader_desc *shader_desc, const struct vkd3d_shader_compile_info *compile_info, const struct vkd3d_shader_scan_descriptor_info1 *scan_descriptor_info, - struct vkd3d_shader_message_context *message_context, const struct vkd3d_shader_location *location) + struct vkd3d_shader_message_context *message_context, const struct vkd3d_shader_location *location, + bool config_flags) { const struct shader_signature *patch_constant_signature = &shader_desc->patch_constant_signature; const struct shader_signature *output_signature = &shader_desc->output_signature; @@ -2324,6 +2327,7 @@ static struct spirv_compiler *spirv_compiler_create(const struct vkd3d_shader_ve memset(compiler, 0, sizeof(*compiler)); compiler->message_context = message_context; compiler->location = *location; + compiler->config_flags = config_flags;
if ((target_info = vkd3d_find_struct(compile_info->next, SPIRV_TARGET_INFO))) { @@ -2710,24 +2714,35 @@ static void vkd3d_spirv_dump(const struct vkd3d_shader_code *spirv, } }
-static void vkd3d_spirv_validate(const struct vkd3d_shader_code *spirv, +static bool vkd3d_spirv_validate(struct spirv_compiler *compiler, const struct vkd3d_shader_code *spirv, enum vkd3d_shader_spirv_environment environment) { spv_diagnostic diagnostic = NULL; spv_context context; - spv_result_t ret; + spv_result_t res; + bool ret = true;
context = spvContextCreate(spv_target_env_from_vkd3d(environment));
- if ((ret = spvValidateBinary(context, spirv->code, spirv->size / sizeof(uint32_t), + if ((res = spvValidateBinary(context, spirv->code, spirv->size / sizeof(uint32_t), &diagnostic))) { - FIXME("Failed to validate SPIR-V binary, ret %d.\n", ret); - FIXME("Diagnostic message: %s.\n", debugstr_a(diagnostic->error)); + FIXME("Failed to validate SPIR-V binary, result %d.\n", res); + TRACE("Diagnostic message: %s.\n", debugstr_a(diagnostic->error)); + vkd3d_shader_trace_text(diagnostic->error, strlen(diagnostic->error)); + + if (compiler->config_flags & VKD3D_SHADER_CONFIG_FLAG_FORCE_VALIDATION) + { + spirv_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INVALID_SHADER, + "Execution generated an invalid shader:\n%s", diagnostic->error); + ret = false; + } }
spvDiagnosticDestroy(diagnostic); spvContextDestroy(context); + + return ret; }
#else @@ -2740,8 +2755,12 @@ static enum vkd3d_result vkd3d_spirv_binary_to_text(const struct vkd3d_shader_co } static void vkd3d_spirv_dump(const struct vkd3d_shader_code *spirv, enum vkd3d_shader_spirv_environment environment) {} -static void vkd3d_spirv_validate(const struct vkd3d_shader_code *spirv, - enum vkd3d_shader_spirv_environment environment) {} + +static bool vkd3d_spirv_validate(struct spirv_compiler *compiler, const struct vkd3d_shader_code *spirv, + enum vkd3d_shader_spirv_environment environment) +{ + return true; +}
#endif /* HAVE_SPIRV_TOOLS */
@@ -9870,11 +9889,13 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, if (!vkd3d_spirv_compile_module(builder, spirv, spirv_compiler_get_entry_point_name(compiler))) return VKD3D_ERROR;
- if (TRACE_ON()) + if (TRACE_ON() || parser->config_flags & VKD3D_SHADER_CONFIG_FLAG_FORCE_VALIDATION) { enum vkd3d_shader_spirv_environment environment = spirv_compiler_get_target_environment(compiler); - vkd3d_spirv_dump(spirv, environment); - vkd3d_spirv_validate(spirv, environment); + if (TRACE_ON()) + vkd3d_spirv_dump(spirv, environment); + if (!vkd3d_spirv_validate(compiler, spirv, environment)) + return VKD3D_ERROR_INVALID_SHADER; }
if (compiler->failed) @@ -9901,7 +9922,7 @@ int spirv_compile(struct vkd3d_shader_parser *parser, int ret;
if (!(spirv_compiler = spirv_compiler_create(&parser->shader_version, &parser->shader_desc, - compile_info, scan_descriptor_info, message_context, &parser->location))) + compile_info, scan_descriptor_info, message_context, &parser->location, parser->config_flags))) { ERR("Failed to create SPIR-V compiler.\n"); return VKD3D_ERROR; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index d3989672b..a0c29d86c 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -96,6 +96,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_SPV_INVALID_TYPE = 2006, VKD3D_SHADER_ERROR_SPV_INVALID_HANDLER = 2007, VKD3D_SHADER_ERROR_SPV_NOT_IMPLEMENTED = 2008, + VKD3D_SHADER_ERROR_SPV_INVALID_SHADER = 2009,
VKD3D_SHADER_WARNING_SPV_INVALID_SWIZZLE = 2300,
The pipeline failure is expected, because until this MR is accepted the Docker image still misses package `spirv-tools`.
-static void vkd3d_spirv_validate(const struct vkd3d_shader_code *spirv, +static bool vkd3d_spirv_validate(struct spirv_compiler *compiler, const struct vkd3d_shader_code *spirv, enum vkd3d_shader_spirv_environment environment) { spv_diagnostic diagnostic = NULL; spv_context context; - spv_result_t ret; + spv_result_t res; + bool ret = true; context = spvContextCreate(spv_target_env_from_vkd3d(environment)); - if ((ret = spvValidateBinary(context, spirv->code, spirv->size / sizeof(uint32_t), + if ((res = spvValidateBinary(context, spirv->code, spirv->size / sizeof(uint32_t), &diagnostic))) { - FIXME("Failed to validate SPIR-V binary, ret %d.\n", ret); - FIXME("Diagnostic message: %s.\n", debugstr_a(diagnostic->error)); + FIXME("Failed to validate SPIR-V binary, result %d.\n", res); + TRACE("Diagnostic message: %s.\n", debugstr_a(diagnostic->error)); + vkd3d_shader_trace_text(diagnostic->error, strlen(diagnostic->error)); + + if (compiler->config_flags & VKD3D_SHADER_CONFIG_FLAG_FORCE_VALIDATION) + { + spirv_compiler_error(compiler, VKD3D_SHADER_ERROR_SPV_INVALID_SHADER, + "Execution generated an invalid shader:\n%s", diagnostic->error); + ret = false; + } } spvDiagnosticDestroy(diagnostic); spvContextDestroy(context); + + return ret; }
Could we just vkd3d_string_buffer_printf() diagnostic->error here? It would allow the caller to decide what to do with the error, slightly reduce the amount of code inside #ifdef, and probably avoid moving a bunch of code around as well.
Can't comment on the last commit for the CI build, but the rest looks good, and I second Henri's comment.
-
Conor McCarthy (＠cmccarthy) -
Giovanni Mascellani
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)