From b629ca1c336725b1c897a19c4ac441d6cf73c9ce Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Tue, 21 Nov 2017 09:27:49 +0100 Subject: [PATCH] Fix complicated Phi case. A continue block might have used a temporary which existed only in the loop body. --- .../asm/frag/temporary-phi-hoisting.asm.frag | 26 +++++++ .../asm/frag/temporary-phi-hoisting.asm.frag | 75 +++++++++++++++++++ spirv_glsl.cpp | 20 +++++ spirv_glsl.hpp | 2 + 4 files changed, 123 insertions(+) create mode 100644 reference/shaders/asm/frag/temporary-phi-hoisting.asm.frag create mode 100644 shaders/asm/frag/temporary-phi-hoisting.asm.frag diff --git a/reference/shaders/asm/frag/temporary-phi-hoisting.asm.frag b/reference/shaders/asm/frag/temporary-phi-hoisting.asm.frag new file mode 100644 index 00000000..3917594d --- /dev/null +++ b/reference/shaders/asm/frag/temporary-phi-hoisting.asm.frag @@ -0,0 +1,26 @@ +#version 450 + +struct MyStruct +{ + vec4 color; +}; + +layout(std140) uniform MyStruct_CB +{ + MyStruct g_MyStruct[4]; +} _6; + +layout(location = 0) out vec4 _entryPointOutput; + +void main() +{ + vec3 _28; + _28 = vec3(0.0); + vec3 _29; + for (int _31 = 0; _31 < 4; _28 = _29, _31++) + { + _29 = _28 + _6.g_MyStruct[_31].color.xyz; + } + _entryPointOutput = vec4(_28, 1.0); +} + diff --git a/shaders/asm/frag/temporary-phi-hoisting.asm.frag b/shaders/asm/frag/temporary-phi-hoisting.asm.frag new file mode 100644 index 00000000..7cedcd58 --- /dev/null +++ b/shaders/asm/frag/temporary-phi-hoisting.asm.frag @@ -0,0 +1,75 @@ +; SPIR-V +; Version: 1.0 +; Generator: Khronos Glslang Reference Front End; 1 +; Bound: 87 +; Schema: 0 + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %main "main" %_entryPointOutput + OpExecutionMode %main OriginUpperLeft + OpSource HLSL 500 + OpName %main "main" + OpName %MyStruct "MyStruct" + OpMemberName %MyStruct 0 "color" + OpName %MyStruct_CB "MyStruct_CB" + OpMemberName %MyStruct_CB 0 "g_MyStruct" + OpName %_ "" + OpName %_entryPointOutput "@entryPointOutput" + OpMemberDecorate %MyStruct 0 Offset 0 + OpDecorate %_arr_MyStruct_uint_4 ArrayStride 16 + OpMemberDecorate %MyStruct_CB 0 Offset 0 + OpDecorate %MyStruct_CB Block + OpDecorate %_ DescriptorSet 0 + OpDecorate %_entryPointOutput Location 0 + %void = OpTypeVoid + %3 = OpTypeFunction %void + %float = OpTypeFloat 32 + %v4float = OpTypeVector %float 4 + %v3float = OpTypeVector %float 3 + %float_0 = OpConstant %float 0 + %15 = OpConstantComposite %v3float %float_0 %float_0 %float_0 + %int = OpTypeInt 32 1 + %int_0 = OpConstant %int 0 + %int_4 = OpConstant %int 4 + %bool = OpTypeBool + %MyStruct = OpTypeStruct %v4float + %uint = OpTypeInt 32 0 + %uint_4 = OpConstant %uint 4 +%_arr_MyStruct_uint_4 = OpTypeArray %MyStruct %uint_4 +%MyStruct_CB = OpTypeStruct %_arr_MyStruct_uint_4 +%_ptr_Uniform_MyStruct_CB = OpTypePointer Uniform %MyStruct_CB + %_ = OpVariable %_ptr_Uniform_MyStruct_CB Uniform +%_ptr_Uniform_v4float = OpTypePointer Uniform %v4float + %int_1 = OpConstant %int 1 + %float_1 = OpConstant %float 1 +%_ptr_Output_v4float = OpTypePointer Output %v4float +%_entryPointOutput = OpVariable %_ptr_Output_v4float Output + %main = OpFunction %void None %3 + %5 = OpLabel + OpBranch %64 + %64 = OpLabel + %85 = OpPhi %v3float %15 %5 %77 %66 + %86 = OpPhi %int %int_0 %5 %79 %66 + OpLoopMerge %65 %66 None + OpBranch %67 + %67 = OpLabel + %69 = OpSLessThan %bool %86 %int_4 + OpBranchConditional %69 %70 %65 + %70 = OpLabel + %72 = OpAccessChain %_ptr_Uniform_v4float %_ %int_0 %86 %int_0 + %73 = OpLoad %v4float %72 + %74 = OpVectorShuffle %v3float %73 %73 0 1 2 + %77 = OpFAdd %v3float %85 %74 + OpBranch %66 + %66 = OpLabel + %79 = OpIAdd %int %86 %int_1 + OpBranch %64 + %65 = OpLabel + %81 = OpCompositeExtract %float %85 0 + %82 = OpCompositeExtract %float %85 1 + %83 = OpCompositeExtract %float %85 2 + %84 = OpCompositeConstruct %v4float %81 %82 %83 %float_1 + OpStore %_entryPointOutput %84 + OpReturn + OpFunctionEnd diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index a0472830..1450ba3e 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -2722,6 +2722,11 @@ string CompilerGLSL::declare_temporary(uint32_t result_type, uint32_t result_id) return join(to_name(result_id), " = "); } + else if (hoisted_temporaries.count(result_id)) + { + // The temporary has already been declared earlier, so just "declare" the temporary by writing to it. + return join(to_name(result_id), " = "); + } else { // The result_id has not been made into an expression yet, so use flags interface. @@ -7246,6 +7251,21 @@ void CompilerGLSL::flush_phi(uint32_t from, uint32_t to) { flush_variable_declaration(phi.function_variable); + // We tried to write an ID which does not yet exist, + // this can happen if we are a continue block, and the value we are trying to write + // only exists inside the loop body. + // In this case we must hoist out the temporary to the same scope as the phi variable, + // declare it at the outer scope (using child.declare_temporary), + // force the temporary to be committed to a value when it is computed (forced_temporaries), + // and declare that variable without type information (hoisted_temporaries). + if (ids[phi.local_variable].empty() && !hoisted_temporaries.count(phi.local_variable)) + { + force_recompile = true; + forced_temporaries.insert(phi.local_variable); + hoisted_temporaries.insert(phi.local_variable); + child.declare_temporary.emplace_back(var.basetype, phi.local_variable); + } + // This might be called in continue block, so make sure we // use this to emit ESSL 1.0 compliant increments/decrements. auto lhs = to_expression(phi.function_variable); diff --git a/spirv_glsl.hpp b/spirv_glsl.hpp index a7ced9db..e4b2af41 100644 --- a/spirv_glsl.hpp +++ b/spirv_glsl.hpp @@ -494,6 +494,8 @@ protected: bool can_use_io_location(spv::StorageClass storage); + std::unordered_set hoisted_temporaries; + private: void init() {