tint/transform: Simplify Renamer transform

Take advantage of the fact that all AST nodes now use an
`ast::Identifier` instead of directly using Symbols.

This will be important for ensuring that un-keyworded identifiers will
be preserved.

Bug: tint:1810
Change-Id: If5c3e9f00b4c1c14a6f11bd617bd8b895250fb7e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/119285
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@chromium.org>
This commit is contained in:
Ben Clayton
2023-02-09 15:35:27 +00:00
committed by Ben Clayton
parent f973d514a1
commit 4e4cada291
20 changed files with 386 additions and 158 deletions

View File

@@ -15,7 +15,6 @@
#include "src/tint/transform/renamer.h"
#include <memory>
#include <unordered_set>
#include <utility>
#include "src/tint/program_builder.h"
@@ -24,7 +23,6 @@
#include "src/tint/sem/type_conversion.h"
#include "src/tint/sem/type_initializer.h"
#include "src/tint/text/unicode.h"
#include "src/tint/type/builtin.h"
TINT_INSTANTIATE_TYPEINFO(tint::transform::Renamer);
TINT_INSTANTIATE_TYPEINFO(tint::transform::Renamer::Data);
@@ -1258,28 +1256,21 @@ Renamer::~Renamer() = default;
Transform::ApplyResult Renamer::Apply(const Program* src,
const DataMap& inputs,
DataMap& outputs) const {
ProgramBuilder b;
CloneContext ctx{&b, src, /* auto_clone_symbols */ false};
utils::Hashset<Symbol, 16> global_decls;
for (auto* decl : src->AST().TypeDecls()) {
global_decls.Add(decl->name->symbol);
}
// Identifiers that need to keep their symbols preserved.
utils::Hashset<const ast::Identifier*, 8> preserved_identifiers;
auto is_type_short_name = [&](const Symbol& symbol) {
auto name = src->Symbols().NameFor(symbol);
if (type::ParseBuiltin(name) != type::Builtin::kUndefined) {
// Identifier *looks* like a builtin type, but check that the builtin type isn't being
// shadowed with a user declared type.
for (auto* decl : src->AST().TypeDecls()) {
if (decl->name->symbol == symbol) {
return false;
}
}
return true;
}
return false;
};
utils::Hashset<const ast::Identifier*, 16> preserved_identifiers;
for (auto* node : src->ASTNodes().Objects()) {
auto preserve_if_builtin_type = [&](const ast::Identifier* ident) {
if (!global_decls.Contains(ident->symbol)) {
preserved_identifiers.Add(ident);
}
};
Switch(
node,
[&](const ast::MemberAccessorExpression* accessor) {
@@ -1294,38 +1285,24 @@ Transform::ApplyResult Renamer::Apply(const Program* src,
}
}
},
[&](const ast::CallExpression* call) {
if (auto* ident = call->target.name) {
Switch(
src->Sem().Get(call)->UnwrapMaterialize()->As<sem::Call>()->Target(),
[&](const sem::Builtin*) { preserved_identifiers.Add(ident); },
[&](const sem::TypeConversion*) {
if (is_type_short_name(ident->symbol)) {
preserved_identifiers.Add(ident);
}
},
[&](const sem::TypeInitializer*) {
if (is_type_short_name(ident->symbol)) {
preserved_identifiers.Add(ident);
}
});
}
},
[&](const ast::DiagnosticAttribute* diagnostic) {
preserved_identifiers.Add(diagnostic->control.rule_name);
},
[&](const ast::DiagnosticDirective* diagnostic) {
preserved_identifiers.Add(diagnostic->control.rule_name);
},
[&](const ast::TypeName* type_name) {
if (is_type_short_name(type_name->name->symbol)) {
preserved_identifiers.Add(type_name->name);
[&](const ast::TypeName* ty) { preserve_if_builtin_type(ty->name); },
[&](const ast::CallExpression* call) {
if (auto* ident = call->target.name) {
Switch(
src->Sem().Get(call)->UnwrapMaterialize()->As<sem::Call>()->Target(),
[&](const sem::Builtin*) { preserved_identifiers.Add(ident); },
[&](const sem::TypeConversion*) { preserve_if_builtin_type(ident); },
[&](const sem::TypeInitializer*) { preserve_if_builtin_type(ident); });
}
});
}
Data::Remappings remappings;
Target target = Target::kAll;
bool preserve_unicode = false;
@@ -1334,62 +1311,70 @@ Transform::ApplyResult Renamer::Apply(const Program* src,
preserve_unicode = cfg->preserve_unicode;
}
ctx.ReplaceAll([&](Symbol sym_in) {
auto name_in = src->Symbols().NameFor(sym_in);
if (preserve_unicode || text::utf8::IsASCII(name_in)) {
switch (target) {
case Target::kAll:
// Always rename.
break;
case Target::kGlslKeywords:
if (!std::binary_search(kReservedKeywordsGLSL,
kReservedKeywordsGLSL +
sizeof(kReservedKeywordsGLSL) / sizeof(const char*),
name_in) &&
name_in.compare(0, 3, "gl_")) {
// No match, just reuse the original name.
return ctx.dst->Symbols().New(name_in);
}
break;
case Target::kHlslKeywords:
if (!std::binary_search(kReservedKeywordsHLSL,
kReservedKeywordsHLSL +
sizeof(kReservedKeywordsHLSL) / sizeof(const char*),
name_in)) {
// No match, just reuse the original name.
return ctx.dst->Symbols().New(name_in);
}
break;
case Target::kMslKeywords:
if (!std::binary_search(kReservedKeywordsMSL,
kReservedKeywordsMSL +
sizeof(kReservedKeywordsMSL) / sizeof(const char*),
name_in)) {
// No match, just reuse the original name.
return ctx.dst->Symbols().New(name_in);
}
break;
}
// Returns true if the symbol should be renamed based on the input configuration settings.
auto should_rename = [&](Symbol symbol) {
if (target == Target::kAll) {
return true;
}
auto name = src->Symbols().NameFor(symbol);
if (!text::utf8::IsASCII(name)) {
// name is non-ascii. All of the backend keywords are ascii, so rename if we're not
// preserving unicode symbols.
return !preserve_unicode;
}
switch (target) {
case Target::kGlslKeywords:
return std::binary_search(kReservedKeywordsGLSL,
kReservedKeywordsGLSL +
sizeof(kReservedKeywordsGLSL) / sizeof(const char*),
name) ||
name.compare(0, 3, "gl_") == 0;
case Target::kHlslKeywords:
return std::binary_search(
kReservedKeywordsHLSL,
kReservedKeywordsHLSL + sizeof(kReservedKeywordsHLSL) / sizeof(const char*),
name);
case Target::kMslKeywords:
return std::binary_search(
kReservedKeywordsMSL,
kReservedKeywordsMSL + sizeof(kReservedKeywordsMSL) / sizeof(const char*),
name);
default:
break;
}
return true;
};
auto sym_out = ctx.dst->Sym();
remappings.emplace(name_in, ctx.dst->Symbols().NameFor(sym_out));
return sym_out;
});
utils::Hashmap<Symbol, Symbol, 32> remappings;
ProgramBuilder b;
CloneContext ctx{&b, src, /* auto_clone_symbols */ false};
ctx.ReplaceAll([&](const ast::Identifier* ident) -> const ast::Identifier* {
if (preserved_identifiers.Contains(ident)) {
auto sym_in = ident->symbol;
auto str = src->Symbols().NameFor(sym_in);
auto sym_out = b.Symbols().Register(str);
return ctx.dst->create<ast::Identifier>(ctx.Clone(ident->source), sym_out);
const auto symbol = ident->symbol;
if (preserved_identifiers.Contains(ident) || !should_rename(symbol)) {
return nullptr; // Preserve symbol
}
return nullptr; // Clone ident. Uses the symbol remapping above.
// Create a replacement for this symbol, if we haven't already.
auto replacement = remappings.GetOrCreate(symbol, [&] { return b.Symbols().New(); });
// Reconstruct the identifier
if (auto* tmpl_ident = ident->As<ast::TemplatedIdentifier>()) {
auto args = ctx.Clone(tmpl_ident->arguments);
return ctx.dst->create<ast::TemplatedIdentifier>(ctx.Clone(ident->source), replacement,
std::move(args));
}
return ctx.dst->create<ast::Identifier>(ctx.Clone(ident->source), replacement);
});
ctx.Clone(); // Must come before the std::move()
ctx.Clone();
outputs.Add<Data>(std::move(remappings));
Data::Remappings out;
for (auto it : remappings) {
out[ctx.src->Symbols().NameFor(it.key)] = ctx.dst->Symbols().NameFor(it.value);
}
outputs.Add<Data>(std::move(out));
return Program(std::move(b));
}

View File

@@ -15,10 +15,13 @@
#include "src/tint/transform/renamer.h"
#include <memory>
#include <unordered_set>
#include <vector>
#include "gmock/gmock.h"
#include "src/tint/transform/test_helper.h"
#include "src/tint/type/builtin.h"
#include "src/tint/type/texel_format.h"
namespace tint::transform {
namespace {
@@ -250,6 +253,30 @@ fn frag_main() {
EXPECT_EQ(expect, str(got));
}
TEST_F(RenamerTest, PreserveUnicodeRenameAll) {
auto src = R"(
@fragment
fn frag_main() {
var )" + std::string(kUnicodeIdentifier) +
R"( : i32;
}
)";
auto expect = R"(
@fragment
fn tint_symbol() {
var tint_symbol_1 : i32;
}
)";
DataMap inputs;
inputs.Add<Renamer::Config>(Renamer::Target::kAll,
/* preserve_unicode */ true);
auto got = Run<Renamer>(src, inputs);
EXPECT_EQ(expect, str(got));
}
TEST_F(RenamerTest, AttemptSymbolCollision) {
auto* src = R"(
@vertex
@@ -287,6 +314,49 @@ fn tint_symbol() -> @builtin(position) vec4<f32> {
EXPECT_THAT(data->remappings, ContainerEq(expected_remappings));
}
TEST_F(RenamerTest, PreserveTexelFormatAndAccess) {
auto src = R"(
@group(0) @binding(0) var texture : texture_storage_2d<rgba8unorm, write>;
fn f() {
var dims = textureDimensions(texture);
}
)";
auto expect = R"(
@group(0) @binding(0) var tint_symbol : texture_storage_2d<rgba8unorm, write>;
fn tint_symbol_1() {
var tint_symbol_2 = textureDimensions(tint_symbol);
}
)";
auto got = Run<Renamer>(src);
EXPECT_EQ(expect, str(got));
}
TEST_F(RenamerTest, PreserveAddressSpace) {
auto src = R"(
var<private> p : i32;
fn f() {
var v = p;
}
)";
auto expect = R"(
var<private> tint_symbol : i32;
fn tint_symbol_1() {
var tint_symbol_2 = tint_symbol;
}
)";
auto got = Run<Renamer>(src);
EXPECT_EQ(expect, str(got));
}
using RenamerTestGlsl = TransformTestWithParam<std::string>;
using RenamerTestHlsl = TransformTestWithParam<std::string>;
using RenamerTestMsl = TransformTestWithParam<std::string>;
@@ -1500,7 +1570,7 @@ INSTANTIATE_TEST_SUITE_P(
// "while" // WGSL reserved keyword
kUnicodeIdentifier));
const char* ExpandBuiltinType(std::string_view name) {
std::string ExpandBuiltinType(std::string_view name) {
if (name == "mat2x2f") {
return "mat2x2<f32>";
}
@@ -1591,13 +1661,47 @@ const char* ExpandBuiltinType(std::string_view name) {
if (name == "vec4u") {
return "vec4<u32>";
}
ADD_FAILURE() << "unhandled type: " << name;
return "<invalid>";
return std::string(name);
}
using RenamerTypeBuiltinTypesTest = TransformTestWithParam<const char*>;
/// @return all the identifiers parsed as keywords
std::unordered_set<std::string> Keywords() {
return {
"bool",
"f16",
"f32",
"i32",
"sampler_comparison",
"sampler",
"texture_depth_2d_array",
"texture_depth_2d",
"texture_depth_cube_array",
"texture_depth_cube",
"texture_depth_multisampled_2d",
"texture_external",
"texture_storage_1d",
"texture_storage_2d",
"texture_storage_2d_array",
"texture_storage_3d",
"u32",
};
}
TEST_P(RenamerTypeBuiltinTypesTest, PreserveTypeUsage) {
/// @return WGSL builtin types that aren't keywords
std::vector<const char*> NonKeywordBuiltinTypes() {
auto keywords = Keywords();
std::vector<const char*> out;
for (auto* ident : type::kBuiltinStrings) {
if (!keywords.count(ident)) {
out.push_back(ident);
}
}
return out;
}
using RenamerBuiltinTypeTest = TransformTestWithParam<const char*>;
TEST_P(RenamerBuiltinTypeTest, PreserveTypeUsage) {
auto expand = [&](const char* source) {
auto out = utils::ReplaceAll(source, "$name", GetParam());
out = utils::ReplaceAll(out, "$type", ExpandBuiltinType(GetParam()));
@@ -1638,7 +1742,7 @@ struct tint_symbol_5 {
EXPECT_EQ(expect, str(got));
}
TEST_P(RenamerTypeBuiltinTypesTest, PreserveTypeInitializer) {
TEST_P(RenamerBuiltinTypeTest, PreserveTypeInitializer) {
auto expand = [&](const char* source) {
auto out = utils::ReplaceAll(source, "$name", GetParam());
out = utils::ReplaceAll(out, "$type", ExpandBuiltinType(GetParam()));
@@ -1668,7 +1772,7 @@ fn tint_symbol() {
EXPECT_EQ(expect, str(got));
}
TEST_P(RenamerTypeBuiltinTypesTest, PreserveTypeConversion) {
TEST_P(RenamerBuiltinTypeTest, PreserveTypeConversion) {
auto expand = [&](const char* source) {
auto out = utils::ReplaceAll(source, "$name", GetParam());
out = utils::ReplaceAll(out, "$type", ExpandBuiltinType(GetParam()));
@@ -1698,7 +1802,7 @@ fn tint_symbol() {
EXPECT_EQ(expect, str(got));
}
TEST_P(RenamerTypeBuiltinTypesTest, RenameShadowedByAlias) {
TEST_P(RenamerBuiltinTypeTest, RenameShadowedByAlias) {
auto expand = [&](const char* source) {
auto out = utils::ReplaceAll(source, "$name", GetParam());
out = utils::ReplaceAll(out, "$type", ExpandBuiltinType(GetParam()));
@@ -1728,7 +1832,7 @@ fn tint_symbol_1() {
EXPECT_EQ(expect, str(got));
}
TEST_P(RenamerTypeBuiltinTypesTest, RenameShadowedByStruct) {
TEST_P(RenamerBuiltinTypeTest, RenameShadowedByStruct) {
auto expand = [&](const char* source) {
auto out = utils::ReplaceAll(source, "$name", GetParam());
out = utils::ReplaceAll(out, "$type", ExpandBuiltinType(GetParam()));
@@ -1764,9 +1868,148 @@ fn tint_symbol_2() {
EXPECT_EQ(expect, str(got));
}
INSTANTIATE_TEST_SUITE_P(RenamerTypeBuiltinTypesTest,
RenamerTypeBuiltinTypesTest,
testing::ValuesIn(type::kBuiltinStrings));
INSTANTIATE_TEST_SUITE_P(RenamerBuiltinTypeTest,
RenamerBuiltinTypeTest,
testing::ValuesIn(NonKeywordBuiltinTypes()));
/// @return WGSL builtin identifiers keywords
std::vector<const char*> NonKeywordIdentifiers() {
auto keywords = Keywords();
std::vector<const char*> out;
for (auto* ident : type::kBuiltinStrings) {
if (!keywords.count(ident)) {
out.push_back(ident);
}
}
for (auto* ident : type::kAddressSpaceStrings) {
if (!keywords.count(ident)) {
out.push_back(ident);
}
}
for (auto* ident : type::kTexelFormatStrings) {
if (!keywords.count(ident)) {
out.push_back(ident);
}
}
for (auto* ident : type::kAccessStrings) {
if (!keywords.count(ident)) {
out.push_back(ident);
}
}
return out;
}
using RenamerBuiltinIdentifierTest = TransformTestWithParam<const char*>;
TEST_P(RenamerBuiltinIdentifierTest, GlobalVarName) {
auto expand = [&](const char* source) {
return utils::ReplaceAll(source, "$name", GetParam());
};
auto src = expand(R"(
var<private> $name = 42;
fn f() {
var v = $name;
}
)");
auto expect = expand(R"(
var<private> tint_symbol = 42;
fn tint_symbol_1() {
var tint_symbol_2 = tint_symbol;
}
)");
auto got = Run<Renamer>(src);
EXPECT_EQ(expect, str(got));
}
TEST_P(RenamerBuiltinIdentifierTest, LocalVarName) {
auto expand = [&](const char* source) {
return utils::ReplaceAll(source, "$name", GetParam());
};
auto src = expand(R"(
fn f() {
var $name = 42;
}
)");
auto expect = expand(R"(
fn tint_symbol() {
var tint_symbol_1 = 42;
}
)");
auto got = Run<Renamer>(src);
EXPECT_EQ(expect, str(got));
}
TEST_P(RenamerBuiltinIdentifierTest, FunctionName) {
auto expand = [&](const char* source) {
return utils::ReplaceAll(source, "$name", GetParam());
};
auto src = expand(R"(
fn $name() {
}
fn f() {
$name();
}
)");
auto expect = expand(R"(
fn tint_symbol() {
}
fn tint_symbol_1() {
tint_symbol();
}
)");
auto got = Run<Renamer>(src);
EXPECT_EQ(expect, str(got));
}
TEST_P(RenamerBuiltinIdentifierTest, StructName) {
auto expand = [&](const char* source) {
return utils::ReplaceAll(source, "$name", GetParam());
};
auto src = expand(R"(
struct $name {
i : i32,
}
fn f() {
var x = $name();
}
)");
auto expect = expand(R"(
struct tint_symbol {
tint_symbol_1 : i32,
}
fn tint_symbol_2() {
var tint_symbol_3 = tint_symbol();
}
)");
auto got = Run<Renamer>(src);
EXPECT_EQ(expect, str(got));
}
INSTANTIATE_TEST_SUITE_P(RenamerBuiltinIdentifierTest,
RenamerBuiltinIdentifierTest,
testing::ValuesIn(NonKeywordIdentifiers()));
} // namespace
} // namespace tint::transform