Skip to content

Commit

Permalink
[naga] fix the way we adjust constant initializers when processing ov…
Browse files Browse the repository at this point in the history
…errides

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
  • Loading branch information
teoxoy authored and jimblandy committed Apr 25, 2024
1 parent 5735f85 commit 55c9d69
Show file tree
Hide file tree
Showing 6 changed files with 169 additions and 2 deletions.
6 changes: 4 additions & 2 deletions naga/src/back/pipeline_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Binary file added naga/tests/in/spv/spec-constants-issue-5598.spv
Binary file not shown.
96 changes: 96 additions & 0 deletions naga/tests/in/spv/spec-constants-issue-5598.spvasm
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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;
}

34 changes: 34 additions & 0 deletions naga/tests/out/glsl/spec-constants-issue-5598.vertex.Vertex.glsl
Original file line number Diff line number Diff line change
@@ -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;
}

1 change: 1 addition & 0 deletions naga/tests/snapshots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 55c9d69

Please sign in to comment.