GLSL: fix bug with padded struct members.
When the same struct is reused as a uniform variable in
both vertex and fragment stages, the padding members may have
different names, causing an error at GLSL link time.
The fix is to ensure struct member name uniqueness only within
a given struct, not throughout the shader.
Bug: tint:2152
Change-Id: If71aa27eeeedb27c27711ee6785319b412929dfa
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/172381
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>
diff --git a/src/dawn/tests/end2end/ShaderTests.cpp b/src/dawn/tests/end2end/ShaderTests.cpp
index b3fd4ff..f10a4ea 100644
--- a/src/dawn/tests/end2end/ShaderTests.cpp
+++ b/src/dawn/tests/end2end/ShaderTests.cpp
@@ -2128,6 +2128,38 @@
device.CreateRenderPipeline(&desc);
}
+// Test that padding is correctly applied to a UBO used in both vert and
+// frag stages. Insert an additional UBO in the frag stage before the reused UBO.
+TEST_P(ShaderTests, UniformAcrossStagesSeparateModuleMismatchWithCustomSize) {
+ wgpu::ShaderModule module = utils::CreateShaderModule(device, R"(
+ struct A {
+ f : f32,
+ };
+ struct B {
+ u : u32,
+ }
+ @group(0) @binding(0) var<uniform> tint_symbol_ubo_0: A;
+ @group(0) @binding(1) var<uniform> tint_symbol_ubo_1: B;
+
+ @vertex fn vertex() -> @builtin(position) vec4f {
+ _ = tint_symbol_ubo_0;
+ _ = tint_symbol_ubo_1;
+ return vec4f(tint_symbol_ubo_0.f) + vec4f(f32(tint_symbol_ubo_1.u));
+ }
+
+ @fragment fn fragment() -> @location(0) vec4f {
+ _ = tint_symbol_ubo_1;
+ return vec4f(f32(tint_symbol_ubo_1.u));
+ }
+ )");
+
+ utils::ComboRenderPipelineDescriptor desc;
+ desc.vertex.module = module;
+ desc.cFragment.module = module;
+
+ device.CreateRenderPipeline(&desc);
+}
+
// Having different block contents at the same binding point used in different stages is allowed.
TEST_P(ShaderTests, UniformAcrossStagesSameBindingPointCollide) {
wgpu::ShaderModule module = utils::CreateShaderModule(device, R"(
diff --git a/src/tint/lang/glsl/writer/ast_raise/pad_structs.cc b/src/tint/lang/glsl/writer/ast_raise/pad_structs.cc
index 0037935..0d4aa89 100644
--- a/src/tint/lang/glsl/writer/ast_raise/pad_structs.cc
+++ b/src/tint/lang/glsl/writer/ast_raise/pad_structs.cc
@@ -50,13 +50,20 @@
void CreatePadding(Vector<const ast::StructMember*, 8>* new_members,
Hashset<const ast::StructMember*, 8>* padding_members,
+ Hashset<std::string, 8>* member_names,
+ int& last_padding_index,
ast::Builder* b,
uint32_t bytes) {
const size_t count = bytes / 4u;
padding_members->Reserve(count);
new_members->Reserve(count);
+ member_names->Reserve(count);
+ std::string name = "pad";
for (uint32_t i = 0; i < count; ++i) {
- auto name = b->Symbols().New("pad");
+ while (member_names->Contains(name)) {
+ name = "pad_" + std::to_string(++last_padding_index);
+ }
+ member_names->Add(name);
auto* member = b->Member(name, b->ty.u32());
padding_members->Add(member);
new_members->Push(member);
@@ -87,11 +94,18 @@
uint32_t offset = 0;
bool has_runtime_sized_array = false;
tint::Vector<const ast::StructMember*, 8> new_members;
+ Hashset<std::string, 8> member_names;
+ for (auto* mem : str->Members()) {
+ member_names.Add(mem->Name().Name());
+ }
+
+ int last_padding_index = 0;
for (auto* mem : str->Members()) {
auto name = mem->Name().Name();
if (offset < mem->Offset()) {
- CreatePadding(&new_members, &padding_members, ctx.dst, mem->Offset() - offset);
+ CreatePadding(&new_members, &padding_members, &member_names, last_padding_index,
+ ctx.dst, mem->Offset() - offset);
offset = mem->Offset();
}
@@ -118,7 +132,8 @@
struct_size = tint::RoundUp(16u, struct_size);
}
if (offset < struct_size && !has_runtime_sized_array) {
- CreatePadding(&new_members, &padding_members, ctx.dst, struct_size - offset);
+ CreatePadding(&new_members, &padding_members, &member_names, last_padding_index,
+ ctx.dst, struct_size - offset);
}
tint::Vector<const ast::Attribute*, 1> struct_attribs;
diff --git a/src/tint/lang/glsl/writer/ast_raise/pad_structs_test.cc b/src/tint/lang/glsl/writer/ast_raise/pad_structs_test.cc
index 170fad5..91107c5 100644
--- a/src/tint/lang/glsl/writer/ast_raise/pad_structs_test.cc
+++ b/src/tint/lang/glsl/writer/ast_raise/pad_structs_test.cc
@@ -580,6 +580,142 @@
EXPECT_EQ(expect, str(got));
}
+TEST_F(PadStructsTest, MemberNamedPad) {
+ auto* src = R"(
+struct S {
+ @align(8) pad_5 : u32,
+ @align(8) pad_3 : u32,
+ @align(8) pad : u32,
+ @align(8) pad_1 : u32,
+}
+
+@group(0) @binding(0) var<uniform> s : S;
+
+fn main() {
+ _ = s;
+}
+)";
+ auto* expect = R"(
+@internal(disable_validation__ignore_struct_member)
+struct S {
+ pad_5 : u32,
+ pad_2 : u32,
+ pad_3 : u32,
+ pad_4 : u32,
+ pad : u32,
+ pad_6 : u32,
+ pad_1 : u32,
+ pad_7 : u32,
+}
+
+@group(0) @binding(0) var<uniform> s : S;
+
+fn main() {
+ _ = s;
+}
+)";
+
+ ast::transform::DataMap data;
+ auto got = Run<PadStructs>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, TwoStructs) {
+ auto* src = R"(
+struct S {
+ @align(8) i : i32,
+ @align(8) u : u32,
+}
+
+struct T {
+ @align(8) i : i32,
+ @align(8) u : u32,
+}
+
+@group(0) @binding(0) var<uniform> s : S;
+@group(0) @binding(1) var<uniform> t : T;
+
+fn main() {
+ _ = s;
+ _ = t;
+}
+)";
+ auto* expect = R"(
+@internal(disable_validation__ignore_struct_member)
+struct S {
+ i : i32,
+ pad : u32,
+ u : u32,
+ pad_1 : u32,
+}
+
+@internal(disable_validation__ignore_struct_member)
+struct T {
+ i : i32,
+ pad : u32,
+ u : u32,
+ pad_1 : u32,
+}
+
+@group(0) @binding(0) var<uniform> s : S;
+
+@group(0) @binding(1) var<uniform> t : T;
+
+fn main() {
+ _ = s;
+ _ = t;
+}
+)";
+
+ ast::transform::DataMap data;
+ auto got = Run<PadStructs>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PadStructsTest, GlobalVariableNamedPad) {
+ auto* src = R"(
+@group(0) @binding(0) var<uniform> pad : u32;
+@group(0) @binding(1) var<uniform> pad_1 : u32;
+
+struct S {
+ @align(8) i : u32,
+ @align(8) j : u32,
+}
+
+@group(0) @binding(1) var<uniform> s : S;
+
+fn main() {
+ _ = s;
+}
+)";
+ auto* expect = R"(
+@group(0) @binding(0) var<uniform> pad : u32;
+
+@group(0) @binding(1) var<uniform> pad_1 : u32;
+
+@internal(disable_validation__ignore_struct_member)
+struct S {
+ i : u32,
+ pad : u32,
+ j : u32,
+ pad_1 : u32,
+}
+
+@group(0) @binding(1) var<uniform> s : S;
+
+fn main() {
+ _ = s;
+}
+)";
+
+ ast::transform::DataMap data;
+ auto got = Run<PadStructs>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
TEST_F(PadStructsTest, InitializerZeroArgs) {
// Calls to a zero-argument initializer of a padded struct should not be modified.
auto* src = R"(
diff --git a/test/tint/array/strides.spvasm.expected.glsl b/test/tint/array/strides.spvasm.expected.glsl
index 94ee04c..bb4e867 100644
--- a/test/tint/array/strides.spvasm.expected.glsl
+++ b/test/tint/array/strides.spvasm.expected.glsl
@@ -7,6 +7,7 @@
struct strided_arr_1 {
strided_arr el[3][2];
+ uint pad;
uint pad_1;
uint pad_2;
uint pad_3;
@@ -26,7 +27,6 @@
uint pad_17;
uint pad_18;
uint pad_19;
- uint pad_20;
};
struct S {
diff --git a/test/tint/buffer/uniform/static_index/read.wgsl.expected.glsl b/test/tint/buffer/uniform/static_index/read.wgsl.expected.glsl
index 165e6ad..a00e376 100644
--- a/test/tint/buffer/uniform/static_index/read.wgsl.expected.glsl
+++ b/test/tint/buffer/uniform/static_index/read.wgsl.expected.glsl
@@ -19,18 +19,18 @@
float scalar_f32;
int scalar_i32;
uint scalar_u32;
- uint pad_6;
+ uint pad;
vec2 vec2_f32;
ivec2 vec2_i32;
uvec2 vec2_u32;
- uint pad_7;
- uint pad_8;
+ uint pad_1;
+ uint pad_2;
vec3 vec3_f32;
- uint pad_9;
+ uint pad_3;
ivec3 vec3_i32;
- uint pad_10;
+ uint pad_4;
uvec3 vec3_u32;
- uint pad_11;
+ uint pad_5;
vec4 vec4_f32;
ivec4 vec4_i32;
uvec4 vec4_u32;
@@ -38,8 +38,8 @@
mat2x3 mat2x3_f32;
mat2x4 mat2x4_f32;
mat3x2 mat3x2_f32;
- uint pad_12;
- uint pad_13;
+ uint pad_6;
+ uint pad_7;
mat3 mat3x3_f32;
mat3x4 mat3x4_f32;
mat4x2 mat4x2_f32;
@@ -54,18 +54,18 @@
float scalar_f32;
int scalar_i32;
uint scalar_u32;
- uint pad_6;
+ uint pad;
vec2 vec2_f32;
ivec2 vec2_i32;
uvec2 vec2_u32;
- uint pad_7;
- uint pad_8;
+ uint pad_1;
+ uint pad_2;
vec3 vec3_f32;
- uint pad_9;
+ uint pad_3;
ivec3 vec3_i32;
- uint pad_10;
+ uint pad_4;
uvec3 vec3_u32;
- uint pad_11;
+ uint pad_5;
vec4 vec4_f32;
ivec4 vec4_i32;
uvec4 vec4_u32;
@@ -76,8 +76,8 @@
vec2 mat3x2_f32_0;
vec2 mat3x2_f32_1;
vec2 mat3x2_f32_2;
- uint pad_12;
- uint pad_13;
+ uint pad_6;
+ uint pad_7;
mat3 mat3x3_f32;
mat3x4 mat3x4_f32;
vec2 mat4x2_f32_0;
diff --git a/test/tint/buffer/uniform/static_index/read_f16.wgsl.expected.glsl b/test/tint/buffer/uniform/static_index/read_f16.wgsl.expected.glsl
index 8964ba4..d0db985 100644
--- a/test/tint/buffer/uniform/static_index/read_f16.wgsl.expected.glsl
+++ b/test/tint/buffer/uniform/static_index/read_f16.wgsl.expected.glsl
@@ -28,28 +28,28 @@
ivec2 vec2_i32;
uvec2 vec2_u32;
f16vec2 vec2_f16;
- uint pad_1;
+ uint pad;
vec3 vec3_f32;
- uint pad_2;
+ uint pad_1;
ivec3 vec3_i32;
- uint pad_3;
+ uint pad_2;
uvec3 vec3_u32;
- uint pad_4;
+ uint pad_3;
f16vec3 vec3_f16;
+ uint pad_4;
uint pad_5;
- uint pad_6;
vec4 vec4_f32;
ivec4 vec4_i32;
uvec4 vec4_u32;
f16vec4 vec4_f16;
mat2 mat2x2_f32;
+ uint pad_6;
uint pad_7;
- uint pad_8;
mat2x3 mat2x3_f32;
mat2x4 mat2x4_f32;
mat3x2 mat3x2_f32;
+ uint pad_8;
uint pad_9;
- uint pad_10;
mat3 mat3x3_f32;
mat3x4 mat3x4_f32;
mat4x2 mat4x2_f32;
@@ -59,14 +59,14 @@
f16mat2x3 mat2x3_f16;
f16mat2x4 mat2x4_f16;
f16mat3x2 mat3x2_f16;
- uint pad_11;
+ uint pad_10;
f16mat3 mat3x3_f16;
f16mat3x4 mat3x4_f16;
f16mat4x2 mat4x2_f16;
f16mat4x3 mat4x3_f16;
f16mat4 mat4x4_f16;
+ uint pad_11;
uint pad_12;
- uint pad_13;
vec3 arr2_vec3_f32[2];
f16mat4x2 arr2_mat4x2_f16[2];
Inner struct_inner;
@@ -82,31 +82,31 @@
ivec2 vec2_i32;
uvec2 vec2_u32;
f16vec2 vec2_f16;
- uint pad_1;
+ uint pad;
vec3 vec3_f32;
- uint pad_2;
+ uint pad_1;
ivec3 vec3_i32;
- uint pad_3;
+ uint pad_2;
uvec3 vec3_u32;
- uint pad_4;
+ uint pad_3;
f16vec3 vec3_f16;
+ uint pad_4;
uint pad_5;
- uint pad_6;
vec4 vec4_f32;
ivec4 vec4_i32;
uvec4 vec4_u32;
f16vec4 vec4_f16;
vec2 mat2x2_f32_0;
vec2 mat2x2_f32_1;
+ uint pad_6;
uint pad_7;
- uint pad_8;
mat2x3 mat2x3_f32;
mat2x4 mat2x4_f32;
vec2 mat3x2_f32_0;
vec2 mat3x2_f32_1;
vec2 mat3x2_f32_2;
+ uint pad_8;
uint pad_9;
- uint pad_10;
mat3 mat3x3_f32;
mat3x4 mat3x4_f32;
vec2 mat4x2_f32_0;
@@ -124,7 +124,7 @@
f16vec2 mat3x2_f16_0;
f16vec2 mat3x2_f16_1;
f16vec2 mat3x2_f16_2;
- uint pad_11;
+ uint pad_10;
f16vec3 mat3x3_f16_0;
f16vec3 mat3x3_f16_1;
f16vec3 mat3x3_f16_2;
@@ -143,8 +143,8 @@
f16vec4 mat4x4_f16_1;
f16vec4 mat4x4_f16_2;
f16vec4 mat4x4_f16_3;
+ uint pad_11;
uint pad_12;
- uint pad_13;
vec3 arr2_vec3_f32[2];
mat4x2_f16_4 arr2_mat4x2_f16[2];
Inner struct_inner;
diff --git a/test/tint/bug/chromium/1434271.wgsl.expected.glsl b/test/tint/bug/chromium/1434271.wgsl.expected.glsl
index c0928e0..c5bf57e 100644
--- a/test/tint/bug/chromium/1434271.wgsl.expected.glsl
+++ b/test/tint/bug/chromium/1434271.wgsl.expected.glsl
@@ -279,8 +279,8 @@
float lifetime;
vec4 color;
vec2 velocity;
- uint pad_3;
- uint pad_4;
+ uint pad;
+ uint pad_1;
};
layout(binding = 0, std140) uniform sim_params_block_ubo {
diff --git a/test/tint/bug/fxc/indexed_assign_to_array_in_struct/1206.wgsl.expected.glsl b/test/tint/bug/fxc/indexed_assign_to_array_in_struct/1206.wgsl.expected.glsl
index 92f22e2..dec3961 100644
--- a/test/tint/bug/fxc/indexed_assign_to_array_in_struct/1206.wgsl.expected.glsl
+++ b/test/tint/bug/fxc/indexed_assign_to_array_in_struct/1206.wgsl.expected.glsl
@@ -10,12 +10,12 @@
struct Particle {
vec3 position[8];
float lifetime;
- uint pad_3;
- uint pad_4;
- uint pad_5;
+ uint pad;
+ uint pad_1;
+ uint pad_2;
vec4 color;
vec3 velocity;
- uint pad_6;
+ uint pad_3;
};
layout(binding = 3, std430) buffer Particles_ssbo {
diff --git a/test/tint/bug/tint/1088.spvasm.expected.glsl b/test/tint/bug/tint/1088.spvasm.expected.glsl
index d2cb29d..4d9812d 100644
--- a/test/tint/bug/tint/1088.spvasm.expected.glsl
+++ b/test/tint/bug/tint/1088.spvasm.expected.glsl
@@ -14,9 +14,9 @@
struct LeftOver {
mat4 worldViewProjection;
float time;
- uint pad_3;
- uint pad_4;
- uint pad_5;
+ uint pad;
+ uint pad_1;
+ uint pad_2;
mat4 test2[2];
strided_arr test[4];
};
diff --git a/test/tint/bug/tint/942.wgsl.expected.glsl b/test/tint/bug/tint/942.wgsl.expected.glsl
index 3fbed02..34c5aec 100644
--- a/test/tint/bug/tint/942.wgsl.expected.glsl
+++ b/test/tint/bug/tint/942.wgsl.expected.glsl
@@ -14,9 +14,9 @@
layout(rgba8) uniform highp writeonly image2D outputTex;
struct Flip {
uint value;
+ uint pad;
+ uint pad_1;
uint pad_2;
- uint pad_3;
- uint pad_4;
};
layout(binding = 3, std140) uniform flip_block_ubo {
diff --git a/test/tint/bug/tint/949.wgsl.expected.glsl b/test/tint/bug/tint/949.wgsl.expected.glsl
index 05c3b39..dfa3c85 100644
--- a/test/tint/bug/tint/949.wgsl.expected.glsl
+++ b/test/tint/bug/tint/949.wgsl.expected.glsl
@@ -33,8 +33,8 @@
uint padding_2;
vec4 shadowsInfo;
vec2 depthValues;
- uint pad_2;
- uint pad_3;
+ uint pad;
+ uint pad_1;
};
float u_Float = 0.0f;
