From ebb5098def15c879125301451a47b08a16c76e92 Mon Sep 17 00:00:00 2001 From: Bill Hollings Date: Tue, 13 Jul 2021 21:22:13 -0400 Subject: [PATCH] MSL: Adjust gl_SampleMaskIn for sample-shading and/or fixed sample mask. Vulkan specifies that the Sample Mask Test occurs before fragment shading. This means gl_SampleMaskIn should be influenced by both sample-shading and VkPipelineMultisampleStateCreateInfo::pSampleMask. CTS tests dEQP-VK.pipeline.multisample_shader_builtin.* bear this out. For sample-shading, gl_SampleMaskIn should only have a single bit set, Since Metal does not filter for this, apply a bitmask based on gl_SampleID. For a fixed sample mask, since Metal is unaware of VkPipelineMultisampleStateCreateInfo::pSampleMask, we need to ensure that we apply it to both gl_SampleMaskIn and gl_SampleMask. This has the side effect of a redundant application of pSampleMask if the shader already includes gl_SampleMaskIn when setting gl_SampleMask, but I don't see an easy way around this. Also, simplify the logic for including the fixed sample mask in gl_ShaderMask, and print the fixed sample mask as a hex value for readability of bits. --- ...nd-out.fixed-sample-mask.force-sample.frag | 20 ++++++ ...ple-mask-in-and-out.fixed-sample-mask.frag | 4 +- ...ample-mask-not-used.fixed-sample-mask.frag | 2 +- .../frag/sample-mask.fixed-sample-mask.frag | 2 +- ...nd-out.fixed-sample-mask.force-sample.frag | 20 ++++++ ...ple-mask-in-and-out.fixed-sample-mask.frag | 4 +- ...ample-mask-not-used.fixed-sample-mask.frag | 2 +- .../frag/sample-mask.fixed-sample-mask.frag | 2 +- ...nd-out.fixed-sample-mask.force-sample.frag | 10 +++ spirv_msl.cpp | 65 +++++++++++-------- spirv_msl.hpp | 3 + 11 files changed, 100 insertions(+), 34 deletions(-) create mode 100644 reference/opt/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag create mode 100644 reference/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag create mode 100644 shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag diff --git a/reference/opt/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag b/reference/opt/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag new file mode 100644 index 00000000..626fe4c7 --- /dev/null +++ b/reference/opt/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag @@ -0,0 +1,20 @@ +#include +#include + +using namespace metal; + +struct main0_out +{ + float4 FragColor [[color(0)]]; + uint gl_SampleMask [[sample_mask]]; +}; + +fragment main0_out main0(uint gl_SampleMaskIn [[sample_mask]], uint gl_SampleID [[sample_id]]) +{ + main0_out out = {}; + out.FragColor = float4(1.0); + out.gl_SampleMask = (gl_SampleMaskIn & 0x22 & (1 << gl_SampleID)); + out.gl_SampleMask &= 0x22; + return out; +} + diff --git a/reference/opt/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.frag b/reference/opt/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.frag index 21ca7178..f478901b 100644 --- a/reference/opt/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.frag +++ b/reference/opt/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.frag @@ -13,8 +13,8 @@ fragment main0_out main0(uint gl_SampleMaskIn [[sample_mask]]) { main0_out out = {}; out.FragColor = float4(1.0); - out.gl_SampleMask = gl_SampleMaskIn; - out.gl_SampleMask &= 34; + out.gl_SampleMask = (gl_SampleMaskIn & 0x22); + out.gl_SampleMask &= 0x22; return out; } diff --git a/reference/opt/shaders-msl/frag/sample-mask-not-used.fixed-sample-mask.frag b/reference/opt/shaders-msl/frag/sample-mask-not-used.fixed-sample-mask.frag index 040c6414..d04f2033 100644 --- a/reference/opt/shaders-msl/frag/sample-mask-not-used.fixed-sample-mask.frag +++ b/reference/opt/shaders-msl/frag/sample-mask-not-used.fixed-sample-mask.frag @@ -13,7 +13,7 @@ fragment main0_out main0() { main0_out out = {}; out.FragColor = float4(1.0); - out.gl_SampleMask = 34; + out.gl_SampleMask = 0x22; return out; } diff --git a/reference/opt/shaders-msl/frag/sample-mask.fixed-sample-mask.frag b/reference/opt/shaders-msl/frag/sample-mask.fixed-sample-mask.frag index 20444779..76306b5a 100644 --- a/reference/opt/shaders-msl/frag/sample-mask.fixed-sample-mask.frag +++ b/reference/opt/shaders-msl/frag/sample-mask.fixed-sample-mask.frag @@ -14,7 +14,7 @@ fragment main0_out main0() main0_out out = {}; out.FragColor = float4(1.0); out.gl_SampleMask = 0; - out.gl_SampleMask &= 34; + out.gl_SampleMask &= 0x22; return out; } diff --git a/reference/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag b/reference/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag new file mode 100644 index 00000000..626fe4c7 --- /dev/null +++ b/reference/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag @@ -0,0 +1,20 @@ +#include +#include + +using namespace metal; + +struct main0_out +{ + float4 FragColor [[color(0)]]; + uint gl_SampleMask [[sample_mask]]; +}; + +fragment main0_out main0(uint gl_SampleMaskIn [[sample_mask]], uint gl_SampleID [[sample_id]]) +{ + main0_out out = {}; + out.FragColor = float4(1.0); + out.gl_SampleMask = (gl_SampleMaskIn & 0x22 & (1 << gl_SampleID)); + out.gl_SampleMask &= 0x22; + return out; +} + diff --git a/reference/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.frag b/reference/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.frag index 21ca7178..f478901b 100644 --- a/reference/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.frag +++ b/reference/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.frag @@ -13,8 +13,8 @@ fragment main0_out main0(uint gl_SampleMaskIn [[sample_mask]]) { main0_out out = {}; out.FragColor = float4(1.0); - out.gl_SampleMask = gl_SampleMaskIn; - out.gl_SampleMask &= 34; + out.gl_SampleMask = (gl_SampleMaskIn & 0x22); + out.gl_SampleMask &= 0x22; return out; } diff --git a/reference/shaders-msl/frag/sample-mask-not-used.fixed-sample-mask.frag b/reference/shaders-msl/frag/sample-mask-not-used.fixed-sample-mask.frag index 040c6414..d04f2033 100644 --- a/reference/shaders-msl/frag/sample-mask-not-used.fixed-sample-mask.frag +++ b/reference/shaders-msl/frag/sample-mask-not-used.fixed-sample-mask.frag @@ -13,7 +13,7 @@ fragment main0_out main0() { main0_out out = {}; out.FragColor = float4(1.0); - out.gl_SampleMask = 34; + out.gl_SampleMask = 0x22; return out; } diff --git a/reference/shaders-msl/frag/sample-mask.fixed-sample-mask.frag b/reference/shaders-msl/frag/sample-mask.fixed-sample-mask.frag index 20444779..76306b5a 100644 --- a/reference/shaders-msl/frag/sample-mask.fixed-sample-mask.frag +++ b/reference/shaders-msl/frag/sample-mask.fixed-sample-mask.frag @@ -14,7 +14,7 @@ fragment main0_out main0() main0_out out = {}; out.FragColor = float4(1.0); out.gl_SampleMask = 0; - out.gl_SampleMask &= 34; + out.gl_SampleMask &= 0x22; return out; } diff --git a/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag b/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag new file mode 100644 index 00000000..b78ee61e --- /dev/null +++ b/shaders-msl/frag/sample-mask-in-and-out.fixed-sample-mask.force-sample.frag @@ -0,0 +1,10 @@ +#version 450 + +layout(location = 0) out vec4 FragColor; + +void main() +{ + FragColor = vec4(1.0); + gl_SampleMask[0] = gl_SampleMaskIn[0]; +} + diff --git a/spirv_msl.cpp b/spirv_msl.cpp index abdbbbe8..33ea24dd 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -230,13 +230,12 @@ void CompilerMSL::build_implicit_builtins() (active_input_builtins.get(BuiltInVertexId) || active_input_builtins.get(BuiltInVertexIndex) || active_input_builtins.get(BuiltInBaseVertex) || active_input_builtins.get(BuiltInInstanceId) || active_input_builtins.get(BuiltInInstanceIndex) || active_input_builtins.get(BuiltInBaseInstance)); - bool need_sample_mask = msl_options.additional_fixed_sample_mask != 0xffffffff; bool need_local_invocation_index = msl_options.emulate_subgroups && active_input_builtins.get(BuiltInSubgroupId); bool need_workgroup_size = msl_options.emulate_subgroups && active_input_builtins.get(BuiltInNumSubgroups); if (need_subpass_input || need_sample_pos || need_subgroup_mask || need_vertex_params || need_tesc_params || need_multiview || need_dispatch_base || need_vertex_base_params || need_grid_params || needs_sample_id || - needs_subgroup_invocation_id || needs_subgroup_size || need_sample_mask || need_local_invocation_index || + needs_subgroup_invocation_id || needs_subgroup_size || has_additional_fixed_sample_mask() || need_local_invocation_index || need_workgroup_size) { bool has_frag_coord = false; @@ -267,7 +266,7 @@ void CompilerMSL::build_implicit_builtins() if (var.storage == StorageClassOutput) { - if (need_sample_mask && builtin == BuiltInSampleMask) + if (has_additional_fixed_sample_mask() && builtin == BuiltInSampleMask) { builtin_sample_mask_id = var.self; mark_implicit_builtin(StorageClassOutput, BuiltInSampleMask, var.self); @@ -757,7 +756,7 @@ void CompilerMSL::build_implicit_builtins() builtin_dispatch_base_id = var_id; } - if (need_sample_mask && !does_shader_write_sample_mask) + if (has_additional_fixed_sample_mask() && !does_shader_write_sample_mask) { uint32_t offset = ir.increase_bound_by(2); uint32_t var_id = offset + 1; @@ -12311,29 +12310,17 @@ void CompilerMSL::fix_up_shader_inputs_outputs() break; } } - else if (var.storage == StorageClassOutput && is_builtin_variable(var) && active_output_builtins.get(bi_type)) + else if (var.storage == StorageClassOutput && get_execution_model() == ExecutionModelFragment && + is_builtin_variable(var) && active_output_builtins.get(bi_type) && + bi_type == BuiltInSampleMask && has_additional_fixed_sample_mask()) { - if (bi_type == BuiltInSampleMask && get_execution_model() == ExecutionModelFragment && - msl_options.additional_fixed_sample_mask != 0xffffffff) - { - // If the additional fixed sample mask was set, we need to adjust the sample_mask - // output to reflect that. If the shader outputs the sample_mask itself too, we need - // to AND the two masks to get the final one. - if (does_shader_write_sample_mask) - { - entry_func.fixup_hooks_out.push_back([=]() { - statement(to_expression(builtin_sample_mask_id), - " &= ", msl_options.additional_fixed_sample_mask, ";"); - }); - } - else - { - entry_func.fixup_hooks_out.push_back([=]() { - statement(to_expression(builtin_sample_mask_id), " = ", - msl_options.additional_fixed_sample_mask, ";"); - }); - } - } + // If the additional fixed sample mask was set, we need to adjust the sample_mask + // output to reflect that. If the shader outputs the sample_mask itself too, we need + // to AND the two masks to get the final one. + string op_str = does_shader_write_sample_mask ? " &= " : " = "; + entry_func.fixup_hooks_out.push_back([=]() { + statement(to_expression(builtin_sample_mask_id), op_str, additional_fixed_sample_mask_str(), ";"); + }); } }); } @@ -14050,9 +14037,28 @@ string CompilerMSL::builtin_to_glsl(BuiltIn builtin, StorageClass storage) case BuiltInClipDistance: case BuiltInCullDistance: case BuiltInLayer: + if (get_execution_model() == ExecutionModelTessellationControl) + break; + if (storage != StorageClassInput && current_function && (current_function->self == ir.default_entry_point) && + !is_stage_output_builtin_masked(builtin)) + return stage_out_var_name + "." + CompilerGLSL::builtin_to_glsl(builtin, storage); + break; + case BuiltInSampleMask: if (get_execution_model() == ExecutionModelTessellationControl) break; + if (storage == StorageClassInput && current_function && (current_function->self == ir.default_entry_point) && + (has_additional_fixed_sample_mask() || needs_sample_id)) + { + string samp_mask_in; + samp_mask_in += "(" + CompilerGLSL::builtin_to_glsl(builtin, storage); + if (has_additional_fixed_sample_mask()) + samp_mask_in += " & " + additional_fixed_sample_mask_str(); + if (needs_sample_id) + samp_mask_in += " & (1 << gl_SampleID)"; + samp_mask_in += ")"; + return samp_mask_in; + } if (storage != StorageClassInput && current_function && (current_function->self == ir.default_entry_point) && !is_stage_output_builtin_masked(builtin)) return stage_out_var_name + "." + CompilerGLSL::builtin_to_glsl(builtin, storage); @@ -15936,3 +15942,10 @@ const char *CompilerMSL::get_combined_sampler_suffix() const void CompilerMSL::emit_block_hints(const SPIRBlock &) { } + +string CompilerMSL::additional_fixed_sample_mask_str() const +{ + char print_buffer[32]; + sprintf(print_buffer, "0x%x", msl_options.additional_fixed_sample_mask); + return print_buffer; +} diff --git a/spirv_msl.hpp b/spirv_msl.hpp index 55e99e48..19971272 100644 --- a/spirv_msl.hpp +++ b/spirv_msl.hpp @@ -1093,6 +1093,9 @@ protected: bool variable_storage_requires_stage_io(spv::StorageClass storage) const; + bool has_additional_fixed_sample_mask() const { return msl_options.additional_fixed_sample_mask != 0xffffffff; } + std::string additional_fixed_sample_mask_str() const; + // OpcodeHandler that handles several MSL preprocessing operations. struct OpCodePreprocessor : OpcodeHandler {