From 281b602f59ea9a5a20f051dc5627a4752c7f66a1 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Thu, 21 Jan 2021 16:35:10 +0000 Subject: [PATCH] type::Manager: Simplify interface and use BlockAllocator Internally use BlockAllocator to allocate the types. When we optimize the allocation patterns of BlockAllocator, this will now benefit both AST nodes and types. Remove Reset(). It was not used. Remove type::Manager::Get(std::unique_ptr) - this was used (via Module::unique_type) in one place, which has easily been migrated to using the standard Module::create<>. Replace all remaining uses of std::unique_ptr<> of types in tests with the standard create<> so we can guarantee uniqueness of the types. Change-Id: Ib0e1fe94e492b31816450df5de0c839a0aefcb9e Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38362 Commit-Queue: Ben Clayton Reviewed-by: dan sinclair --- fuzzers/tint_ast_clone_fuzzer.cc | 4 +- src/ast/module.h | 14 +-- src/ast/module_clone_test.cc | 4 +- src/inspector/inspector_test.cc | 165 +++++++++++++++---------------- src/reader/wgsl/parser_impl.cc | 7 +- src/reader/wgsl/parser_impl.h | 2 +- src/type/type_manager.cc | 13 --- src/type/type_manager.h | 38 ++++--- src/type/type_manager_test.cc | 23 +---- src/type_determiner.cc | 3 +- src/type_determiner_test.cc | 37 ++++--- 11 files changed, 138 insertions(+), 172 deletions(-) diff --git a/fuzzers/tint_ast_clone_fuzzer.cc b/fuzzers/tint_ast_clone_fuzzer.cc index 0ccf481694..6a898f1a3c 100644 --- a/fuzzers/tint_ast_clone_fuzzer.cc +++ b/fuzzers/tint_ast_clone_fuzzer.cc @@ -69,13 +69,13 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { } std::unordered_set src_types; for (auto& src_type : src.types()) { - src_types.emplace(src_type.second.get()); + src_types.emplace(src_type.second); } for (auto* dst_node : dst.nodes()) { ASSERT_EQ(src_nodes.count(dst_node), 0u); } for (auto& dst_type : dst.types()) { - ASSERT_EQ(src_types.count(dst_type.second.get()), 0u); + ASSERT_EQ(src_types.count(dst_type.second), 0u); } // Regenerate the wgsl for the src module. We use this instead of the original diff --git a/src/ast/module.h b/src/ast/module.h index 59858f9640..798a761900 100644 --- a/src/ast/module.h +++ b/src/ast/module.h @@ -135,21 +135,9 @@ class Module { return type_mgr_.Get(std::forward(args)...); } - /// Moves the type `ty` to the Module, returning a pointer to the unique - /// (de-aliased) type. - /// When the Module is destructed, the returned `Type` will also be - /// destructed. - /// @see create() - /// @param ty the type to add to the module - /// @returns the de-aliased type pointer - template - traits::EnableIfIsType* unique_type(std::unique_ptr ty) { - return static_cast(type_mgr_.Get(std::move(ty))); - } - /// Returns all the declared types in the module /// @returns the mapping from name string to type. - const std::unordered_map>& types() { + const std::unordered_map& types() { return type_mgr_.types(); } diff --git a/src/ast/module_clone_test.cc b/src/ast/module_clone_test.cc index e5007b0713..f855fcb2eb 100644 --- a/src/ast/module_clone_test.cc +++ b/src/ast/module_clone_test.cc @@ -128,13 +128,13 @@ fn main() -> void { } std::unordered_set src_types; for (auto& src_type : src.types()) { - src_types.emplace(src_type.second.get()); + src_types.emplace(src_type.second); } for (auto* dst_node : dst.nodes()) { ASSERT_EQ(src_nodes.count(dst_node), 0u) << dst_node->str(); } for (auto& dst_type : dst.types()) { - ASSERT_EQ(src_types.count(dst_type.second.get()), 0u) + ASSERT_EQ(src_types.count(dst_type.second), 0u) << dst_type.second->type_name(); } diff --git a/src/inspector/inspector_test.cc b/src/inspector/inspector_test.cc index ae64c86839..bd9ad5d0b6 100644 --- a/src/inspector/inspector_test.cc +++ b/src/inspector/inspector_test.cc @@ -275,13 +275,12 @@ class InspectorHelper : public ast::BuilderWithModule { /// @returns a tuple {struct type, access control type}, where the struct has /// the layout for an uniform buffer, and the control type wraps the /// struct. - std::tuple> - MakeUniformBufferTypes( + std::tuple MakeUniformBufferTypes( const std::string& name, std::vector> members_info) { auto* struct_type = MakeStructType(name, members_info, true); - auto access_type = std::make_unique( - ast::AccessControl::kReadOnly, struct_type); + auto* access_type = + create(ast::AccessControl::kReadOnly, struct_type); return {struct_type, std::move(access_type)}; } @@ -292,12 +291,11 @@ class InspectorHelper : public ast::BuilderWithModule { /// @returns a tuple {struct type, access control type}, where the struct has /// the layout for a storage buffer, and the control type wraps the /// struct. - std::tuple> - MakeStorageBufferTypes( + std::tuple MakeStorageBufferTypes( const std::string& name, std::vector> members_info) { auto* struct_type = MakeStructType(name, members_info, false); - auto access_type = std::make_unique( + auto* access_type = create( ast::AccessControl::kReadWrite, struct_type); return {struct_type, std::move(access_type)}; } @@ -309,13 +307,13 @@ class InspectorHelper : public ast::BuilderWithModule { /// @returns a tuple {struct type, access control type}, where the struct has /// the layout for a read-only storage buffer, and the control type /// wraps the struct. - std::tuple> + std::tuple MakeReadOnlyStorageBufferTypes( const std::string& name, std::vector> members_info) { auto* struct_type = MakeStructType(name, members_info, false); - auto access_type = std::make_unique( - ast::AccessControl::kReadOnly, struct_type); + auto* access_type = + create(ast::AccessControl::kReadOnly, struct_type); return {struct_type, std::move(access_type)}; } @@ -424,28 +422,26 @@ class InspectorHelper : public ast::BuilderWithModule { /// @param dim the dimensions of the texture /// @param type the data type of the sampled texture /// @returns the generated SampleTextureType - std::unique_ptr MakeSampledTextureType( - type::TextureDimension dim, - type::Type* type) { - return std::make_unique(dim, type); + type::SampledTexture* MakeSampledTextureType(type::TextureDimension dim, + type::Type* type) { + return create(dim, type); } /// Generates a DepthTexture appropriate for the params /// @param dim the dimensions of the texture /// @returns the generated DepthTexture - std::unique_ptr MakeDepthTextureType( - type::TextureDimension dim) { - return std::make_unique(dim); + type::DepthTexture* MakeDepthTextureType(type::TextureDimension dim) { + return create(dim); } /// Generates a MultisampledTexture appropriate for the params /// @param dim the dimensions of the texture /// @param type the data type of the sampled texture /// @returns the generated SampleTextureType - std::unique_ptr MakeMultisampledTextureType( + type::MultisampledTexture* MakeMultisampledTextureType( type::TextureDimension dim, type::Type* type) { - return std::make_unique(dim, type); + return create(dim, type); } /// Adds a sampled texture variable to the module @@ -632,9 +628,9 @@ class InspectorHelper : public ast::BuilderWithModule { if (vector_type_memo_.find(std::tie(type, count)) == vector_type_memo_.end()) { vector_type_memo_[std::tie(type, count)] = - std::make_unique(ty.u32, count); + create(ty.u32, count); } - return vector_type_memo_[std::tie(type, count)].get(); + return vector_type_memo_[std::tie(type, count)]; } type::Sampler* sampler_type() { return &sampler_type_; } type::Sampler* comparison_sampler_type() { return &comparison_sampler_type_; } @@ -646,8 +642,7 @@ class InspectorHelper : public ast::BuilderWithModule { type::Sampler sampler_type_; type::Sampler comparison_sampler_type_; std::map array_type_memo_; - std::map, std::unique_ptr> - vector_type_memo_; + std::map, type::Vector*> vector_type_memo_; }; class InspectorGetEntryPointTest : public InspectorHelper, @@ -1282,10 +1277,10 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, MissingEntryPoint) { TEST_F(InspectorGetUniformBufferResourceBindingsTest, NonEntryPointFunc) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeUniformBufferTypes("foo_type", {{ty.i32, 0}}); - AddUniformBuffer("foo_ub", foo_control_type.get(), 0, 0); + AddUniformBuffer("foo_ub", foo_control_type, 0, 0); auto* ub_func = MakeStructVariableReferenceBodyFunction("ub_func", "foo_ub", {{0, ty.i32}}); @@ -1335,10 +1330,10 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, MissingBlockDeco) { TEST_F(InspectorGetUniformBufferResourceBindingsTest, Simple) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeUniformBufferTypes("foo_type", {{ty.i32, 0}}); - AddUniformBuffer("foo_ub", foo_control_type.get(), 0, 0); + AddUniformBuffer("foo_ub", foo_control_type, 0, 0); auto* ub_func = MakeStructVariableReferenceBodyFunction("ub_func", "foo_ub", {{0, ty.i32}}); @@ -1364,10 +1359,10 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, Simple) { TEST_F(InspectorGetUniformBufferResourceBindingsTest, MultipleMembers) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeUniformBufferTypes( "foo_type", {{ty.i32, 0}, {ty.u32, 4}, {ty.f32, 8}}); - AddUniformBuffer("foo_ub", foo_control_type.get(), 0, 0); + AddUniformBuffer("foo_ub", foo_control_type, 0, 0); auto* ub_func = MakeStructVariableReferenceBodyFunction( "ub_func", "foo_ub", {{0, ty.i32}, {1, ty.u32}, {2, ty.f32}}); @@ -1393,12 +1388,12 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, MultipleMembers) { TEST_F(InspectorGetUniformBufferResourceBindingsTest, MultipleUniformBuffers) { type::Struct* ub_struct_type; - std::unique_ptr ub_control_type; + type::AccessControl* ub_control_type; std::tie(ub_struct_type, ub_control_type) = MakeUniformBufferTypes( "ub_type", {{ty.i32, 0}, {ty.u32, 4}, {ty.f32, 8}}); - AddUniformBuffer("ub_foo", ub_control_type.get(), 0, 0); - AddUniformBuffer("ub_bar", ub_control_type.get(), 0, 1); - AddUniformBuffer("ub_baz", ub_control_type.get(), 2, 0); + AddUniformBuffer("ub_foo", ub_control_type, 0, 0); + AddUniformBuffer("ub_bar", ub_control_type, 0, 1); + AddUniformBuffer("ub_baz", ub_control_type, 2, 0); auto AddReferenceFunc = [this](const std::string& func_name, const std::string& var_name) { @@ -1445,10 +1440,10 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, MultipleUniformBuffers) { TEST_F(InspectorGetUniformBufferResourceBindingsTest, ContainingArray) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeUniformBufferTypes("foo_type", {{ty.i32, 0}, {u32_array_type(4), 4}}); - AddUniformBuffer("foo_ub", foo_control_type.get(), 0, 0); + AddUniformBuffer("foo_ub", foo_control_type, 0, 0); auto* ub_func = MakeStructVariableReferenceBodyFunction("ub_func", "foo_ub", {{0, ty.i32}}); @@ -1474,10 +1469,10 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, ContainingArray) { TEST_F(InspectorGetStorageBufferResourceBindingsTest, Simple) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeStorageBufferTypes("foo_type", {{ty.i32, 0}}); - AddStorageBuffer("foo_sb", foo_control_type.get(), 0, 0); + AddStorageBuffer("foo_sb", foo_control_type, 0, 0); auto* sb_func = MakeStructVariableReferenceBodyFunction("sb_func", "foo_sb", {{0, ty.i32}}); @@ -1503,10 +1498,10 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, Simple) { TEST_F(InspectorGetStorageBufferResourceBindingsTest, MultipleMembers) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeStorageBufferTypes( "foo_type", {{ty.i32, 0}, {ty.u32, 4}, {ty.f32, 8}}); - AddStorageBuffer("foo_sb", foo_control_type.get(), 0, 0); + AddStorageBuffer("foo_sb", foo_control_type, 0, 0); auto* sb_func = MakeStructVariableReferenceBodyFunction( "sb_func", "foo_sb", {{0, ty.i32}, {1, ty.u32}, {2, ty.f32}}); @@ -1532,12 +1527,12 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, MultipleMembers) { TEST_F(InspectorGetStorageBufferResourceBindingsTest, MultipleStorageBuffers) { type::Struct* sb_struct_type; - std::unique_ptr sb_control_type; + type::AccessControl* sb_control_type; std::tie(sb_struct_type, sb_control_type) = MakeStorageBufferTypes( "sb_type", {{ty.i32, 0}, {ty.u32, 4}, {ty.f32, 8}}); - AddStorageBuffer("sb_foo", sb_control_type.get(), 0, 0); - AddStorageBuffer("sb_bar", sb_control_type.get(), 0, 1); - AddStorageBuffer("sb_baz", sb_control_type.get(), 2, 0); + AddStorageBuffer("sb_foo", sb_control_type, 0, 0); + AddStorageBuffer("sb_bar", sb_control_type, 0, 1); + AddStorageBuffer("sb_baz", sb_control_type, 2, 0); auto AddReferenceFunc = [this](const std::string& func_name, const std::string& var_name) { @@ -1587,10 +1582,10 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, MultipleStorageBuffers) { TEST_F(InspectorGetStorageBufferResourceBindingsTest, ContainingArray) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeStorageBufferTypes("foo_type", {{ty.i32, 0}, {u32_array_type(4), 4}}); - AddStorageBuffer("foo_sb", foo_control_type.get(), 0, 0); + AddStorageBuffer("foo_sb", foo_control_type, 0, 0); auto* sb_func = MakeStructVariableReferenceBodyFunction("sb_func", "foo_sb", {{0, ty.i32}}); @@ -1616,10 +1611,10 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, ContainingArray) { TEST_F(InspectorGetStorageBufferResourceBindingsTest, ContainingRuntimeArray) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeStorageBufferTypes("foo_type", {{ty.i32, 0}, {u32_array_type(0), 4}}); - AddStorageBuffer("foo_sb", foo_control_type.get(), 0, 0); + AddStorageBuffer("foo_sb", foo_control_type, 0, 0); auto* sb_func = MakeStructVariableReferenceBodyFunction("sb_func", "foo_sb", {{0, ty.i32}}); @@ -1645,10 +1640,10 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, ContainingRuntimeArray) { TEST_F(InspectorGetStorageBufferResourceBindingsTest, SkipReadOnly) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeReadOnlyStorageBufferTypes("foo_type", {{ty.i32, 0}}); - AddStorageBuffer("foo_sb", foo_control_type.get(), 0, 0); + AddStorageBuffer("foo_sb", foo_control_type, 0, 0); auto* sb_func = MakeStructVariableReferenceBodyFunction("sb_func", "foo_sb", {{0, ty.i32}}); @@ -1670,10 +1665,10 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, SkipReadOnly) { TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, Simple) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeReadOnlyStorageBufferTypes("foo_type", {{ty.i32, 0}}); - AddStorageBuffer("foo_sb", foo_control_type.get(), 0, 0); + AddStorageBuffer("foo_sb", foo_control_type, 0, 0); auto* sb_func = MakeStructVariableReferenceBodyFunction("sb_func", "foo_sb", {{0, ty.i32}}); @@ -1701,12 +1696,12 @@ TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, Simple) { TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, MultipleStorageBuffers) { type::Struct* sb_struct_type; - std::unique_ptr sb_control_type; + type::AccessControl* sb_control_type; std::tie(sb_struct_type, sb_control_type) = MakeReadOnlyStorageBufferTypes( "sb_type", {{ty.i32, 0}, {ty.u32, 4}, {ty.f32, 8}}); - AddStorageBuffer("sb_foo", sb_control_type.get(), 0, 0); - AddStorageBuffer("sb_bar", sb_control_type.get(), 0, 1); - AddStorageBuffer("sb_baz", sb_control_type.get(), 2, 0); + AddStorageBuffer("sb_foo", sb_control_type, 0, 0); + AddStorageBuffer("sb_bar", sb_control_type, 0, 1); + AddStorageBuffer("sb_baz", sb_control_type, 2, 0); auto AddReferenceFunc = [this](const std::string& func_name, const std::string& var_name) { @@ -1757,10 +1752,10 @@ TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, ContainingArray) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeReadOnlyStorageBufferTypes( "foo_type", {{ty.i32, 0}, {u32_array_type(4), 4}}); - AddStorageBuffer("foo_sb", foo_control_type.get(), 0, 0); + AddStorageBuffer("foo_sb", foo_control_type, 0, 0); auto* sb_func = MakeStructVariableReferenceBodyFunction("sb_func", "foo_sb", {{0, ty.i32}}); @@ -1788,10 +1783,10 @@ TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, ContainingArray) { TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, ContainingRuntimeArray) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeReadOnlyStorageBufferTypes( "foo_type", {{ty.i32, 0}, {u32_array_type(0), 4}}); - AddStorageBuffer("foo_sb", foo_control_type.get(), 0, 0); + AddStorageBuffer("foo_sb", foo_control_type, 0, 0); auto* sb_func = MakeStructVariableReferenceBodyFunction("sb_func", "foo_sb", {{0, ty.i32}}); @@ -1818,10 +1813,10 @@ TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, SkipNonReadOnly) { type::Struct* foo_struct_type; - std::unique_ptr foo_control_type; + type::AccessControl* foo_control_type; std::tie(foo_struct_type, foo_control_type) = MakeStorageBufferTypes("foo_type", {{ty.i32, 0}}); - AddStorageBuffer("foo_sb", foo_control_type.get(), 0, 0); + AddStorageBuffer("foo_sb", foo_control_type, 0, 0); auto* sb_func = MakeStructVariableReferenceBodyFunction("sb_func", "foo_sb", {{0, ty.i32}}); @@ -1843,9 +1838,9 @@ TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, SkipNonReadOnly) { } TEST_F(InspectorGetSamplerResourceBindingsTest, Simple) { - auto sampled_texture_type = + auto* sampled_texture_type = MakeSampledTextureType(type::TextureDimension::k1d, ty.f32); - AddSampledTexture("foo_texture", sampled_texture_type.get(), 0, 0); + AddSampledTexture("foo_texture", sampled_texture_type, 0, 0); AddSampler("foo_sampler", 0, 1); AddGlobalVariable("foo_coords", ty.f32); @@ -1882,9 +1877,9 @@ TEST_F(InspectorGetSamplerResourceBindingsTest, NoSampler) { } TEST_F(InspectorGetSamplerResourceBindingsTest, InFunction) { - auto sampled_texture_type = + auto* sampled_texture_type = MakeSampledTextureType(type::TextureDimension::k1d, ty.f32); - AddSampledTexture("foo_texture", sampled_texture_type.get(), 0, 0); + AddSampledTexture("foo_texture", sampled_texture_type, 0, 0); AddSampler("foo_sampler", 0, 1); AddGlobalVariable("foo_coords", ty.f32); @@ -1910,9 +1905,9 @@ TEST_F(InspectorGetSamplerResourceBindingsTest, InFunction) { } TEST_F(InspectorGetSamplerResourceBindingsTest, UnknownEntryPoint) { - auto sampled_texture_type = + auto* sampled_texture_type = MakeSampledTextureType(type::TextureDimension::k1d, ty.f32); - AddSampledTexture("foo_texture", sampled_texture_type.get(), 0, 0); + AddSampledTexture("foo_texture", sampled_texture_type, 0, 0); AddSampler("foo_sampler", 0, 1); AddGlobalVariable("foo_coords", ty.f32); @@ -1930,8 +1925,8 @@ TEST_F(InspectorGetSamplerResourceBindingsTest, UnknownEntryPoint) { } TEST_F(InspectorGetSamplerResourceBindingsTest, SkipsComparisonSamplers) { - auto depth_texture_type = MakeDepthTextureType(type::TextureDimension::k2d); - AddDepthTexture("foo_texture", depth_texture_type.get()); + auto* depth_texture_type = MakeDepthTextureType(type::TextureDimension::k2d); + AddDepthTexture("foo_texture", depth_texture_type); AddComparisonSampler("foo_sampler", 0, 1); AddGlobalVariable("foo_coords", ty.f32); AddGlobalVariable("foo_depth", ty.f32); @@ -1952,8 +1947,8 @@ TEST_F(InspectorGetSamplerResourceBindingsTest, SkipsComparisonSamplers) { } TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, Simple) { - auto depth_texture_type = MakeDepthTextureType(type::TextureDimension::k2d); - AddDepthTexture("foo_texture", depth_texture_type.get()); + auto* depth_texture_type = MakeDepthTextureType(type::TextureDimension::k2d); + AddDepthTexture("foo_texture", depth_texture_type); AddComparisonSampler("foo_sampler", 0, 1); AddGlobalVariable("foo_coords", ty.f32); AddGlobalVariable("foo_depth", ty.f32); @@ -1991,8 +1986,8 @@ TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, NoSampler) { } TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, InFunction) { - auto depth_texture_type = MakeDepthTextureType(type::TextureDimension::k2d); - AddDepthTexture("foo_texture", depth_texture_type.get()); + auto* depth_texture_type = MakeDepthTextureType(type::TextureDimension::k2d); + AddDepthTexture("foo_texture", depth_texture_type); AddComparisonSampler("foo_sampler", 0, 1); AddGlobalVariable("foo_coords", ty.f32); AddGlobalVariable("foo_depth", ty.f32); @@ -2020,8 +2015,8 @@ TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, InFunction) { } TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, UnknownEntryPoint) { - auto depth_texture_type = MakeDepthTextureType(type::TextureDimension::k2d); - AddDepthTexture("foo_texture", depth_texture_type.get()); + auto* depth_texture_type = MakeDepthTextureType(type::TextureDimension::k2d); + AddDepthTexture("foo_texture", depth_texture_type); AddComparisonSampler("foo_sampler", 0, 1); AddGlobalVariable("foo_coords", ty.f32); AddGlobalVariable("foo_depth", ty.f32); @@ -2040,9 +2035,9 @@ TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, UnknownEntryPoint) { } TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, SkipsSamplers) { - auto sampled_texture_type = + auto* sampled_texture_type = MakeSampledTextureType(type::TextureDimension::k1d, ty.f32); - AddSampledTexture("foo_texture", sampled_texture_type.get(), 0, 0); + AddSampledTexture("foo_texture", sampled_texture_type, 0, 0); AddSampler("foo_sampler", 0, 1); AddGlobalVariable("foo_coords", ty.f32); @@ -2075,9 +2070,9 @@ TEST_F(InspectorGetSampledTextureResourceBindingsTest, Empty) { } TEST_P(InspectorGetSampledTextureResourceBindingsTestWithParam, textureSample) { - auto sampled_texture_type = MakeSampledTextureType( + auto* sampled_texture_type = MakeSampledTextureType( GetParam().type_dim, GetBaseType(GetParam().sampled_kind)); - AddSampledTexture("foo_texture", sampled_texture_type.get(), 0, 0); + AddSampledTexture("foo_texture", sampled_texture_type, 0, 0); AddSampler("foo_sampler", 0, 1); auto* coord_type = GetCoordsType(GetParam().type_dim, GetParam().sampled_kind); @@ -2165,9 +2160,9 @@ INSTANTIATE_TEST_SUITE_P( TEST_P(InspectorGetSampledArrayTextureResourceBindingsTestWithParam, textureSample) { - auto sampled_texture_type = MakeSampledTextureType( + auto* sampled_texture_type = MakeSampledTextureType( GetParam().type_dim, GetBaseType(GetParam().sampled_kind)); - AddSampledTexture("foo_texture", sampled_texture_type.get(), 0, 0); + AddSampledTexture("foo_texture", sampled_texture_type, 0, 0); AddSampler("foo_sampler", 0, 1); auto* coord_type = GetCoordsType(GetParam().type_dim, GetParam().sampled_kind); @@ -2237,9 +2232,9 @@ INSTANTIATE_TEST_SUITE_P( TEST_P(InspectorGetMultisampledTextureResourceBindingsTestWithParam, textureSample) { - auto multisampled_texture_type = MakeMultisampledTextureType( + auto* multisampled_texture_type = MakeMultisampledTextureType( GetParam().type_dim, GetBaseType(GetParam().sampled_kind)); - AddMultisampledTexture("foo_texture", multisampled_texture_type.get(), 0, 0); + AddMultisampledTexture("foo_texture", multisampled_texture_type, 0, 0); AddSampler("foo_sampler", 0, 1); auto* coord_type = GetCoordsType(GetParam().type_dim, GetParam().sampled_kind); @@ -2316,9 +2311,9 @@ TEST_F(InspectorGetMultisampledArrayTextureResourceBindingsTest, Empty) { TEST_P(InspectorGetMultisampledArrayTextureResourceBindingsTestWithParam, textureSample) { - auto multisampled_texture_type = MakeMultisampledTextureType( + auto* multisampled_texture_type = MakeMultisampledTextureType( GetParam().type_dim, GetBaseType(GetParam().sampled_kind)); - AddMultisampledTexture("foo_texture", multisampled_texture_type.get(), 0, 0); + AddMultisampledTexture("foo_texture", multisampled_texture_type, 0, 0); AddSampler("foo_sampler", 0, 1); auto* coord_type = GetCoordsType(GetParam().type_dim, GetParam().sampled_kind); diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index 3d56aaf04c..7816725d9f 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -359,7 +359,7 @@ Expect ParserImpl::expect_global_decl() { if (!expect("struct declaration", Token::Type::kSemicolon)) return Failure::kErrored; - auto* type = module_.unique_type(std::move(str.value)); + auto* type = str.value; register_constructed( module_.SymbolToName(type->As()->symbol()), type); module_.AddConstructedType(type); @@ -1205,8 +1205,7 @@ Expect ParserImpl::expect_storage_class( // struct_decl // : struct_decoration_decl* STRUCT IDENT struct_body_decl -Maybe> ParserImpl::struct_decl( - ast::DecorationList& decos) { +Maybe ParserImpl::struct_decl(ast::DecorationList& decos) { auto t = peek(); auto source = t.source(); @@ -1225,7 +1224,7 @@ Maybe> ParserImpl::struct_decl( if (struct_decos.errored) return Failure::kErrored; - return std::make_unique( + return create( module_.RegisterSymbol(name.value), create(source, std::move(body.value), std::move(struct_decos.value))); diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h index d4d6a4d7b7..838f0bb42d 100644 --- a/src/reader/wgsl/parser_impl.h +++ b/src/reader/wgsl/parser_impl.h @@ -397,7 +397,7 @@ class ParserImpl { /// `struct_decoration_decl*` provided as `decos`. /// @returns the struct type or nullptr on error /// @param decos the list of decorations for the struct declaration. - Maybe> struct_decl(ast::DecorationList& decos); + Maybe struct_decl(ast::DecorationList& decos); /// Parses a `struct_body_decl` grammar element, erroring on parse failure. /// @returns the struct members Expect expect_struct_body_decl(); diff --git a/src/type/type_manager.cc b/src/type/type_manager.cc index 235819fdee..a9b1121f38 100644 --- a/src/type/type_manager.cc +++ b/src/type/type_manager.cc @@ -24,18 +24,5 @@ Manager::Manager(Manager&&) = default; Manager& Manager::operator=(Manager&& rhs) = default; Manager::~Manager() = default; -void Manager::Reset() { - types_.clear(); -} - -type::Type* Manager::Get(std::unique_ptr type) { - auto name = type->type_name(); - - if (types_.find(name) == types_.end()) { - types_[name] = std::move(type); - } - return types_.find(name)->second.get(); -} - } // namespace type } // namespace tint diff --git a/src/type/type_manager.h b/src/type/type_manager.h index 5e2835541d..302a3c7a3a 100644 --- a/src/type/type_manager.h +++ b/src/type/type_manager.h @@ -15,11 +15,11 @@ #ifndef SRC_TYPE_TYPE_MANAGER_H_ #define SRC_TYPE_TYPE_MANAGER_H_ -#include #include #include #include +#include "src/block_allocator.h" #include "src/type/type.h" namespace tint { @@ -28,6 +28,9 @@ namespace type { /// The type manager holds all the pointers to the known types. class Manager { public: + /// Iterator is the type returned by begin() and end() + using Iterator = BlockAllocator::ConstIterator; + /// Constructor Manager(); @@ -42,31 +45,38 @@ class Manager { /// Destructor ~Manager(); - /// Clears all registered types. - void Reset(); - - /// Get the given type from the type manager - /// @param type The type to register - /// @return the pointer to the registered type - type::Type* Get(std::unique_ptr type); - /// Get the given type `T` from the type manager /// @param args the arguments to pass to the type constructor /// @return the pointer to the registered type template T* Get(ARGS&&... args) { - auto ty = Get(std::make_unique(std::forward(args)...)); - return static_cast(ty); + // Note: We do not use std::forward here, as we may need to use the + // arguments again for the call to Create() below. + auto name = T(args...).type_name(); + auto it = by_name_.find(name); + if (it != by_name_.end()) { + return static_cast(it->second); + } + + auto* type = types_.Create(std::forward(args)...); + by_name_.emplace(name, type); + return type; } /// Returns the type map /// @returns the mapping from name string to type. - const std::unordered_map>& types() { - return types_; + const std::unordered_map& types() { + return by_name_; } + /// @returns an iterator to the beginning of the types + Iterator begin() const { return types_.Objects().begin(); } + /// @returns an iterator to the end of the types + Iterator end() const { return types_.Objects().end(); } + private: - std::unordered_map> types_; + std::unordered_map by_name_; + BlockAllocator types_; }; } // namespace type diff --git a/src/type/type_manager_test.cc b/src/type/type_manager_test.cc index d8cf49546b..5a938d5766 100644 --- a/src/type/type_manager_test.cc +++ b/src/type/type_manager_test.cc @@ -26,46 +26,33 @@ using TypeManagerTest = testing::Test; TEST_F(TypeManagerTest, GetUnregistered) { Manager tm; - auto* t = tm.Get(std::make_unique()); + auto* t = tm.Get(); ASSERT_NE(t, nullptr); EXPECT_TRUE(t->Is()); } TEST_F(TypeManagerTest, GetSameTypeReturnsSamePtr) { Manager tm; - auto* t = tm.Get(std::make_unique()); + auto* t = tm.Get(); ASSERT_NE(t, nullptr); EXPECT_TRUE(t->Is()); - auto* t2 = tm.Get(std::make_unique()); + auto* t2 = tm.Get(); EXPECT_EQ(t, t2); } TEST_F(TypeManagerTest, GetDifferentTypeReturnsDifferentPtr) { Manager tm; - auto* t = tm.Get(std::make_unique()); + Type* t = tm.Get(); ASSERT_NE(t, nullptr); EXPECT_TRUE(t->Is()); - auto* t2 = tm.Get(std::make_unique()); + Type* t2 = tm.Get(); ASSERT_NE(t2, nullptr); EXPECT_NE(t, t2); EXPECT_TRUE(t2->Is()); } -TEST_F(TypeManagerTest, ResetClearsPreviousData) { - Manager tm; - auto* t = tm.Get(std::make_unique()); - ASSERT_NE(t, nullptr); - - EXPECT_FALSE(tm.types().empty()); - tm.Reset(); - EXPECT_TRUE(tm.types().empty()); - - auto* t2 = tm.Get(std::make_unique()); - ASSERT_NE(t2, nullptr); -} - } // namespace } // namespace type } // namespace tint diff --git a/src/type_determiner.cc b/src/type_determiner.cc index 6dcce00a8f..cf30907478 100644 --- a/src/type_determiner.cc +++ b/src/type_determiner.cc @@ -1109,7 +1109,8 @@ bool TypeDeterminer::DetermineMemberAccessor( // The vector will have a number of components equal to the length of the // swizzle. This assumes the validator will check that the swizzle // is correct. - ret = mod_->create(vec->type(), size); + ret = + mod_->create(vec->type(), static_cast(size)); } } else { set_error( diff --git a/src/type_determiner_test.cc b/src/type_determiner_test.cc index 27a43790d9..4637ce7fa9 100644 --- a/src/type_determiner_test.cc +++ b/src/type_determiner_test.cc @@ -1345,23 +1345,22 @@ inline std::ostream& operator<<(std::ostream& out, TextureTestParams data) { class Intrinsic_TextureOperation : public TypeDeterminerTestWithParam { public: - std::unique_ptr get_coords_type(type::TextureDimension dim, - type::Type* type) { + type::Type* get_coords_type(type::TextureDimension dim, type::Type* type) { if (dim == type::TextureDimension::k1d) { if (type->Is()) { - return std::make_unique(); + return create(); } else if (type->Is()) { - return std::make_unique(); + return create(); } else { - return std::make_unique(); + return create(); } } else if (dim == type::TextureDimension::k1dArray || dim == type::TextureDimension::k2d) { - return std::make_unique(type, 2); + return create(type, 2); } else if (dim == type::TextureDimension::kCubeArray) { - return std::make_unique(type, 4); + return create(type, 4); } else { - return std::make_unique(type, 3); + return create(type, 3); } } @@ -1373,14 +1372,14 @@ class Intrinsic_TextureOperation call_params->push_back(Expr(name)); } - std::unique_ptr subtype(Texture type) { + type::Type* subtype(Texture type) { if (type == Texture::kF32) { - return std::make_unique(); + return create(); } if (type == Texture::kI32) { - return std::make_unique(); + return create(); } - return std::make_unique(); + return create(); } }; @@ -1390,14 +1389,14 @@ TEST_P(Intrinsic_StorageTextureOperation, TextureLoadRo) { auto type = GetParam().type; auto format = GetParam().format; - auto coords_type = get_coords_type(dim, ty.i32); + auto* coords_type = get_coords_type(dim, ty.i32); type::Type* texture_type = mod->create(dim, format); ast::ExpressionList call_params; add_call_param("texture", texture_type, &call_params); - add_call_param("coords", coords_type.get(), &call_params); + add_call_param("coords", coords_type, &call_params); add_call_param("lod", ty.i32, &call_params); auto* expr = Call("textureLoad", call_params); @@ -1460,14 +1459,14 @@ TEST_P(Intrinsic_SampledTextureOperation, TextureLoadSampled) { auto dim = GetParam().dim; auto type = GetParam().type; - std::unique_ptr s = subtype(type); - auto coords_type = get_coords_type(dim, ty.i32); - auto texture_type = std::make_unique(dim, s.get()); + type::Type* s = subtype(type); + auto* coords_type = get_coords_type(dim, ty.i32); + auto* texture_type = create(dim, s); ast::ExpressionList call_params; - add_call_param("texture", texture_type.get(), &call_params); - add_call_param("coords", coords_type.get(), &call_params); + add_call_param("texture", texture_type, &call_params); + add_call_param("coords", coords_type, &call_params); add_call_param("lod", ty.i32, &call_params); auto* expr = Call("textureLoad", call_params);