From f01ed13a254d3d2f4387f78aba88989de8212bd6 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 13 Apr 2021 23:08:37 +0000 Subject: [PATCH] transform/VertexPulling: s/_tint/tint Don't use a leading underscore in identifiers. Tint cannot parse these, they're illegal in MSL. Bug: tint:640 Change-Id: I6e923cf3317a646cc5f4f2a10a7a522036000c70 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47634 Commit-Queue: Ben Clayton Reviewed-by: James Price --- src/transform/vertex_pulling.cc | 22 ++-- src/transform/vertex_pulling_test.cc | 153 ++++++++++++++++++--------- 2 files changed, 116 insertions(+), 59 deletions(-) diff --git a/src/transform/vertex_pulling.cc b/src/transform/vertex_pulling.cc index df00a17a10..af4f00b7c4 100644 --- a/src/transform/vertex_pulling.cc +++ b/src/transform/vertex_pulling.cc @@ -48,7 +48,6 @@ struct State { CloneContext& ctx; VertexPulling::Config const cfg; std::unordered_map location_to_var; - std::vector location_replacements; Symbol vertex_index_name; Symbol instance_index_name; Symbol pulling_position_name; @@ -60,7 +59,7 @@ struct State { Symbol GetVertexBufferName(uint32_t index) { return utils::GetOrCreate(vertex_buffer_names, index, [&] { static const char kVertexBufferNamePrefix[] = - "_tint_pulling_vertex_buffer_"; + "tint_pulling_vertex_buffer_"; return ctx.dst->Symbols().New(kVertexBufferNamePrefix + std::to_string(index)); }); @@ -69,7 +68,7 @@ struct State { /// Lazily generates the pulling position symbol Symbol GetPullingPositionName() { if (!pulling_position_name.IsValid()) { - static const char kPullingPosVarName[] = "_tint_pulling_pos"; + static const char kPullingPosVarName[] = "tint_pulling_pos"; pulling_position_name = ctx.dst->Symbols().New(kPullingPosVarName); } return pulling_position_name; @@ -78,7 +77,7 @@ struct State { /// Lazily generates the structure buffer symbol Symbol GetStructBufferName() { if (!struct_buffer_name.IsValid()) { - static const char kStructBufferName[] = "_tint_vertex_data"; + static const char kStructBufferName[] = "tint_vertex_data"; struct_buffer_name = ctx.dst->Symbols().New(kStructBufferName); } return struct_buffer_name; @@ -115,7 +114,7 @@ struct State { } // We didn't find a vertex index builtin, so create one - static const char kDefaultVertexIndexName[] = "_tint_pulling_vertex_index"; + static const char kDefaultVertexIndexName[] = "tint_pulling_vertex_index"; vertex_index_name = ctx.dst->Symbols().New(kDefaultVertexIndexName); ctx.dst->Global( @@ -158,7 +157,7 @@ struct State { // We didn't find an instance index builtin, so create one static const char kDefaultInstanceIndexName[] = - "_tint_pulling_instance_index"; + "tint_pulling_instance_index"; instance_index_name = ctx.dst->Symbols().New(kDefaultInstanceIndexName); ctx.dst->Global(instance_index_name, ctx.dst->ty.u32(), @@ -187,8 +186,7 @@ struct State { ctx.Clone(v->declared_type()), ast::StorageClass::kPrivate); location_to_var[location] = replacement; - location_replacements.emplace_back( - LocationReplacement{v, replacement}); + ctx.Replace(v, replacement); break; } } @@ -415,15 +413,17 @@ Transform::Output VertexPulling::Run(const Program* in, const DataMap& data) { // following stages will pass CloneContext ctx(&out, in); + + // Start by cloning all the symbols. This ensures that the authored symbols + // won't get renamed if they collide with new symbols below. + ctx.CloneSymbols(); + State state{ctx, cfg}; state.FindOrInsertVertexIndexIfUsed(); state.FindOrInsertInstanceIndexIfUsed(); state.ConvertVertexInputVariablesToPrivate(); state.AddVertexStorageBuffers(); - for (auto& replacement : state.location_replacements) { - ctx.Replace(replacement.from, replacement.to); - } ctx.ReplaceAll([&](ast::Function* f) -> ast::Function* { if (f == func) { return CloneWithStatementsAtStart(&ctx, f, diff --git a/src/transform/vertex_pulling_test.cc b/src/transform/vertex_pulling_test.cc index ce01f87ef8..827b6d4bc1 100644 --- a/src/transform/vertex_pulling_test.cc +++ b/src/transform/vertex_pulling_test.cc @@ -81,13 +81,13 @@ fn main() {} auto* expect = R"( [[block]] struct TintVertexData { - _tint_vertex_data : [[stride(4)]] array; + tint_vertex_data : [[stride(4)]] array; }; [[stage(vertex)]] fn main() { { - var _tint_pulling_pos : u32; + var tint_pulling_pos : u32; } } )"; @@ -111,23 +111,23 @@ fn main() {} )"; auto* expect = R"( -[[builtin(vertex_index)]] var _tint_pulling_vertex_index : u32; +[[builtin(vertex_index)]] var tint_pulling_vertex_index : u32; [[block]] struct TintVertexData { - _tint_vertex_data : [[stride(4)]] array; + tint_vertex_data : [[stride(4)]] array; }; -[[binding(0), group(4)]] var _tint_pulling_vertex_buffer_0 : TintVertexData; +[[binding(0), group(4)]] var tint_pulling_vertex_buffer_0 : TintVertexData; var var_a : f32; [[stage(vertex)]] fn main() { { - var _tint_pulling_pos : u32; - _tint_pulling_pos = ((_tint_pulling_vertex_index * 4u) + 0u); - var_a = bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[(_tint_pulling_pos / 4u)]); + var tint_pulling_pos : u32; + tint_pulling_pos = ((tint_pulling_vertex_index * 4u) + 0u); + var_a = bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[(tint_pulling_pos / 4u)]); } } )"; @@ -153,23 +153,23 @@ fn main() {} )"; auto* expect = R"( -[[builtin(instance_index)]] var _tint_pulling_instance_index : u32; +[[builtin(instance_index)]] var tint_pulling_instance_index : u32; [[block]] struct TintVertexData { - _tint_vertex_data : [[stride(4)]] array; + tint_vertex_data : [[stride(4)]] array; }; -[[binding(0), group(4)]] var _tint_pulling_vertex_buffer_0 : TintVertexData; +[[binding(0), group(4)]] var tint_pulling_vertex_buffer_0 : TintVertexData; var var_a : f32; [[stage(vertex)]] fn main() { { - var _tint_pulling_pos : u32; - _tint_pulling_pos = ((_tint_pulling_instance_index * 4u) + 0u); - var_a = bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[(_tint_pulling_pos / 4u)]); + var tint_pulling_pos : u32; + tint_pulling_pos = ((tint_pulling_instance_index * 4u) + 0u); + var_a = bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[(tint_pulling_pos / 4u)]); } } )"; @@ -195,23 +195,23 @@ fn main() {} )"; auto* expect = R"( -[[builtin(vertex_index)]] var _tint_pulling_vertex_index : u32; +[[builtin(vertex_index)]] var tint_pulling_vertex_index : u32; [[block]] struct TintVertexData { - _tint_vertex_data : [[stride(4)]] array; + tint_vertex_data : [[stride(4)]] array; }; -[[binding(0), group(5)]] var _tint_pulling_vertex_buffer_0 : TintVertexData; +[[binding(0), group(5)]] var tint_pulling_vertex_buffer_0 : TintVertexData; var var_a : f32; [[stage(vertex)]] fn main() { { - var _tint_pulling_pos : u32; - _tint_pulling_pos = ((_tint_pulling_vertex_index * 4u) + 0u); - var_a = bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[(_tint_pulling_pos / 4u)]); + var tint_pulling_pos : u32; + tint_pulling_pos = ((tint_pulling_vertex_index * 4u) + 0u); + var_a = bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[(tint_pulling_pos / 4u)]); } } )"; @@ -244,12 +244,12 @@ fn main() {} auto* expect = R"( [[block]] struct TintVertexData { - _tint_vertex_data : [[stride(4)]] array; + tint_vertex_data : [[stride(4)]] array; }; -[[binding(0), group(4)]] var _tint_pulling_vertex_buffer_0 : TintVertexData; +[[binding(0), group(4)]] var tint_pulling_vertex_buffer_0 : TintVertexData; -[[binding(1), group(4)]] var _tint_pulling_vertex_buffer_1 : TintVertexData; +[[binding(1), group(4)]] var tint_pulling_vertex_buffer_1 : TintVertexData; var var_a : f32; @@ -262,11 +262,11 @@ var var_b : f32; [[stage(vertex)]] fn main() { { - var _tint_pulling_pos : u32; - _tint_pulling_pos = ((custom_vertex_index * 4u) + 0u); - var_a = bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[(_tint_pulling_pos / 4u)]); - _tint_pulling_pos = ((custom_instance_index * 4u) + 0u); - var_b = bitcast(_tint_pulling_vertex_buffer_1._tint_vertex_data[(_tint_pulling_pos / 4u)]); + var tint_pulling_pos : u32; + tint_pulling_pos = ((custom_vertex_index * 4u) + 0u); + var_a = bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[(tint_pulling_pos / 4u)]); + tint_pulling_pos = ((custom_instance_index * 4u) + 0u); + var_b = bitcast(tint_pulling_vertex_buffer_1.tint_vertex_data[(tint_pulling_pos / 4u)]); } } )"; @@ -303,14 +303,14 @@ fn main() {} )"; auto* expect = R"( -[[builtin(vertex_index)]] var _tint_pulling_vertex_index : u32; +[[builtin(vertex_index)]] var tint_pulling_vertex_index : u32; [[block]] struct TintVertexData { - _tint_vertex_data : [[stride(4)]] array; + tint_vertex_data : [[stride(4)]] array; }; -[[binding(0), group(4)]] var _tint_pulling_vertex_buffer_0 : TintVertexData; +[[binding(0), group(4)]] var tint_pulling_vertex_buffer_0 : TintVertexData; var var_a : f32; @@ -319,11 +319,11 @@ var var_b : vec4; [[stage(vertex)]] fn main() { { - var _tint_pulling_pos : u32; - _tint_pulling_pos = ((_tint_pulling_vertex_index * 16u) + 0u); - var_a = bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[(_tint_pulling_pos / 4u)]); - _tint_pulling_pos = ((_tint_pulling_vertex_index * 16u) + 0u); - var_b = vec4(bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[((_tint_pulling_pos + 0u) / 4u)]), bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[((_tint_pulling_pos + 4u) / 4u)]), bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[((_tint_pulling_pos + 8u) / 4u)]), bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[((_tint_pulling_pos + 12u) / 4u)])); + var tint_pulling_pos : u32; + tint_pulling_pos = ((tint_pulling_vertex_index * 16u) + 0u); + var_a = bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[(tint_pulling_pos / 4u)]); + tint_pulling_pos = ((tint_pulling_vertex_index * 16u) + 0u); + var_b = vec4(bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[((tint_pulling_pos + 0u) / 4u)]), bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[((tint_pulling_pos + 4u) / 4u)]), bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[((tint_pulling_pos + 8u) / 4u)]), bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[((tint_pulling_pos + 12u) / 4u)])); } } )"; @@ -353,18 +353,18 @@ fn main() {} )"; auto* expect = R"( -[[builtin(vertex_index)]] var _tint_pulling_vertex_index : u32; +[[builtin(vertex_index)]] var tint_pulling_vertex_index : u32; [[block]] struct TintVertexData { - _tint_vertex_data : [[stride(4)]] array; + tint_vertex_data : [[stride(4)]] array; }; -[[binding(0), group(4)]] var _tint_pulling_vertex_buffer_0 : TintVertexData; +[[binding(0), group(4)]] var tint_pulling_vertex_buffer_0 : TintVertexData; -[[binding(1), group(4)]] var _tint_pulling_vertex_buffer_1 : TintVertexData; +[[binding(1), group(4)]] var tint_pulling_vertex_buffer_1 : TintVertexData; -[[binding(2), group(4)]] var _tint_pulling_vertex_buffer_2 : TintVertexData; +[[binding(2), group(4)]] var tint_pulling_vertex_buffer_2 : TintVertexData; var var_a : vec2; @@ -375,13 +375,13 @@ var var_c : vec4; [[stage(vertex)]] fn main() { { - var _tint_pulling_pos : u32; - _tint_pulling_pos = ((_tint_pulling_vertex_index * 8u) + 0u); - var_a = vec2(bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[((_tint_pulling_pos + 0u) / 4u)]), bitcast(_tint_pulling_vertex_buffer_0._tint_vertex_data[((_tint_pulling_pos + 4u) / 4u)])); - _tint_pulling_pos = ((_tint_pulling_vertex_index * 12u) + 0u); - var_b = vec3(bitcast(_tint_pulling_vertex_buffer_1._tint_vertex_data[((_tint_pulling_pos + 0u) / 4u)]), bitcast(_tint_pulling_vertex_buffer_1._tint_vertex_data[((_tint_pulling_pos + 4u) / 4u)]), bitcast(_tint_pulling_vertex_buffer_1._tint_vertex_data[((_tint_pulling_pos + 8u) / 4u)])); - _tint_pulling_pos = ((_tint_pulling_vertex_index * 16u) + 0u); - var_c = vec4(bitcast(_tint_pulling_vertex_buffer_2._tint_vertex_data[((_tint_pulling_pos + 0u) / 4u)]), bitcast(_tint_pulling_vertex_buffer_2._tint_vertex_data[((_tint_pulling_pos + 4u) / 4u)]), bitcast(_tint_pulling_vertex_buffer_2._tint_vertex_data[((_tint_pulling_pos + 8u) / 4u)]), bitcast(_tint_pulling_vertex_buffer_2._tint_vertex_data[((_tint_pulling_pos + 12u) / 4u)])); + var tint_pulling_pos : u32; + tint_pulling_pos = ((tint_pulling_vertex_index * 8u) + 0u); + var_a = vec2(bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[((tint_pulling_pos + 0u) / 4u)]), bitcast(tint_pulling_vertex_buffer_0.tint_vertex_data[((tint_pulling_pos + 4u) / 4u)])); + tint_pulling_pos = ((tint_pulling_vertex_index * 12u) + 0u); + var_b = vec3(bitcast(tint_pulling_vertex_buffer_1.tint_vertex_data[((tint_pulling_pos + 0u) / 4u)]), bitcast(tint_pulling_vertex_buffer_1.tint_vertex_data[((tint_pulling_pos + 4u) / 4u)]), bitcast(tint_pulling_vertex_buffer_1.tint_vertex_data[((tint_pulling_pos + 8u) / 4u)])); + tint_pulling_pos = ((tint_pulling_vertex_index * 16u) + 0u); + var_c = vec4(bitcast(tint_pulling_vertex_buffer_2.tint_vertex_data[((tint_pulling_pos + 0u) / 4u)]), bitcast(tint_pulling_vertex_buffer_2.tint_vertex_data[((tint_pulling_pos + 4u) / 4u)]), bitcast(tint_pulling_vertex_buffer_2.tint_vertex_data[((tint_pulling_pos + 8u) / 4u)]), bitcast(tint_pulling_vertex_buffer_2.tint_vertex_data[((tint_pulling_pos + 12u) / 4u)])); } } )"; @@ -401,6 +401,63 @@ fn main() { EXPECT_EQ(expect, str(got)); } +TEST_F(VertexPullingTest, AttemptSymbolCollision) { + auto* src = R"( +[[location(0)]] var var_a : f32; +[[location(1)]] var var_b : vec4; + +[[stage(vertex)]] +fn main() { + var tint_pulling_vertex_index : i32; + var tint_pulling_vertex_buffer_0 : i32; + var tint_vertex_data : i32; + var tint_pulling_pos : i32; +} +)"; + + auto* expect = R"( +[[builtin(vertex_index)]] var tint_pulling_vertex_index_1 : u32; + +[[block]] +struct TintVertexData { + tint_vertex_data_1 : [[stride(4)]] array; +}; + +[[binding(0), group(4)]] var tint_pulling_vertex_buffer_0_1 : TintVertexData; + +var var_a : f32; + +var var_b : vec4; + +[[stage(vertex)]] +fn main() { + { + var tint_pulling_pos_1 : u32; + tint_pulling_pos_1 = ((tint_pulling_vertex_index_1 * 16u) + 0u); + var_a = bitcast(tint_pulling_vertex_buffer_0_1.tint_vertex_data_1[(tint_pulling_pos_1 / 4u)]); + tint_pulling_pos_1 = ((tint_pulling_vertex_index_1 * 16u) + 0u); + var_b = vec4(bitcast(tint_pulling_vertex_buffer_0_1.tint_vertex_data_1[((tint_pulling_pos_1 + 0u) / 4u)]), bitcast(tint_pulling_vertex_buffer_0_1.tint_vertex_data_1[((tint_pulling_pos_1 + 4u) / 4u)]), bitcast(tint_pulling_vertex_buffer_0_1.tint_vertex_data_1[((tint_pulling_pos_1 + 8u) / 4u)]), bitcast(tint_pulling_vertex_buffer_0_1.tint_vertex_data_1[((tint_pulling_pos_1 + 12u) / 4u)])); + } + var tint_pulling_vertex_index : i32; + var tint_pulling_vertex_buffer_0 : i32; + var tint_vertex_data : i32; + var tint_pulling_pos : i32; +} +)"; + + VertexPulling::Config cfg; + cfg.vertex_state = { + {{16, + InputStepMode::kVertex, + {{VertexFormat::kF32, 0, 0}, {VertexFormat::kVec4F32, 0, 1}}}}}; + cfg.entry_point_name = "main"; + + DataMap data; + data.Add(cfg); + auto got = Run(src, std::move(data)); + + EXPECT_EQ(expect, str(got)); +} } // namespace } // namespace transform } // namespace tint