From 5b322555d6477e42dc4456b95b88b4ddbef3e003 Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Wed, 7 Jun 2023 14:49:42 +0200 Subject: [PATCH] GLSL: Fix bug with mixed precision on PHI variables. --- ...rnative-precision.asm..vk.nocompat.frag.vk | 29 +++++++++++ ...laxed-precision-inheritance-rules.asm.frag | 6 ++- ...lternative-precision.asm..vk.nocompat.frag | 51 +++++++++++++++++++ spirv_glsl.cpp | 27 +++++++++- 4 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 reference/shaders-no-opt/asm/frag/phi-alternative-precision.asm..vk.nocompat.frag.vk create mode 100644 shaders-no-opt/asm/frag/phi-alternative-precision.asm..vk.nocompat.frag diff --git a/reference/shaders-no-opt/asm/frag/phi-alternative-precision.asm..vk.nocompat.frag.vk b/reference/shaders-no-opt/asm/frag/phi-alternative-precision.asm..vk.nocompat.frag.vk new file mode 100644 index 00000000..ae9a8d69 --- /dev/null +++ b/reference/shaders-no-opt/asm/frag/phi-alternative-precision.asm..vk.nocompat.frag.vk @@ -0,0 +1,29 @@ +#version 450 + +layout(location = 0) in mediump float b; +layout(location = 0) out float FragColor; + +void main() +{ + float hp_copy_7; + mediump float _7; + int _22; + _22 = 0; + _7 = 0.0; + for (;;) + { + hp_copy_7 = _7; + int _23 = _22 + 1; + if (_23 < 4) + { + _22 = _23; + _7 = fma(_7, b, b); + } + else + { + break; + } + } + FragColor = hp_copy_7 * 4.0; +} + diff --git a/reference/shaders-no-opt/asm/frag/relaxed-precision-inheritance-rules.asm.frag b/reference/shaders-no-opt/asm/frag/relaxed-precision-inheritance-rules.asm.frag index b0b3a8db..5aef013e 100644 --- a/reference/shaders-no-opt/asm/frag/relaxed-precision-inheritance-rules.asm.frag +++ b/reference/shaders-no-opt/asm/frag/relaxed-precision-inheritance-rules.asm.frag @@ -16,6 +16,8 @@ layout(location = 0) in vec4 V4; void main() { + highp float hp_copy_phi_mp; + float mp_copy_phi_hp; vec4 V4_value0 = V4; highp vec4 hp_copy_V4_value0 = V4_value0; float V1_value0 = V4.x; @@ -46,8 +48,8 @@ void main() highp float phi_hp; phi_mp = _21; phi_hp = _49; - highp float hp_copy_phi_mp = phi_mp; - float mp_copy_phi_hp = phi_hp; + hp_copy_phi_mp = phi_mp; + mp_copy_phi_hp = phi_hp; FragColor2 = vec4(phi_mp + phi_mp, hp_copy_phi_mp + hp_copy_phi_mp, mp_copy_phi_hp + mp_copy_phi_hp, phi_hp + phi_hp); } diff --git a/shaders-no-opt/asm/frag/phi-alternative-precision.asm..vk.nocompat.frag b/shaders-no-opt/asm/frag/phi-alternative-precision.asm..vk.nocompat.frag new file mode 100644 index 00000000..12b47d23 --- /dev/null +++ b/shaders-no-opt/asm/frag/phi-alternative-precision.asm..vk.nocompat.frag @@ -0,0 +1,51 @@ +; SPIR-V +; Version: 1.0 +; Generator: Khronos Glslang Reference Front End; 11 +; Bound: 41 +; Schema: 0 + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %main "main" %b %FragColor + OpExecutionMode %main OriginUpperLeft + OpSource GLSL 450 + OpName %main "main" + OpName %b "b" + OpName %FragColor "FragColor" + OpDecorate %b RelaxedPrecision + OpDecorate %b Location 0 + OpDecorate %21 RelaxedPrecision + OpDecorate %24 RelaxedPrecision + OpDecorate %FragColor Location 0 + OpDecorate %39 RelaxedPrecision + %void = OpTypeVoid + %3 = OpTypeFunction %void + %float = OpTypeFloat 32 + %float_0 = OpConstant %float 0 + %int = OpTypeInt 32 1 + %int_0 = OpConstant %int 0 +%_ptr_Input_float = OpTypePointer Input %float + %b = OpVariable %_ptr_Input_float Input + %int_1 = OpConstant %int 1 + %int_4 = OpConstant %int 4 + %bool = OpTypeBool + %float_4 = OpConstant %float 4 +%_ptr_Output_float = OpTypePointer Output %float + %FragColor = OpVariable %_ptr_Output_float Output + %main = OpFunction %void None %3 + %5 = OpLabel + OpBranch %14 + %14 = OpLabel + %40 = OpPhi %int %int_0 %5 %27 %14 + %39 = OpPhi %float %float_0 %5 %24 %14 + %21 = OpLoad %float %b + %24 = OpExtInst %float %1 Fma %39 %21 %21 + %27 = OpIAdd %int %40 %int_1 + %31 = OpSLessThan %bool %27 %int_4 + OpLoopMerge %16 %14 None + OpBranchConditional %31 %14 %16 + %16 = OpLabel + %38 = OpFMul %float %39 %float_4 + OpStore %FragColor %38 + OpReturn + OpFunctionEnd diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 281d4faa..bcf48ae3 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -11197,8 +11197,15 @@ void CompilerGLSL::emit_block_instructions(SPIRBlock &block) if (backend.requires_relaxed_precision_analysis) { // If PHI variables are consumed in unexpected precision contexts, copy them here. - for (auto &phi : block.phi_variables) + for (size_t i = 0, n = block.phi_variables.size(); i < n; i++) { + auto &phi = block.phi_variables[i]; + + // Ensure we only copy once. We know a-priori that this array will lay out + // the same function variables together. + if (i && block.phi_variables[i - 1].function_variable == phi.function_variable) + continue; + auto itr = temporary_to_mirror_precision_alias.find(phi.function_variable); if (itr != temporary_to_mirror_precision_alias.end()) { @@ -16562,6 +16569,24 @@ void CompilerGLSL::emit_block_chain(SPIRBlock &block) if (block.merge == SPIRBlock::MergeLoop) add_loop_level(); + // If we're emitting PHI variables with precision aliases, we have to emit them as hoisted temporaries. + for (auto var_id : block.dominated_variables) + { + auto &var = get(var_id); + if (var.phi_variable) + { + auto mirrored_precision_itr = temporary_to_mirror_precision_alias.find(var_id); + if (mirrored_precision_itr != temporary_to_mirror_precision_alias.end() && + find_if(block.declare_temporary.begin(), block.declare_temporary.end(), + [mirrored_precision_itr](const std::pair &p) { + return p.second == mirrored_precision_itr->second; + }) == block.declare_temporary.end()) + { + block.declare_temporary.push_back({ var.basetype, mirrored_precision_itr->second }); + } + } + } + emit_hoisted_temporaries(block.declare_temporary); SPIRBlock::ContinueBlockType continue_type = SPIRBlock::ContinueNone;