diff --git a/src/BUILD.gn b/src/BUILD.gn index 89c5b969ce..8c0a9fe952 100644 --- a/src/BUILD.gn +++ b/src/BUILD.gn @@ -509,8 +509,6 @@ libtint_source_set("libtint_core_all_src") { "transform/canonicalize_entry_point_io.h", "transform/decompose_storage_access.cc", "transform/decompose_storage_access.h", - "transform/duplicate_storage_structs.cc", - "transform/duplicate_storage_structs.h", "transform/external_texture_transform.cc", "transform/external_texture_transform.h", "transform/first_index_offset.cc", diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f9f5a82206..9927bdde1a 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -273,8 +273,6 @@ set(TINT_LIB_SRCS transform/canonicalize_entry_point_io.h transform/decompose_storage_access.cc transform/decompose_storage_access.h - transform/duplicate_storage_structs.cc - transform/duplicate_storage_structs.h transform/external_texture_transform.cc transform/external_texture_transform.h transform/first_index_offset.cc @@ -802,7 +800,6 @@ if(${TINT_BUILD_TESTS}) transform/calculate_array_length_test.cc transform/canonicalize_entry_point_io_test.cc transform/decompose_storage_access_test.cc - transform/duplicate_storage_structs_test.cc transform/external_texture_transform_test.cc transform/first_index_offset_test.cc transform/renamer_test.cc diff --git a/src/transform/duplicate_storage_structs.cc b/src/transform/duplicate_storage_structs.cc deleted file mode 100644 index 42ffccc828..0000000000 --- a/src/transform/duplicate_storage_structs.cc +++ /dev/null @@ -1,126 +0,0 @@ -// Copyright 2021 The Tint Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "src/transform/duplicate_storage_structs.h" - -#include -#include -#include - -#include "src/program_builder.h" -#include "src/sem/variable.h" -#include "src/utils/get_or_create.h" -#include "src/utils/hash.h" - -namespace { -struct StructAccessPair { - tint::sem::Struct const* str; - tint::ast::AccessControl::Access access; - - bool operator==(const StructAccessPair& rhs) const { - return str == rhs.str && access == rhs.access; - } -}; -} // namespace - -namespace std { -/// Custom std::hash specialization for StructAccessPair so -/// StructAccessPairs can be used as keys for std::unordered_map and -/// std::unordered_set. -template <> -class hash { - public: - /// @param struct_access_pair the StructAccessPair to create a hash for - /// @return the hash value - inline std::size_t operator()( - const StructAccessPair& struct_access_pair) const { - return tint::utils::Hash(struct_access_pair.str, struct_access_pair.access); - } -}; -} // namespace std - -namespace tint { -namespace transform { -DuplicateStorageStructs::DuplicateStorageStructs() = default; -DuplicateStorageStructs::~DuplicateStorageStructs() = default; - -Output DuplicateStorageStructs::Run(const Program* in, const DataMap&) { - ProgramBuilder out; - CloneContext ctx(&out, in); - - // Create struct duplicates of storage buffers per access type. - // We do this by finding all storage variables, and creating a copy of the - // Struct they point to per struct/access pair. - - std::unordered_map sap_to_new_struct; - - for (auto* node : in->ASTNodes().Objects()) { - if (auto* ast_var = node->As()) { - auto* var = in->Sem().Get(ast_var); - if (auto* str = var->Type()->UnwrapRef()->As()) { - // Skip non-storage structs - if (!str->UsedAs(ast::StorageClass::kStorage)) { - continue; - } - - auto sap = StructAccessPair{str, var->AccessControl()}; - - auto* new_str = utils::GetOrCreate(sap_to_new_struct, sap, [&] { - // For this ast::Struct/Access pair, create a clone of the existing - // ast::Struct with a new name. - auto* old_str = str->Declaration(); - std::string old_name = ctx.src->Symbols().NameFor(old_str->name()); - - auto* new_struct = [&] { - Symbol new_name = ctx.dst->Symbols().New(old_name); - auto new_members = ctx.Clone(old_str->members()); - auto new_decorations = ctx.Clone(old_str->decorations()); - return ctx.dst->create(new_name, new_members, - new_decorations); - }(); - - // Insert it after the original struct - ctx.InsertAfter(ctx.src->AST().GlobalDeclarations(), - str->Declaration(), new_struct); - - // Remove the original struct - ctx.Remove(ctx.src->AST().GlobalDeclarations(), old_str); - - return new_struct; - }); - - // Replace the existing variable with a new one who's type is an - // AccessControl>. - auto* new_var = [&] { - auto new_name = ctx.Clone(ast_var->symbol()); - auto* new_ty = ctx.dst->ty.access( - var->AccessControl(), ctx.dst->ty.type_name(new_str->name())); - auto* new_constructor = ctx.Clone(ast_var->constructor()); - auto new_decorations = ctx.Clone(ast_var->decorations()); - return ctx.dst->create( - new_name, ast_var->declared_storage_class(), new_ty, - ast_var->is_const(), new_constructor, new_decorations); - }(); - ctx.Replace(ast_var, new_var); - } - } - } - - ctx.Clone(); - - return Output(Program(std::move(out))); -} - -} // namespace transform -} // namespace tint diff --git a/src/transform/duplicate_storage_structs.h b/src/transform/duplicate_storage_structs.h deleted file mode 100644 index f843977b3b..0000000000 --- a/src/transform/duplicate_storage_structs.h +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2021 The Tint Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef SRC_TRANSFORM_DUPLICATE_STORAGE_STRUCTS_H_ -#define SRC_TRANSFORM_DUPLICATE_STORAGE_STRUCTS_H_ - -#include -#include - -#include "src/transform/transform.h" - -namespace tint { -namespace transform { - -/// DuplicateStorageStructs is a Transform that creates duplicates of storage -/// structures, one per access control type. -/// -/// This transform is currently required by Dawn for it's GLSL backend so that -/// SPIRV-Cross does not end up renaming the structures internally, which it -/// does to fix aliasing problems (see -/// https://github.com/KhronosGroup/SPIRV-Cross/pull/799). We can remove this -/// transform if/once Dawn using binding numbers rather than names for -/// uniform/storage buffer data. See https://crbug.com/tint/386. -/// -class DuplicateStorageStructs : public Transform { - public: - /// Constructor - DuplicateStorageStructs(); - - /// Destructor - ~DuplicateStorageStructs() override; - - /// Runs the transform on `program`, returning the transformation result. - /// @param program the source program to transform - /// @param data optional extra transform-specific data - /// @returns the transformation result - Output Run(const Program* program, const DataMap& data = {}) override; -}; - -} // namespace transform -} // namespace tint - -#endif // SRC_TRANSFORM_DUPLICATE_STORAGE_STRUCTS_H_ diff --git a/src/transform/duplicate_storage_structs_test.cc b/src/transform/duplicate_storage_structs_test.cc deleted file mode 100644 index ab58a1dd36..0000000000 --- a/src/transform/duplicate_storage_structs_test.cc +++ /dev/null @@ -1,275 +0,0 @@ -// Copyright 2021 The Tint Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "src/transform/duplicate_storage_structs.h" - -#include "src/transform/test_helper.h" -namespace tint { -namespace transform { -namespace { - -using DuplicateStorageStructsTransformTest = TransformTest; - -TEST_F(DuplicateStorageStructsTransformTest, Simple) { - auto* src = R"( -[[block]] - struct S { - a : f32; -}; - -[[group(0), binding(0)]] var r : [[access(read)]] S; -[[group(0), binding(1)]] var w : [[access(write)]] S; - -[[stage(compute)]] - fn main() { -} -)"; - - auto* expect = R"( -[[block]] -struct S_1 { - a : f32; -}; - -[[block]] -struct S_2 { - a : f32; -}; - -[[group(0), binding(0)]] var r : [[access(read)]] S_1; - -[[group(0), binding(1)]] var w : [[access(write)]] S_2; - -[[stage(compute)]] -fn main() { -} -)"; - - auto got = Run(src); - EXPECT_EQ(expect, str(got)); -} - -TEST_F(DuplicateStorageStructsTransformTest, MultipleStorageStructs) { - auto* src = R"( -[[block]] - struct S1 { - a : f32; -}; - -[[block]] - struct S2 { - a : i32; -}; - -[[group(0), binding(0)]] var r1 : [[access(read)]] S1; -[[group(0), binding(1)]] var w1 : [[access(write)]] S1; - -[[group(0), binding(2)]] var r2 : [[access(read)]] S2; -[[group(0), binding(3)]] var w2 : [[access(write)]] S2; - -[[stage(compute)]] - fn main() { -} -)"; - - auto* expect = R"( -[[block]] -struct S1_1 { - a : f32; -}; - -[[block]] -struct S1_2 { - a : f32; -}; - -[[block]] -struct S2_1 { - a : i32; -}; - -[[block]] -struct S2_2 { - a : i32; -}; - -[[group(0), binding(0)]] var r1 : [[access(read)]] S1_1; - -[[group(0), binding(1)]] var w1 : [[access(write)]] S1_2; - -[[group(0), binding(2)]] var r2 : [[access(read)]] S2_1; - -[[group(0), binding(3)]] var w2 : [[access(write)]] S2_2; - -[[stage(compute)]] -fn main() { -} -)"; - - auto got = Run(src); - EXPECT_EQ(expect, str(got)); -} - -TEST_F(DuplicateStorageStructsTransformTest, MultipleStorageVariables) { - auto* src = R"( -[[block]] - struct S { - a : f32; -}; - -[[group(0), binding(0)]] var r1 : [[access(read)]] S; -[[group(0), binding(1)]] var r2 : [[access(read)]] S; -[[group(0), binding(2)]] var r3 : [[access(read)]] S; -[[group(0), binding(3)]] var w1 : [[access(write)]] S; -[[group(0), binding(4)]] var w2 : [[access(write)]] S; -[[group(0), binding(5)]] var w3 : [[access(write)]] S; -[[group(0), binding(3)]] var rw1 : [[access(read_write)]] S; -[[group(0), binding(4)]] var rw2 : [[access(read_write)]] S; -[[group(0), binding(5)]] var rw3 : [[access(read_write)]] S; - -[[stage(compute)]] - fn main() { -} -)"; - - auto* expect = R"( -[[block]] -struct S_1 { - a : f32; -}; - -[[block]] -struct S_2 { - a : f32; -}; - -[[block]] -struct S_3 { - a : f32; -}; - -[[group(0), binding(0)]] var r1 : [[access(read)]] S_1; - -[[group(0), binding(1)]] var r2 : [[access(read)]] S_1; - -[[group(0), binding(2)]] var r3 : [[access(read)]] S_1; - -[[group(0), binding(3)]] var w1 : [[access(write)]] S_2; - -[[group(0), binding(4)]] var w2 : [[access(write)]] S_2; - -[[group(0), binding(5)]] var w3 : [[access(write)]] S_2; - -[[group(0), binding(3)]] var rw1 : [[access(read_write)]] S_3; - -[[group(0), binding(4)]] var rw2 : [[access(read_write)]] S_3; - -[[group(0), binding(5)]] var rw3 : [[access(read_write)]] S_3; - -[[stage(compute)]] -fn main() { -} -)"; - - auto got = Run(src); - EXPECT_EQ(expect, str(got)); -} - -TEST_F(DuplicateStorageStructsTransformTest, MixedStructTypes) { - auto* src = R"( - -[[block]] -struct S_1 { - a : f32; -}; - -[[block]] -struct S_2 { - a : f32; -}; - -[[block]] -struct S_3 { - a : f32; -}; - -[[group(0), binding(0)]] var r1 : [[access(read)]] S_1; - -[[group(0), binding(1)]] var r2 : [[access(read)]] S_1; - -[[group(0), binding(2)]] var r3 : [[access(read)]] S_1; - -[[group(0), binding(3)]] var w1 : [[access(write)]] S_2; - -[[group(0), binding(4)]] var w2 : [[access(write)]] S_2; - -[[group(0), binding(5)]] var w3 : [[access(write)]] S_2; - -[[group(0), binding(3)]] var rw1 : [[access(read_write)]] S_3; - -[[group(0), binding(4)]] var rw2 : [[access(read_write)]] S_3; - -[[group(0), binding(5)]] var rw3 : [[access(read_write)]] S_3; - -[[stage(compute)]] -fn main() { -} -)"; - - auto* expect = R"( -[[block]] -struct S_1_1 { - a : f32; -}; - -[[block]] -struct S_2_1 { - a : f32; -}; - -[[block]] -struct S_3_1 { - a : f32; -}; - -[[group(0), binding(0)]] var r1 : [[access(read)]] S_1_1; - -[[group(0), binding(1)]] var r2 : [[access(read)]] S_1_1; - -[[group(0), binding(2)]] var r3 : [[access(read)]] S_1_1; - -[[group(0), binding(3)]] var w1 : [[access(write)]] S_2_1; - -[[group(0), binding(4)]] var w2 : [[access(write)]] S_2_1; - -[[group(0), binding(5)]] var w3 : [[access(write)]] S_2_1; - -[[group(0), binding(3)]] var rw1 : [[access(read_write)]] S_3_1; - -[[group(0), binding(4)]] var rw2 : [[access(read_write)]] S_3_1; - -[[group(0), binding(5)]] var rw3 : [[access(read_write)]] S_3_1; - -[[stage(compute)]] -fn main() { -} -)"; - - auto got = Run(src); - EXPECT_EQ(expect, str(got)); -} - -} // namespace -} // namespace transform -} // namespace tint diff --git a/src/transform/spirv.cc b/src/transform/spirv.cc index 75525cbeaa..20bdf68c72 100644 --- a/src/transform/spirv.cc +++ b/src/transform/spirv.cc @@ -26,7 +26,6 @@ #include "src/sem/statement.h" #include "src/sem/struct.h" #include "src/sem/variable.h" -#include "src/transform/duplicate_storage_structs.h" #include "src/transform/external_texture_transform.h" #include "src/transform/manager.h" @@ -41,7 +40,6 @@ Spirv::~Spirv() = default; Output Spirv::Run(const Program* in, const DataMap& data) { Manager manager; manager.Add(); - manager.Add(); auto transformedInput = manager.Run(in, data); auto* cfg = data.Get(); diff --git a/test/BUILD.gn b/test/BUILD.gn index 616ab312cf..867e8d7a7d 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -257,9 +257,9 @@ tint_unittests_source_set("tint_unittests_core_src") { "../src/resolver/intrinsic_test.cc", "../src/resolver/is_host_shareable_test.cc", "../src/resolver/is_storeable_test.cc", - "../src/resolver/pipeline_overridable_constant_test.cc", "../src/resolver/ptr_ref_test.cc", "../src/resolver/ptr_ref_validation_test.cc", + "../src/resolver/pipeline_overridable_constant_test.cc", "../src/resolver/resolver_test.cc", "../src/resolver/resolver_test_helper.cc", "../src/resolver/resolver_test_helper.h", @@ -299,7 +299,6 @@ tint_unittests_source_set("tint_unittests_core_src") { "../src/transform/calculate_array_length_test.cc", "../src/transform/canonicalize_entry_point_io_test.cc", "../src/transform/decompose_storage_access_test.cc", - "../src/transform/duplicate_storage_structs_test.cc", "../src/transform/external_texture_transform_test.cc", "../src/transform/first_index_offset_test.cc", "../src/transform/renamer_test.cc",