transform/Renamer: Use SymbolTable::New()

Avoids potential symbol collisions. Simplifies the logic.

Bug: tint:712
Change-Id: I4416307b10f2dbe38964b6fd9342042c7e5505ec
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47632
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: James Price <jrprice@google.com>
This commit is contained in:
Ben Clayton 2021-04-13 22:58:07 +00:00 committed by Commit Bot service account
parent 363a2e26e3
commit bd1597a545
2 changed files with 64 additions and 31 deletions

View File

@ -72,15 +72,11 @@ Transform::Output Renamer::Run(const Program* in, const DataMap&) {
switch (cfg_.method) {
case Method::kMonotonic:
ctx.ReplaceAll([&](Symbol sym) {
auto str_in = in->Symbols().NameFor(sym);
auto it = remappings.find(str_in);
if (it != remappings.end()) {
return out.Symbols().Get(it->second);
}
auto str_out = "_tint_" + std::to_string(remappings.size() + 1);
remappings.emplace(str_in, str_out);
return out.Symbols().Register(str_out);
ctx.ReplaceAll([&](Symbol sym_in) {
auto sym_out = ctx.dst->Symbols().New();
remappings.emplace(ctx.src->Symbols().NameFor(sym_in),
ctx.dst->Symbols().NameFor(sym_out));
return sym_out;
});
ctx.ReplaceAll(

View File

@ -53,15 +53,15 @@ fn entry() {
)";
auto* expect = R"(
[[builtin(vertex_index)]] var<in> _tint_1 : u32;
[[builtin(vertex_index)]] var<in> tint_symbol : u32;
fn _tint_2() -> u32 {
return _tint_1;
fn tint_symbol_1() -> u32 {
return tint_symbol;
}
[[stage(vertex)]]
fn _tint_3() {
_tint_2();
fn tint_symbol_2() {
tint_symbol_1();
}
)";
@ -73,9 +73,9 @@ fn _tint_3() {
ASSERT_NE(data, nullptr);
Renamer::Data::Remappings expected_remappings = {
{"vert_idx", "_tint_1"},
{"test", "_tint_2"},
{"entry", "_tint_3"},
{"vert_idx", "tint_symbol"},
{"test", "tint_symbol_1"},
{"entry", "tint_symbol_2"},
};
EXPECT_THAT(data->remappings, ContainerEq(expected_remappings));
}
@ -93,11 +93,11 @@ fn entry() -> [[builtin(position)]] vec4<f32> {
auto* expect = R"(
[[stage(vertex)]]
fn _tint_1() -> [[builtin(position)]] vec4<f32> {
var _tint_2 : vec4<f32>;
var _tint_3 : f32;
var _tint_4 : f32;
return (_tint_2.zyxw + _tint_2.rgab);
fn tint_symbol() -> [[builtin(position)]] vec4<f32> {
var tint_symbol_1 : vec4<f32>;
var tint_symbol_2 : f32;
var tint_symbol_3 : f32;
return (tint_symbol_1.zyxw + tint_symbol_1.rgab);
}
)";
@ -109,10 +109,10 @@ fn _tint_1() -> [[builtin(position)]] vec4<f32> {
ASSERT_NE(data, nullptr);
Renamer::Data::Remappings expected_remappings = {
{"entry", "_tint_1"},
{"v", "_tint_2"},
{"rgba", "_tint_3"},
{"xyzw", "_tint_4"},
{"entry", "tint_symbol"},
{"v", "tint_symbol_1"},
{"rgba", "tint_symbol_2"},
{"xyzw", "tint_symbol_3"},
};
EXPECT_THAT(data->remappings, ContainerEq(expected_remappings));
}
@ -128,9 +128,9 @@ fn entry() -> [[builtin(position)]] vec4<f32> {
auto* expect = R"(
[[stage(vertex)]]
fn _tint_1() -> [[builtin(position)]] vec4<f32> {
var _tint_2 : vec4<f32>;
return abs(_tint_2);
fn tint_symbol() -> [[builtin(position)]] vec4<f32> {
var tint_symbol_1 : vec4<f32>;
return abs(tint_symbol_1);
}
)";
@ -142,8 +142,45 @@ fn _tint_1() -> [[builtin(position)]] vec4<f32> {
ASSERT_NE(data, nullptr);
Renamer::Data::Remappings expected_remappings = {
{"entry", "_tint_1"},
{"blah", "_tint_2"},
{"entry", "tint_symbol"},
{"blah", "tint_symbol_1"},
};
EXPECT_THAT(data->remappings, ContainerEq(expected_remappings));
}
TEST_F(RenamerTest, AttemptSymbolCollision) {
auto* src = R"(
[[stage(vertex)]]
fn entry() -> [[builtin(position)]] vec4<f32> {
var tint_symbol : vec4<f32>;
var tint_symbol_2 : vec4<f32>;
var tint_symbol_4 : vec4<f32>;
return tint_symbol + tint_symbol_2 + tint_symbol_4;
}
)";
auto* expect = R"(
[[stage(vertex)]]
fn tint_symbol() -> [[builtin(position)]] vec4<f32> {
var tint_symbol_1 : vec4<f32>;
var tint_symbol_2 : vec4<f32>;
var tint_symbol_3 : vec4<f32>;
return ((tint_symbol_1 + tint_symbol_2) + tint_symbol_3);
}
)";
auto got = Run<Renamer>(src);
EXPECT_EQ(expect, str(got));
auto* data = got.data.Get<Renamer::Data>();
ASSERT_NE(data, nullptr);
Renamer::Data::Remappings expected_remappings = {
{"entry", "tint_symbol"},
{"tint_symbol", "tint_symbol_1"},
{"tint_symbol_2", "tint_symbol_2"},
{"tint_symbol_4", "tint_symbol_3"},
};
EXPECT_THAT(data->remappings, ContainerEq(expected_remappings));
}