transform: Avoid symbol collision in Canonicalize IO
Correctly rename fields when combining two or more input structures together into a single input structure.
Bug: chromium:1251009
Change-Id: I0c7ab5ed3116b97035e100d1ef96e772e819f640
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/64545
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/src/transform/canonicalize_entry_point_io.cc b/src/transform/canonicalize_entry_point_io.cc
index 91568cb..4814e12 100644
--- a/src/transform/canonicalize_entry_point_io.cc
+++ b/src/transform/canonicalize_entry_point_io.cc
@@ -112,6 +112,8 @@
std::vector<OutputValue> wrapper_output_values;
/// The body of the wrapper function.
ast::StatementList wrapper_body;
+ /// Input names used by the entrypoint
+ std::unordered_set<std::string> input_names;
/// Constructor
/// @param context the clone context
@@ -171,29 +173,35 @@
ctx.dst->ID(), ast::DisabledValidation::kIgnoreStorageClass));
// Create the global variable and use its value for the shader input.
- auto var = ctx.dst->Symbols().New(name);
- ast::Expression* value = ctx.dst->Expr(var);
+ auto symbol = ctx.dst->Symbols().New(name);
+ ast::Expression* value = ctx.dst->Expr(symbol);
if (HasSampleMask(attributes)) {
// Vulkan requires the type of a SampleMask builtin to be an array.
// Declare it as array<u32, 1> and then load the first element.
type = ctx.dst->ty.array(type, 1);
value = ctx.dst->IndexAccessor(value, 0);
}
- ctx.dst->Global(var, type, ast::StorageClass::kInput,
+ ctx.dst->Global(symbol, type, ast::StorageClass::kInput,
std::move(attributes));
return value;
} else if (cfg.shader_style == ShaderStyle::kMsl &&
ast::HasDecoration<ast::BuiltinDecoration>(attributes)) {
// If this input is a builtin and we are targeting MSL, then add it to the
// parameter list and pass it directly to the inner function.
+ Symbol symbol = input_names.emplace(name).second
+ ? ctx.dst->Symbols().Register(name)
+ : ctx.dst->Symbols().New(name);
wrapper_ep_parameters.push_back(
- ctx.dst->Param(name, type, std::move(attributes)));
- return ctx.dst->Expr(name);
+ ctx.dst->Param(symbol, type, std::move(attributes)));
+ return ctx.dst->Expr(symbol);
} else {
// Otherwise, move it to the new structure member list.
+ Symbol symbol = input_names.emplace(name).second
+ ? ctx.dst->Symbols().Register(name)
+ : ctx.dst->Symbols().New(name);
wrapper_struct_param_members.push_back(
- ctx.dst->Member(name, type, std::move(attributes)));
- return ctx.dst->MemberAccessor(InputStructSymbol(), name);
+ ctx.dst->Member(symbol, type, std::move(attributes)));
+ return ctx.dst->MemberAccessor(InputStructSymbol(), symbol);
}
}
diff --git a/src/transform/canonicalize_entry_point_io_test.cc b/src/transform/canonicalize_entry_point_io_test.cc
index 1dc93b1..084d221 100644
--- a/src/transform/canonicalize_entry_point_io_test.cc
+++ b/src/transform/canonicalize_entry_point_io_test.cc
@@ -2062,18 +2062,31 @@
var<private> vertex_point_size_1 : f32;
var<private> vertex_point_size_2 : f32;
+struct VertIn1 {
+ [[location(0)]] collide : f32;
+};
+
+struct VertIn2 {
+ [[location(1)]] collide : f32;
+};
+
struct VertOut {
[[location(0)]] vertex_point_size : f32;
[[builtin(position)]] vertex_point_size_1 : vec4<f32>;
};
[[stage(vertex)]]
-fn vert_main() -> VertOut {
+fn vert_main(collide : VertIn1, collide_1 : VertIn2) -> VertOut {
+ let x = collide.collide + collide_1.collide;
return VertOut();
}
)";
auto* expect = R"(
+[[location(0), internal(disable_validation__ignore_storage_class)]] var<in> collide_2 : f32;
+
+[[location(1), internal(disable_validation__ignore_storage_class)]] var<in> collide_3 : f32;
+
[[location(0), internal(disable_validation__ignore_storage_class)]] var<out> vertex_point_size_3 : f32;
[[builtin(position), internal(disable_validation__ignore_storage_class)]] var<out> vertex_point_size_1_1 : vec4<f32>;
@@ -2086,18 +2099,27 @@
var<private> vertex_point_size_2 : f32;
+struct VertIn1 {
+ collide : f32;
+};
+
+struct VertIn2 {
+ collide : f32;
+};
+
struct VertOut {
vertex_point_size : f32;
vertex_point_size_1 : vec4<f32>;
};
-fn vert_main_inner() -> VertOut {
+fn vert_main_inner(collide : VertIn1, collide_1 : VertIn2) -> VertOut {
+ let x = (collide.collide + collide_1.collide);
return VertOut();
}
[[stage(vertex)]]
fn vert_main() {
- let inner_result = vert_main_inner();
+ let inner_result = vert_main_inner(VertIn1(collide_2), VertIn2(collide_3));
vertex_point_size_3 = inner_result.vertex_point_size;
vertex_point_size_1_1 = inner_result.vertex_point_size_1;
vertex_point_size_4 = 1.0;
@@ -2114,24 +2136,48 @@
TEST_F(CanonicalizeEntryPointIOTest, EmitVertexPointSize_AvoidNameClash_Msl) {
auto* src = R"(
+struct VertIn1 {
+ [[location(0)]] collide : f32;
+};
+
+struct VertIn2 {
+ [[location(1)]] collide : f32;
+};
+
struct VertOut {
[[location(0)]] vertex_point_size : vec4<f32>;
[[builtin(position)]] vertex_point_size_1 : vec4<f32>;
};
[[stage(vertex)]]
-fn vert_main() -> VertOut {
+fn vert_main(collide : VertIn1, collide_1 : VertIn2) -> VertOut {
+ let x = collide.collide + collide_1.collide;
return VertOut();
}
)";
auto* expect = R"(
+struct VertIn1 {
+ collide : f32;
+};
+
+struct VertIn2 {
+ collide : f32;
+};
+
struct VertOut {
vertex_point_size : vec4<f32>;
vertex_point_size_1 : vec4<f32>;
};
-struct tint_symbol {
+struct tint_symbol_1 {
+ [[location(0)]]
+ collide : f32;
+ [[location(1)]]
+ collide_2 : f32;
+};
+
+struct tint_symbol_2 {
[[location(0)]]
vertex_point_size : vec4<f32>;
[[builtin(position)]]
@@ -2140,14 +2186,15 @@
vertex_point_size_2 : f32;
};
-fn vert_main_inner() -> VertOut {
+fn vert_main_inner(collide : VertIn1, collide_1 : VertIn2) -> VertOut {
+ let x = (collide.collide + collide_1.collide);
return VertOut();
}
[[stage(vertex)]]
-fn vert_main() -> tint_symbol {
- let inner_result = vert_main_inner();
- var wrapper_result : tint_symbol;
+fn vert_main(tint_symbol : tint_symbol_1) -> tint_symbol_2 {
+ let inner_result = vert_main_inner(VertIn1(tint_symbol.collide), VertIn2(tint_symbol.collide_2));
+ var wrapper_result : tint_symbol_2;
wrapper_result.vertex_point_size = inner_result.vertex_point_size;
wrapper_result.vertex_point_size_1 = inner_result.vertex_point_size_1;
wrapper_result.vertex_point_size_2 = 1.0;
@@ -2163,6 +2210,82 @@
EXPECT_EQ(expect, str(got));
}
+TEST_F(CanonicalizeEntryPointIOTest, EmitVertexPointSize_AvoidNameClash_Hlsl) {
+ auto* src = R"(
+struct VertIn1 {
+ [[location(0)]] collide : f32;
+};
+
+struct VertIn2 {
+ [[location(1)]] collide : f32;
+};
+
+struct VertOut {
+ [[location(0)]] vertex_point_size : vec4<f32>;
+ [[builtin(position)]] vertex_point_size_1 : vec4<f32>;
+};
+
+[[stage(vertex)]]
+fn vert_main(collide : VertIn1, collide_1 : VertIn2) -> VertOut {
+ let x = collide.collide + collide_1.collide;
+ return VertOut();
+}
+)";
+
+ auto* expect = R"(
+struct VertIn1 {
+ collide : f32;
+};
+
+struct VertIn2 {
+ collide : f32;
+};
+
+struct VertOut {
+ vertex_point_size : vec4<f32>;
+ vertex_point_size_1 : vec4<f32>;
+};
+
+struct tint_symbol_1 {
+ [[location(0)]]
+ collide : f32;
+ [[location(1)]]
+ collide_2 : f32;
+};
+
+struct tint_symbol_2 {
+ [[location(0)]]
+ vertex_point_size : vec4<f32>;
+ [[builtin(position)]]
+ vertex_point_size_1 : vec4<f32>;
+ [[builtin(pointsize)]]
+ vertex_point_size_2 : f32;
+};
+
+fn vert_main_inner(collide : VertIn1, collide_1 : VertIn2) -> VertOut {
+ let x = (collide.collide + collide_1.collide);
+ return VertOut();
+}
+
+[[stage(vertex)]]
+fn vert_main(tint_symbol : tint_symbol_1) -> tint_symbol_2 {
+ let inner_result = vert_main_inner(VertIn1(tint_symbol.collide), VertIn2(tint_symbol.collide_2));
+ var wrapper_result : tint_symbol_2;
+ wrapper_result.vertex_point_size = inner_result.vertex_point_size;
+ wrapper_result.vertex_point_size_1 = inner_result.vertex_point_size_1;
+ wrapper_result.vertex_point_size_2 = 1.0;
+ return wrapper_result;
+}
+)";
+
+ DataMap data;
+ data.Add<CanonicalizeEntryPointIO::Config>(
+ CanonicalizeEntryPointIO::ShaderStyle::kHlsl, 0xFFFFFFFF, true);
+ auto got = Run<CanonicalizeEntryPointIO>(src, data);
+
+ EXPECT_EQ(expect, str(got));
+}
+
TEST_F(CanonicalizeEntryPointIOTest, SpirvSampleMaskBuiltins) {
auto* src = R"(
[[stage(fragment)]]
diff --git a/test/bug/chromium/1251009.wgsl b/test/bug/chromium/1251009.wgsl
new file mode 100644
index 0000000..4abe77e
--- /dev/null
+++ b/test/bug/chromium/1251009.wgsl
@@ -0,0 +1,19 @@
+struct VertexInputs0 {
+ [[builtin(vertex_index)]] vertex_index : u32;
+ [[location(0)]] loc0 : i32;
+};
+struct VertexInputs1 {
+ [[location(2)]] loc1 : u32;
+ [[location(3)]] loc3 : vec4<f32>;
+};
+
+[[stage(vertex)]]
+fn main(
+ inputs0 : VertexInputs0,
+ [[location(1)]] loc1 : u32,
+ [[builtin(instance_index)]] instance_index : u32,
+ inputs1 : VertexInputs1,
+) -> [[builtin(position)]] vec4<f32> {
+ let foo : u32 = inputs0.vertex_index + instance_index;
+ return vec4<f32>();
+}
diff --git a/test/bug/chromium/1251009.wgsl.expected.hlsl b/test/bug/chromium/1251009.wgsl.expected.hlsl
new file mode 100644
index 0000000..d0c8230
--- /dev/null
+++ b/test/bug/chromium/1251009.wgsl.expected.hlsl
@@ -0,0 +1,33 @@
+struct VertexInputs0 {
+ uint vertex_index;
+ int loc0;
+};
+struct VertexInputs1 {
+ uint loc1;
+ float4 loc3;
+};
+struct tint_symbol_1 {
+ int loc0 : TEXCOORD0;
+ uint loc1 : TEXCOORD1;
+ uint loc1_1 : TEXCOORD2;
+ float4 loc3 : TEXCOORD3;
+ uint vertex_index : SV_VertexID;
+ uint instance_index : SV_InstanceID;
+};
+struct tint_symbol_2 {
+ float4 value : SV_Position;
+};
+
+float4 main_inner(VertexInputs0 inputs0, uint loc1, uint instance_index, VertexInputs1 inputs1) {
+ const uint foo = (inputs0.vertex_index + instance_index);
+ return float4(0.0f, 0.0f, 0.0f, 0.0f);
+}
+
+tint_symbol_2 main(tint_symbol_1 tint_symbol) {
+ const VertexInputs0 tint_symbol_3 = {tint_symbol.vertex_index, tint_symbol.loc0};
+ const VertexInputs1 tint_symbol_4 = {tint_symbol.loc1_1, tint_symbol.loc3};
+ const float4 inner_result = main_inner(tint_symbol_3, tint_symbol.loc1, tint_symbol.instance_index, tint_symbol_4);
+ tint_symbol_2 wrapper_result = (tint_symbol_2)0;
+ wrapper_result.value = inner_result;
+ return wrapper_result;
+}
diff --git a/test/bug/chromium/1251009.wgsl.expected.msl b/test/bug/chromium/1251009.wgsl.expected.msl
new file mode 100644
index 0000000..b958532
--- /dev/null
+++ b/test/bug/chromium/1251009.wgsl.expected.msl
@@ -0,0 +1,35 @@
+#include <metal_stdlib>
+
+using namespace metal;
+struct VertexInputs0 {
+ uint vertex_index;
+ int loc0;
+};
+struct VertexInputs1 {
+ uint loc1;
+ float4 loc3;
+};
+struct tint_symbol_2 {
+ int loc0 [[attribute(0)]];
+ uint loc1 [[attribute(1)]];
+ uint loc1_1 [[attribute(2)]];
+ float4 loc3 [[attribute(3)]];
+};
+struct tint_symbol_3 {
+ float4 value [[position]];
+};
+
+float4 tint_symbol_inner(VertexInputs0 inputs0, uint loc1, uint instance_index, VertexInputs1 inputs1) {
+ uint const foo = (inputs0.vertex_index + instance_index);
+ return float4();
+}
+
+vertex tint_symbol_3 tint_symbol(uint vertex_index [[vertex_id]], uint instance_index [[instance_id]], tint_symbol_2 tint_symbol_1 [[stage_in]]) {
+ VertexInputs0 const tint_symbol_4 = {.vertex_index=vertex_index, .loc0=tint_symbol_1.loc0};
+ VertexInputs1 const tint_symbol_5 = {.loc1=tint_symbol_1.loc1_1, .loc3=tint_symbol_1.loc3};
+ float4 const inner_result = tint_symbol_inner(tint_symbol_4, tint_symbol_1.loc1, instance_index, tint_symbol_5);
+ tint_symbol_3 wrapper_result = {};
+ wrapper_result.value = inner_result;
+ return wrapper_result;
+}
+
diff --git a/test/bug/chromium/1251009.wgsl.expected.spvasm b/test/bug/chromium/1251009.wgsl.expected.spvasm
new file mode 100644
index 0000000..0a2969f
--- /dev/null
+++ b/test/bug/chromium/1251009.wgsl.expected.spvasm
@@ -0,0 +1,90 @@
+; SPIR-V
+; Version: 1.3
+; Generator: Google Tint Compiler; 0
+; Bound: 45
+; Schema: 0
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Vertex %main "main" %vertex_index_1 %loc0_1 %loc1_1 %instance_index_1 %loc1_2 %loc3_1 %value %vertex_point_size
+ OpName %vertex_index_1 "vertex_index_1"
+ OpName %loc0_1 "loc0_1"
+ OpName %loc1_1 "loc1_1"
+ OpName %instance_index_1 "instance_index_1"
+ OpName %loc1_2 "loc1_2"
+ OpName %loc3_1 "loc3_1"
+ OpName %value "value"
+ OpName %vertex_point_size "vertex_point_size"
+ OpName %VertexInputs0 "VertexInputs0"
+ OpMemberName %VertexInputs0 0 "vertex_index"
+ OpMemberName %VertexInputs0 1 "loc0"
+ OpName %VertexInputs1 "VertexInputs1"
+ OpMemberName %VertexInputs1 0 "loc1"
+ OpMemberName %VertexInputs1 1 "loc3"
+ OpName %main_inner "main_inner"
+ OpName %inputs0 "inputs0"
+ OpName %loc1 "loc1"
+ OpName %instance_index "instance_index"
+ OpName %inputs1 "inputs1"
+ OpName %main "main"
+ OpDecorate %vertex_index_1 BuiltIn VertexIndex
+ OpDecorate %loc0_1 Location 0
+ OpDecorate %loc1_1 Location 1
+ OpDecorate %instance_index_1 BuiltIn InstanceIndex
+ OpDecorate %loc1_2 Location 2
+ OpDecorate %loc3_1 Location 3
+ OpDecorate %value BuiltIn Position
+ OpDecorate %vertex_point_size BuiltIn PointSize
+ OpMemberDecorate %VertexInputs0 0 Offset 0
+ OpMemberDecorate %VertexInputs0 1 Offset 4
+ OpMemberDecorate %VertexInputs1 0 Offset 0
+ OpMemberDecorate %VertexInputs1 1 Offset 16
+ %uint = OpTypeInt 32 0
+%_ptr_Input_uint = OpTypePointer Input %uint
+%vertex_index_1 = OpVariable %_ptr_Input_uint Input
+ %int = OpTypeInt 32 1
+%_ptr_Input_int = OpTypePointer Input %int
+ %loc0_1 = OpVariable %_ptr_Input_int Input
+ %loc1_1 = OpVariable %_ptr_Input_uint Input
+%instance_index_1 = OpVariable %_ptr_Input_uint Input
+ %loc1_2 = OpVariable %_ptr_Input_uint Input
+ %float = OpTypeFloat 32
+ %v4float = OpTypeVector %float 4
+%_ptr_Input_v4float = OpTypePointer Input %v4float
+ %loc3_1 = OpVariable %_ptr_Input_v4float Input
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+ %16 = OpConstantNull %v4float
+ %value = OpVariable %_ptr_Output_v4float Output %16
+%_ptr_Output_float = OpTypePointer Output %float
+ %19 = OpConstantNull %float
+%vertex_point_size = OpVariable %_ptr_Output_float Output %19
+%VertexInputs0 = OpTypeStruct %uint %int
+%VertexInputs1 = OpTypeStruct %uint %v4float
+ %20 = OpTypeFunction %v4float %VertexInputs0 %uint %uint %VertexInputs1
+ %void = OpTypeVoid
+ %31 = OpTypeFunction %void
+ %float_1 = OpConstant %float 1
+ %main_inner = OpFunction %v4float None %20
+ %inputs0 = OpFunctionParameter %VertexInputs0
+ %loc1 = OpFunctionParameter %uint
+%instance_index = OpFunctionParameter %uint
+ %inputs1 = OpFunctionParameter %VertexInputs1
+ %28 = OpLabel
+ %29 = OpCompositeExtract %uint %inputs0 0
+ %30 = OpIAdd %uint %29 %instance_index
+ OpReturnValue %16
+ OpFunctionEnd
+ %main = OpFunction %void None %31
+ %34 = OpLabel
+ %36 = OpLoad %uint %vertex_index_1
+ %37 = OpLoad %int %loc0_1
+ %38 = OpCompositeConstruct %VertexInputs0 %36 %37
+ %39 = OpLoad %uint %loc1_1
+ %40 = OpLoad %uint %instance_index_1
+ %41 = OpLoad %uint %loc1_2
+ %42 = OpLoad %v4float %loc3_1
+ %43 = OpCompositeConstruct %VertexInputs1 %41 %42
+ %35 = OpFunctionCall %v4float %main_inner %38 %39 %40 %43
+ OpStore %value %35
+ OpStore %vertex_point_size %float_1
+ OpReturn
+ OpFunctionEnd
diff --git a/test/bug/chromium/1251009.wgsl.expected.wgsl b/test/bug/chromium/1251009.wgsl.expected.wgsl
new file mode 100644
index 0000000..5de6e50
--- /dev/null
+++ b/test/bug/chromium/1251009.wgsl.expected.wgsl
@@ -0,0 +1,19 @@
+struct VertexInputs0 {
+ [[builtin(vertex_index)]]
+ vertex_index : u32;
+ [[location(0)]]
+ loc0 : i32;
+};
+
+struct VertexInputs1 {
+ [[location(2)]]
+ loc1 : u32;
+ [[location(3)]]
+ loc3 : vec4<f32>;
+};
+
+[[stage(vertex)]]
+fn main(inputs0 : VertexInputs0, [[location(1)]] loc1 : u32, [[builtin(instance_index)]] instance_index : u32, inputs1 : VertexInputs1) -> [[builtin(position)]] vec4<f32> {
+ let foo : u32 = (inputs0.vertex_index + instance_index);
+ return vec4<f32>();
+}
