From 1e0472cdba5aaca5e352ff578c16383df4959432 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Thu, 10 Dec 2020 18:40:01 +0000 Subject: [PATCH] Update namer to use symbol table. This Cl updates the various namer objects to work off the symbol table instead of names. Change-Id: I94b00a10225d0587f037cfaa6d9b42e2a8885734 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35101 Reviewed-by: Ben Clayton Commit-Queue: dan sinclair Auto-Submit: dan sinclair --- src/ast/module.cc | 4 +++ src/ast/module.h | 5 ++++ src/namer.cc | 60 +++++++++++++++++++------------------- src/namer.h | 73 ++++++++++++++++++++++++++++++----------------- src/namer_test.cc | 63 ++++++++++++++++++++-------------------- 5 files changed, 116 insertions(+), 89 deletions(-) diff --git a/src/ast/module.cc b/src/ast/module.cc index 1e458c02a0..5dcc12bd51 100644 --- a/src/ast/module.cc +++ b/src/ast/module.cc @@ -81,6 +81,10 @@ Symbol Module::RegisterSymbol(const std::string& name) { return symbol_table_.Register(name); } +std::string Module::SymbolToName(const Symbol sym) const { + return symbol_table_.NameFor(sym); +} + bool Module::IsValid() const { for (auto* var : global_variables_) { if (var == nullptr || !var->IsValid()) { diff --git a/src/ast/module.h b/src/ast/module.h index 59a67b0d50..b274be110f 100644 --- a/src/ast/module.h +++ b/src/ast/module.h @@ -169,6 +169,11 @@ class Module { /// previously generated symbol will be returned. Symbol RegisterSymbol(const std::string& name); + /// Returns the `name` for `sym` + /// @param sym the symbol to retrieve the name for + /// @returns the use provided `name` for the symbol or "" if not found + std::string SymbolToName(const Symbol sym) const; + private: Module(const Module&) = delete; diff --git a/src/namer.cc b/src/namer.cc index 54c17ad1be..f5f56004ca 100644 --- a/src/namer.cc +++ b/src/namer.cc @@ -18,46 +18,44 @@ #include #include +#include "src/symbol.h" + namespace tint { -Namer::Namer() = default; +Namer::Namer(ast::Module* mod) : module_(mod) {} Namer::~Namer() = default; -bool Namer::IsMapped(const std::string& name) { - auto it = name_map_.find(name); - return it != name_map_.end(); +bool Namer::IsUsed(const std::string& name) { + auto it = used_.find(name); + return it != used_.end(); } -HashingNamer::HashingNamer() = default; - -HashingNamer::~HashingNamer() = default; - -std::string HashingNamer::NameFor(const std::string& name) { - auto it = name_map_.find(name); - if (it != name_map_.end()) { - return it->second; +std::string Namer::GenerateName(const std::string& prefix) { + std::string name = prefix; + uint32_t i = 0; + while (IsUsed(name)) { + name = prefix + "_" + std::to_string(i); + ++i; } - - std::stringstream ret_name; - ret_name << "tint_"; - - ret_name << std::hex << std::setfill('0') << std::setw(2); - for (size_t i = 0; i < name.size(); ++i) { - ret_name << static_cast(name[i]); - } - - name_map_[name] = ret_name.str(); - return ret_name.str(); -} - -NoopNamer::NoopNamer() = default; - -NoopNamer::~NoopNamer() = default; - -std::string NoopNamer::NameFor(const std::string& name) { - name_map_[name] = name; + used_.insert(name); return name; } +MangleNamer::MangleNamer(ast::Module* mod) : Namer(mod) {} + +MangleNamer::~MangleNamer() = default; + +std::string MangleNamer::NameFor(const Symbol& sym) { + return sym.to_str(); +} + +UnsafeNamer::UnsafeNamer(ast::Module* mod) : Namer(mod) {} + +UnsafeNamer::~UnsafeNamer() = default; + +std::string UnsafeNamer::NameFor(const Symbol& sym) { + return module_->SymbolToName(sym); +} + } // namespace tint diff --git a/src/namer.h b/src/namer.h index 28a8de464c..8d05eb2f34 100644 --- a/src/namer.h +++ b/src/namer.h @@ -19,52 +19,73 @@ #include #include +#include "src/ast/module.h" + namespace tint { /// Base class for the namers. class Namer { public: /// Constructor - Namer(); + /// @param mod the module this namer works with + explicit Namer(ast::Module* mod); + /// Destructor virtual ~Namer(); - /// Returns a sanitized version of `name` - /// @param name the name to sanitize - /// @returns the sanitized version of `name` - virtual std::string NameFor(const std::string& name) = 0; + /// Returns the name for `sym` + /// @param sym the symbol to retrieve the name for + /// @returns the sanitized version of `name` or "" if not found + virtual std::string NameFor(const Symbol& sym) = 0; - /// Returns if the given name has been mapped already - /// @param name the name to check - /// @returns true if the name has been mapped - bool IsMapped(const std::string& name); + /// Generates a unique name for `prefix` + /// @param prefix the prefix name + /// @returns the unique name string + std::string GenerateName(const std::string& prefix); protected: - /// Map of original name to new name. - std::unordered_map name_map_; + /// Checks if `name` has been used + /// @param name the name to check + /// @returns true if `name` has already been used + bool IsUsed(const std::string& name); + + /// The module storing the symbol table + ast::Module* module_ = nullptr; + + private: + // The list of names taken by the remapper + std::unordered_set used_; }; -/// A namer class which hashes the name -class HashingNamer : public Namer { +/// A namer class which mangles the name +class MangleNamer : public Namer { public: - HashingNamer(); - ~HashingNamer() override; + /// Constructor + /// @param mod the module to retrieve names from + explicit MangleNamer(ast::Module* mod); + /// Destructor + ~MangleNamer() override; - /// Returns a sanitized version of `name` - /// @param name the name to sanitize - /// @returns the sanitized version of `name` - std::string NameFor(const std::string& name) override; + /// Returns a mangled name for `sym` + /// @param sym the symbol to name + /// @returns the name for `sym` or "" if not found + std::string NameFor(const Symbol& sym) override; }; -/// A namer which just returns the provided string -class NoopNamer : public Namer { +/// A namer which returns the user provided name. This is unsafe in general as +/// it passes user provided data through to the backend compiler. It is useful +/// for development and debugging. +class UnsafeNamer : public Namer { public: - NoopNamer(); - ~NoopNamer() override; + /// Constructor + /// @param mod the module to retrieve names from + explicit UnsafeNamer(ast::Module* mod); + /// Destructor + ~UnsafeNamer() override; /// Returns `name` - /// @param name the name - /// @returns `name` - std::string NameFor(const std::string& name) override; + /// @param sym the symbol + /// @returns `name` or "" if not found + std::string NameFor(const Symbol& sym) override; }; } // namespace tint diff --git a/src/namer_test.cc b/src/namer_test.cc index bc3bbbdd65..5a318f7df0 100644 --- a/src/namer_test.cc +++ b/src/namer_test.cc @@ -15,50 +15,49 @@ #include "src/namer.h" #include "gtest/gtest.h" +#include "src/ast/module.h" namespace tint { namespace { -using Namer_HashingNamer_Test = testing::Test; +using NamerTest = testing::Test; -TEST_F(Namer_HashingNamer_Test, ReturnsName) { - HashingNamer n; - EXPECT_EQ("tint_6d795f6e616d65", n.NameFor("my_name")); +TEST_F(NamerTest, GenerateName) { + ast::Module m; + MangleNamer n(&m); + EXPECT_EQ("name", n.GenerateName("name")); + EXPECT_EQ("name_0", n.GenerateName("name")); + EXPECT_EQ("name_1", n.GenerateName("name")); } -TEST_F(Namer_HashingNamer_Test, ReturnsSameValueForSameName) { - HashingNamer n; - EXPECT_EQ("tint_6e616d6531", n.NameFor("name1")); - EXPECT_EQ("tint_6e616d6532", n.NameFor("name2")); - EXPECT_EQ("tint_6e616d6531", n.NameFor("name1")); +using MangleNamerTest = testing::Test; + +TEST_F(MangleNamerTest, ReturnsName) { + ast::Module m; + auto s = m.RegisterSymbol("my_sym"); + + MangleNamer n(&m); + EXPECT_EQ("tint_symbol_1", n.NameFor(s)); } -TEST_F(Namer_HashingNamer_Test, IsMapped) { - HashingNamer n; - EXPECT_FALSE(n.IsMapped("my_name")); - EXPECT_EQ("tint_6d795f6e616d65", n.NameFor("my_name")); - EXPECT_TRUE(n.IsMapped("my_name")); +TEST_F(MangleNamerTest, ReturnsSameValueForSameName) { + ast::Module m; + auto s1 = m.RegisterSymbol("my_sym"); + auto s2 = m.RegisterSymbol("my_sym2"); + + MangleNamer n(&m); + EXPECT_EQ("tint_symbol_1", n.NameFor(s1)); + EXPECT_EQ("tint_symbol_2", n.NameFor(s2)); + EXPECT_EQ("tint_symbol_1", n.NameFor(s1)); } -using Namer_NoopNamer_Test = testing::Test; +using UnsafeNamerTest = testing::Test; +TEST_F(UnsafeNamerTest, ReturnsName) { + ast::Module m; + auto s = m.RegisterSymbol("my_sym"); -TEST_F(Namer_NoopNamer_Test, ReturnsName) { - NoopNamer n; - EXPECT_EQ("my_name", n.NameFor("my_name")); -} - -TEST_F(Namer_NoopNamer_Test, ReturnsSameValueForSameName) { - NoopNamer n; - EXPECT_EQ("name1", n.NameFor("name1")); - EXPECT_EQ("name2", n.NameFor("name2")); - EXPECT_EQ("name1", n.NameFor("name1")); -} - -TEST_F(Namer_NoopNamer_Test, IsMapped) { - NoopNamer n; - EXPECT_FALSE(n.IsMapped("my_name")); - EXPECT_EQ("my_name", n.NameFor("my_name")); - EXPECT_TRUE(n.IsMapped("my_name")); + UnsafeNamer n(&m); + EXPECT_EQ("my_sym", n.NameFor(s)); } } // namespace