From c80cbde7aaa4e53d2ebca671e29688cfd3ea0d19 Mon Sep 17 00:00:00 2001 From: comex Date: Wed, 25 Nov 2020 12:43:01 -0500 Subject: spirv_msl: Don't add fixup hooks for builtin variables if they're unused. This is necessary to avoid invalid output because of how implicit dependencies on builtins work. For example, the fixup for `BuiltInSubgroupEqMask` initializes the variable based on `builtin_subgroup_invocation_id_id`, a field storing the ID for a variable with decoration `BuiltInSubgroupLocalInvocationId`. This could be either a variable that already exists in the input (spirv_msl.cpp:300) or, if necessary, a newly created one (spirv_msl.cpp:621). In both cases, though, `builtin_subgroup_invocation_id_id` is only set under the condition `need_subgroup_mask || needs_subgroup_invocation_id`. `need_subgroup_mask` is true if any of the `BuiltInSubgroupXXMask` are set in `active_input_builtins`. Normally, if the program contains `BuiltInSubgroupEqMask`, `Compiler::ActiveBuiltinHandler` will set it in `active_input_builtins`. But this only happens if the variable is actually used, whereas `fix_up_shader_inputs_outputs` loops over all variables in the program regardless of whether they're used. If `BuiltInSubgroupEqMask` is not used, `builtin_subgroup_invocation_id_id` is never set, but before this patch the fixup hook would try to use it anyway, producing MSL that references a nonexistent variable named `_0`. Avoid this by changing `fix_up_shader_inputs_outputs` to skip builtins which are not set in `active_input_builtins` or `active_output_builtins`. And add a test case. --- .../shaders-msl-no-opt/vert/unused-subgroup-builtin.msl22.vert | 9 +++++++++ shaders-msl-no-opt/vert/unused-subgroup-builtin.msl22.vert | 7 +++++++ spirv_msl.cpp | 4 ++-- 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 reference/shaders-msl-no-opt/vert/unused-subgroup-builtin.msl22.vert create mode 100644 shaders-msl-no-opt/vert/unused-subgroup-builtin.msl22.vert diff --git a/reference/shaders-msl-no-opt/vert/unused-subgroup-builtin.msl22.vert b/reference/shaders-msl-no-opt/vert/unused-subgroup-builtin.msl22.vert new file mode 100644 index 00000000..9e024c20 --- /dev/null +++ b/reference/shaders-msl-no-opt/vert/unused-subgroup-builtin.msl22.vert @@ -0,0 +1,9 @@ +#include +#include + +using namespace metal; + +vertex void main0() +{ +} + diff --git a/shaders-msl-no-opt/vert/unused-subgroup-builtin.msl22.vert b/shaders-msl-no-opt/vert/unused-subgroup-builtin.msl22.vert new file mode 100644 index 00000000..4ec228df --- /dev/null +++ b/shaders-msl-no-opt/vert/unused-subgroup-builtin.msl22.vert @@ -0,0 +1,7 @@ +#version 450 +#extension GL_KHR_shader_subgroup_ballot : require + +void main() +{ + gl_SubgroupEqMask; +} diff --git a/spirv_msl.cpp b/spirv_msl.cpp index a74f62e0..a9ec650c 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -11052,7 +11052,7 @@ void CompilerMSL::fix_up_shader_inputs_outputs() uint32_t var_id = var.self; BuiltIn bi_type = ir.meta[var_id].decoration.builtin_type; - if (var.storage == StorageClassInput && is_builtin_variable(var)) + if (var.storage == StorageClassInput && is_builtin_variable(var) && active_input_builtins.get(bi_type)) { switch (bi_type) { @@ -11518,7 +11518,7 @@ void CompilerMSL::fix_up_shader_inputs_outputs() break; } } - else if (var.storage == StorageClassOutput && is_builtin_variable(var)) + else if (var.storage == StorageClassOutput && is_builtin_variable(var) && active_output_builtins.get(bi_type)) { if (bi_type == BuiltInSampleMask && get_execution_model() == ExecutionModelFragment && msl_options.additional_fixed_sample_mask != 0xffffffff) -- cgit v1.2.3