From 55c9d69ba03fb6b1a545354e5a011e324552bf15 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 25 Apr 2024 15:50:51 +0200 Subject: [PATCH] [naga] fix the way we adjust constant initializers when processing overrides This fixes 2 issues: - we used to index `adjusted_global_expressions` with the handle index of the constant instead of its initializer - we used to adjust the initializer multiple times if the arena contained multiple `Expression::Constant`s pointing to the same constant --- naga/src/back/pipeline_constants.rs | 6 +- .../in/spv/spec-constants-issue-5598.spv | Bin 0 -> 1432 bytes .../in/spv/spec-constants-issue-5598.spvasm | 96 ++++++++++++++++++ ...onstants-issue-5598.fragment.Fragment.glsl | 34 +++++++ ...ec-constants-issue-5598.vertex.Vertex.glsl | 34 +++++++ naga/tests/snapshots.rs | 1 + 6 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 naga/tests/in/spv/spec-constants-issue-5598.spv create mode 100644 naga/tests/in/spv/spec-constants-issue-5598.spvasm create mode 100644 naga/tests/out/glsl/spec-constants-issue-5598.fragment.Fragment.glsl create mode 100644 naga/tests/out/glsl/spec-constants-issue-5598.vertex.Vertex.glsl diff --git a/naga/src/back/pipeline_constants.rs b/naga/src/back/pipeline_constants.rs index f8a022c99a..0dbe9cf4e8 100644 --- a/naga/src/back/pipeline_constants.rs +++ b/naga/src/back/pipeline_constants.rs @@ -129,8 +129,10 @@ pub fn process_overrides<'a>( Expression::Constant(c_h) } Expression::Constant(c_h) => { - adjusted_constant_initializers.insert(c_h); - module.constants[c_h].init = adjusted_global_expressions[c_h.index()]; + if adjusted_constant_initializers.insert(c_h) { + let init = &mut module.constants[c_h].init; + *init = adjusted_global_expressions[init.index()]; + } expr } expr => expr, diff --git a/naga/tests/in/spv/spec-constants-issue-5598.spv b/naga/tests/in/spv/spec-constants-issue-5598.spv new file mode 100644 index 0000000000000000000000000000000000000000..2f32de970d217598e0aa7c5454f846058b38a85c GIT binary patch literal 1432 zcmZvbOHLI*6owBM5T9I-hcDDde4wBxDx%1}fF?|J2_{}0>3|U9kcpdd>D>1}QIzD5jUp8z9dSZBs28P=Qu?P8xs8h>AG`K|$%`Q|n}jV-Uf zdnIm6+qFqA?eky~{VcKq7vMRE)b<<|-(jSE^=rEyduxxe%Ms)>o>8RwN~v-G){T>6 zp9%b9$O^a?i|x70ohvpYt?S&&B)WSsZtsgod+W1L?nPhLUqW}8e;Mf>?3?i`=$?Iw zxbyHF`plm~I)DB0Tkjm@u-?8`LB!6_wDS(qzTU*U^Ds3tYig`B*LcRJi~YaYjS@5e z-?|xm*8CkiRqy*I{tCz~hcLgYF%Y+qp3%MK44p^L&@*lT&e+K_y{hf#E-?-7xUaG6 zJKyJaZey=9A35)U7I;gpT(iZ8_~W}^B_p%Oz*lJwlhu^QUhST`qme^%&h;W5>DV+-17< zdjjm~Ud^{p&g3b&ao0Ng^Ib0i@4)xlLDM!bYo4K7*YCa>(6r?0T^ya+yDRo literal 0 HcmV?d00001 diff --git a/naga/tests/in/spv/spec-constants-issue-5598.spvasm b/naga/tests/in/spv/spec-constants-issue-5598.spvasm new file mode 100644 index 0000000000..a1fdbcbdd8 --- /dev/null +++ b/naga/tests/in/spv/spec-constants-issue-5598.spvasm @@ -0,0 +1,96 @@ +; SPIR-V +; Version: 1.5 +; Generator: Google rspirv; 0 +; Bound: 68 +; Schema: 0 + OpCapability Shader + OpCapability VulkanMemoryModel + OpMemoryModel Logical Vulkan + OpEntryPoint Fragment %1 "fragment" %gl_FragCoord %3 + OpEntryPoint Vertex %4 "vertex" %gl_VertexIndex %gl_Position + OpExecutionMode %1 OriginUpperLeft + OpDecorate %gl_FragCoord BuiltIn FragCoord + OpDecorate %10 SpecId 100 + OpDecorate %3 Location 0 + OpDecorate %_arr_v4float_uint_6 ArrayStride 16 + OpDecorate %gl_VertexIndex BuiltIn VertexIndex + OpDecorate %gl_Position BuiltIn Position + OpDecorate %gl_Position Invariant + %float = OpTypeFloat 32 + %v4float = OpTypeVector %float 4 +%_ptr_Input_v4float = OpTypePointer Input %v4float +%_ptr_Output_v4float = OpTypePointer Output %v4float + %void = OpTypeVoid + %17 = OpTypeFunction %void +%gl_FragCoord = OpVariable %_ptr_Input_v4float Input + %bool = OpTypeBool + %uint = OpTypeInt 32 0 + %10 = OpSpecConstant %uint 2 + %uint_1 = OpConstant %uint 1 + %v2float = OpTypeVector %float 2 +%_ptr_Output_float = OpTypePointer Output %float + %3 = OpVariable %_ptr_Output_v4float Output + %uint_0 = OpConstant %uint 0 +%_ptr_Input_uint = OpTypePointer Input %uint + %uint_6 = OpConstant %uint 6 +%_arr_v4float_uint_6 = OpTypeArray %v4float %uint_6 +%_ptr_Function__arr_v4float_uint_6 = OpTypePointer Function %_arr_v4float_uint_6 +%gl_VertexIndex = OpVariable %_ptr_Input_uint Input + %float_n1 = OpConstant %float -1 + %float_0 = OpConstant %float 0 + %float_1 = OpConstant %float 1 + %32 = OpConstantComposite %v4float %float_n1 %float_n1 %float_0 %float_1 + %33 = OpConstantComposite %v4float %float_1 %float_n1 %float_0 %float_1 + %34 = OpConstantComposite %v4float %float_1 %float_1 %float_0 %float_1 + %35 = OpConstantComposite %v4float %float_n1 %float_1 %float_0 %float_1 + %36 = OpConstantComposite %_arr_v4float_uint_6 %32 %33 %34 %34 %35 %32 +%_ptr_Function_v4float = OpTypePointer Function %v4float +%gl_Position = OpVariable %_ptr_Output_v4float Output + %float_0_25 = OpConstant %float 0.25 + %float_0_5 = OpConstant %float 0.5 + %1 = OpFunction %void None %17 + %38 = OpLabel + %39 = OpLoad %v4float %gl_FragCoord + %40 = OpCompositeExtract %float %39 0 + %41 = OpCompositeExtract %float %39 1 + %42 = OpIEqual %bool %10 %uint_1 + OpSelectionMerge %43 None + OpBranchConditional %42 %44 %45 + %44 = OpLabel + %46 = OpFMul %float %40 %float_0_5 + %47 = OpFMul %float %41 %float_0_5 + %48 = OpCompositeConstruct %v2float %46 %47 + OpBranch %43 + %45 = OpLabel + %49 = OpFMul %float %40 %float_0_25 + %50 = OpFMul %float %41 %float_0_25 + %51 = OpCompositeConstruct %v2float %49 %50 + OpBranch %43 + %43 = OpLabel + %52 = OpPhi %v2float %48 %44 %51 %45 + %53 = OpCompositeExtract %float %52 0 + %54 = OpAccessChain %_ptr_Output_float %3 %uint_0 + OpStore %54 %53 + %55 = OpCompositeExtract %float %52 1 + %56 = OpAccessChain %_ptr_Output_float %3 %uint_1 + OpStore %56 %55 + OpReturn + OpFunctionEnd + %4 = OpFunction %void None %17 + %57 = OpLabel + %58 = OpVariable %_ptr_Function__arr_v4float_uint_6 Function + %59 = OpLoad %uint %gl_VertexIndex + OpStore %58 %36 + %60 = OpULessThan %bool %59 %uint_6 + OpSelectionMerge %61 None + OpBranchConditional %60 %62 %63 + %62 = OpLabel + %64 = OpInBoundsAccessChain %_ptr_Function_v4float %58 %59 + %65 = OpLoad %v4float %64 + OpStore %gl_Position %65 + OpBranch %61 + %63 = OpLabel + OpBranch %61 + %61 = OpLabel + OpReturn + OpFunctionEnd diff --git a/naga/tests/out/glsl/spec-constants-issue-5598.fragment.Fragment.glsl b/naga/tests/out/glsl/spec-constants-issue-5598.fragment.Fragment.glsl new file mode 100644 index 0000000000..e81d8fa1b1 --- /dev/null +++ b/naga/tests/out/glsl/spec-constants-issue-5598.fragment.Fragment.glsl @@ -0,0 +1,34 @@ +#version 310 es + +precision highp float; +precision highp int; + +vec4 global = vec4(0.0); + +vec4 global_1 = vec4(0.0); + +layout(location = 0) out vec4 _fs2p_location0; + +void function() { + vec2 phi_52_ = vec2(0.0); + vec4 _e7 = global; + if (false) { + phi_52_ = vec2((_e7.x * 0.5), (_e7.y * 0.5)); + } else { + phi_52_ = vec2((_e7.x * 0.25), (_e7.y * 0.25)); + } + vec2 _e20 = phi_52_; + global_1[0u] = _e20.x; + global_1[1u] = _e20.y; + return; +} + +void main() { + vec4 param = gl_FragCoord; + global = param; + function(); + vec4 _e3 = global_1; + _fs2p_location0 = _e3; + return; +} + diff --git a/naga/tests/out/glsl/spec-constants-issue-5598.vertex.Vertex.glsl b/naga/tests/out/glsl/spec-constants-issue-5598.vertex.Vertex.glsl new file mode 100644 index 0000000000..256e9380ac --- /dev/null +++ b/naga/tests/out/glsl/spec-constants-issue-5598.vertex.Vertex.glsl @@ -0,0 +1,34 @@ +#version 310 es + +precision highp float; +precision highp int; + +uint global_2 = 0u; + +vec4 global_3 = vec4(0.0, 0.0, 0.0, 1.0); + +invariant gl_Position; + +void function_1() { + vec4 local[6] = vec4[6](vec4(0.0), vec4(0.0), vec4(0.0), vec4(0.0), vec4(0.0), vec4(0.0)); + uint _e5 = global_2; + local = vec4[6](vec4(-1.0, -1.0, 0.0, 1.0), vec4(1.0, -1.0, 0.0, 1.0), vec4(1.0, 1.0, 0.0, 1.0), vec4(1.0, 1.0, 0.0, 1.0), vec4(-1.0, 1.0, 0.0, 1.0), vec4(-1.0, -1.0, 0.0, 1.0)); + if ((_e5 < 6u)) { + vec4 _e8 = local[_e5]; + global_3 = _e8; + } + return; +} + +void main() { + uint param_1 = uint(gl_VertexID); + global_2 = param_1; + function_1(); + float _e4 = global_3.y; + global_3.y = -(_e4); + vec4 _e6 = global_3; + gl_Position = _e6; + gl_Position.yz = vec2(-gl_Position.y, gl_Position.z * 2.0 - gl_Position.w); + return; +} + diff --git a/naga/tests/snapshots.rs b/naga/tests/snapshots.rs index 80ddc6ba1d..ee775a3e63 100644 --- a/naga/tests/snapshots.rs +++ b/naga/tests/snapshots.rs @@ -973,6 +973,7 @@ fn convert_spv_all() { ); convert_spv("builtin-accessed-outside-entrypoint", true, Targets::WGSL); convert_spv("spec-constants", true, Targets::IR); + convert_spv("spec-constants-issue-5598", true, Targets::GLSL); convert_spv( "subgroup-operations-s", false,