From 35e92e6ffb1a762700190fbf1ca55bc2b1debaff Mon Sep 17 00:00:00 2001 From: Bill Hollings Date: Sun, 12 Sep 2021 16:28:21 -0400 Subject: [PATCH] MSL: Return fragment function value even when last SPIR-V Op is discard (OpKill). Add test shader for new functionality. Add legacy test reference shader for unrelated buffer-bitcast test, that doesn't seem to have been added previously. --- ...return-value-after-discard-terminator.frag | 25 ++++++++++++++++++ ...itcast-uvec2-2.nocompat.invalid.vk.comp.vk | 22 ++++++++++++++++ ...return-value-after-discard-terminator.frag | 26 +++++++++++++++++++ ...return-value-after-discard-terminator.frag | 17 ++++++++++++ spirv_glsl.cpp | 3 +++ spirv_msl.cpp | 3 ++- 6 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 reference/opt/shaders-msl/frag/return-value-after-discard-terminator.frag create mode 100644 reference/opt/shaders/vulkan/comp/buffer-reference-bitcast-uvec2-2.nocompat.invalid.vk.comp.vk create mode 100644 reference/shaders-msl/frag/return-value-after-discard-terminator.frag create mode 100644 shaders-msl/frag/return-value-after-discard-terminator.frag diff --git a/reference/opt/shaders-msl/frag/return-value-after-discard-terminator.frag b/reference/opt/shaders-msl/frag/return-value-after-discard-terminator.frag new file mode 100644 index 00000000..92097dfa --- /dev/null +++ b/reference/opt/shaders-msl/frag/return-value-after-discard-terminator.frag @@ -0,0 +1,25 @@ +#include +#include + +using namespace metal; + +struct buff_t +{ + int m0[1024]; +}; + +struct main0_out +{ + float4 frag_clr [[color(0)]]; +}; + +fragment main0_out main0(device buff_t& buff [[buffer(0)]], float4 gl_FragCoord [[position]]) +{ + main0_out out = {}; + int4 _16 = int4(gl_FragCoord); + out.frag_clr = float4(0.0, 0.0, 1.0, 1.0); + buff.m0[(_16.y * 32) + _16.x] = 1; + discard_fragment(); + return out; +} + diff --git a/reference/opt/shaders/vulkan/comp/buffer-reference-bitcast-uvec2-2.nocompat.invalid.vk.comp.vk b/reference/opt/shaders/vulkan/comp/buffer-reference-bitcast-uvec2-2.nocompat.invalid.vk.comp.vk new file mode 100644 index 00000000..53c3d21c --- /dev/null +++ b/reference/opt/shaders/vulkan/comp/buffer-reference-bitcast-uvec2-2.nocompat.invalid.vk.comp.vk @@ -0,0 +1,22 @@ +#version 450 +#extension GL_EXT_buffer_reference : require +#extension GL_EXT_buffer_reference_uvec2 : require +layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in; + +layout(buffer_reference) buffer PtrInt; +layout(buffer_reference, std430) buffer PtrInt +{ + int value; +}; + +layout(set = 0, binding = 0, std430) buffer Buf +{ + uvec2 ptr; + PtrInt ptrint; +} _13; + +void main() +{ + _13.ptr = uvec2(_13.ptrint); +} + diff --git a/reference/shaders-msl/frag/return-value-after-discard-terminator.frag b/reference/shaders-msl/frag/return-value-after-discard-terminator.frag new file mode 100644 index 00000000..d8895e0e --- /dev/null +++ b/reference/shaders-msl/frag/return-value-after-discard-terminator.frag @@ -0,0 +1,26 @@ +#include +#include + +using namespace metal; + +struct buff_t +{ + int m0[1024]; +}; + +struct main0_out +{ + float4 frag_clr [[color(0)]]; +}; + +fragment main0_out main0(device buff_t& buff [[buffer(0)]], float4 gl_FragCoord [[position]]) +{ + main0_out out = {}; + int2 frag_coord = int2(int4(gl_FragCoord).xy); + int buff_idx = (frag_coord.y * 32) + frag_coord.x; + out.frag_clr = float4(0.0, 0.0, 1.0, 1.0); + buff.m0[buff_idx] = 1; + discard_fragment(); + return out; +} + diff --git a/shaders-msl/frag/return-value-after-discard-terminator.frag b/shaders-msl/frag/return-value-after-discard-terminator.frag new file mode 100644 index 00000000..2ab410cb --- /dev/null +++ b/shaders-msl/frag/return-value-after-discard-terminator.frag @@ -0,0 +1,17 @@ +#version 450 + +layout(set = 0, binding = 0, std430) buffer buff_t +{ + int m0[1024]; +} buff; + +layout(location = 0) out vec4 frag_clr; + +void main() +{ + ivec2 frag_coord = ivec2(ivec4(gl_FragCoord).xy); + int buff_idx = (frag_coord.y * 32) + frag_coord.x; + frag_clr = vec4(0.0, 0.0, 1.0, 1.0); + buff.m0[buff_idx] = 1; + discard; +} diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 4f28e6b5..2b19346d 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -15084,8 +15084,11 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block) break; } + // If the Kill is terminating a block with a (probably synthetic) return value, emit a return value statement. case SPIRBlock::Kill: statement(backend.discard_literal, ";"); + if (block.return_value) + statement("return ", to_expression(block.return_value), ";"); break; case SPIRBlock::Unreachable: diff --git a/spirv_msl.cpp b/spirv_msl.cpp index d20f1643..06e27e59 100644 --- a/spirv_msl.cpp +++ b/spirv_msl.cpp @@ -3467,6 +3467,7 @@ uint32_t CompilerMSL::add_interface_block(StorageClass storage, bool patch) // Add the output interface struct as a local variable to the entry function. // If the entry point should return the output struct, set the entry function // to return the output interface struct, otherwise to return nothing. + // Watch out for the rare case where the terminator of the last entry point block is a Kill, instead of a Return. // Indicate the output var requires early initialization. bool ep_should_return_output = !get_is_rasterization_disabled(); uint32_t rtn_id = ep_should_return_output ? ib_var_id : 0; @@ -3476,7 +3477,7 @@ uint32_t CompilerMSL::add_interface_block(StorageClass storage, bool patch) for (auto &blk_id : entry_func.blocks) { auto &blk = get(blk_id); - if (blk.terminator == SPIRBlock::Return) + if (blk.terminator == SPIRBlock::Return || (blk.terminator == SPIRBlock::Kill && blk_id == entry_func.blocks.back())) blk.return_value = rtn_id; } vars_needing_early_declaration.push_back(ib_var_id);