diff --git a/include/tint/tint.h b/include/tint/tint.h index 551c20477c..b6d0031785 100644 --- a/include/tint/tint.h +++ b/include/tint/tint.h @@ -23,6 +23,7 @@ #include "src/diagnostic/printer.h" #include "src/inspector/inspector.h" #include "src/reader/reader.h" +#include "src/transform/binding_remapper.h" #include "src/transform/bound_array_accessors.h" #include "src/transform/emit_vertex_point_size.h" #include "src/transform/first_index_offset.h" diff --git a/src/BUILD.gn b/src/BUILD.gn index d71ba688ef..3139e7990d 100644 --- a/src/BUILD.gn +++ b/src/BUILD.gn @@ -396,6 +396,9 @@ source_set("libtint_core_src") { "symbol_table.cc", "symbol_table.h", "traits.h", + "transform/binding_point.h", + "transform/binding_remapper.cc", + "transform/binding_remapper.h", "transform/bound_array_accessors.cc", "transform/bound_array_accessors.h", "transform/emit_vertex_point_size.cc", diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 04b87e1ac2..599c114f47 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -211,10 +211,13 @@ set(TINT_LIB_SRCS symbol_table.cc symbol_table.h traits.h - transform/emit_vertex_point_size.cc - transform/emit_vertex_point_size.h + transform/binding_point.h + transform/binding_remapper.cc + transform/binding_remapper.h transform/bound_array_accessors.cc transform/bound_array_accessors.h + transform/emit_vertex_point_size.cc + transform/emit_vertex_point_size.h transform/first_index_offset.cc transform/first_index_offset.h transform/manager.cc @@ -724,6 +727,7 @@ if(${TINT_BUILD_TESTS}) if(${TINT_BUILD_WGSL_READER} AND ${TINT_BUILD_WGSL_WRITER}) list(APPEND TINT_TEST_SRCS + transform/binding_remapper_test.cc transform/bound_array_accessors_test.cc transform/emit_vertex_point_size_test.cc transform/first_index_offset_test.cc diff --git a/src/transform/binding_point.h b/src/transform/binding_point.h new file mode 100644 index 0000000000..131da44bd0 --- /dev/null +++ b/src/transform/binding_point.h @@ -0,0 +1,70 @@ +// 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_BINDING_POINT_H_ +#define SRC_TRANSFORM_BINDING_POINT_H_ + +#include + +#include + +#include "src/utils/hash.h" + +namespace tint { +namespace transform { + +/// BindingPoint holds a group and binding index. +struct BindingPoint { + /// The `[[group]]` part of the binding point + uint32_t group = 0; + /// The `[[binding]]` part of the binding point + uint32_t binding = 0; + + /// Equality operator + /// @param rhs the BindingPoint to compare against + /// @returns true if this BindingPoint is equal to `rhs` + inline bool operator==(const BindingPoint& rhs) const { + return group == rhs.group && binding == rhs.binding; + } + + /// Inequality operator + /// @param rhs the BindingPoint to compare against + /// @returns true if this BindingPoint is not equal to `rhs` + inline bool operator!=(const BindingPoint& rhs) const { + return !(*this == rhs); + } +}; + +} // namespace transform +} // namespace tint + +namespace std { + +/// Custom std::hash specialization for tint::transform::BindingPoint so +/// BindingPoints can be used as keys for std::unordered_map and +/// std::unordered_set. +template <> +class hash { + public: + /// @param binding_point the binding point to create a hash for + /// @return the hash value + inline std::size_t operator()( + const tint::transform::BindingPoint& binding_point) const { + return tint::utils::Hash(binding_point.group, binding_point.binding); + } +}; + +} // namespace std + +#endif // SRC_TRANSFORM_BINDING_POINT_H_ diff --git a/src/transform/binding_remapper.cc b/src/transform/binding_remapper.cc new file mode 100644 index 0000000000..4c8ed7bd50 --- /dev/null +++ b/src/transform/binding_remapper.cc @@ -0,0 +1,90 @@ +// 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/binding_remapper.h" + +#include + +#include "src/program_builder.h" +#include "src/semantic/variable.h" +#include "src/type/access_control_type.h" + +TINT_INSTANTIATE_TYPEINFO(tint::transform::BindingRemapper::Remappings); + +namespace tint { +namespace transform { + +BindingRemapper::Remappings::Remappings(BindingPoints bp, AccessControls ac) + : binding_points(std::move(bp)), access_controls(std::move(ac)) {} + +BindingRemapper::Remappings::Remappings(const Remappings&) = default; +BindingRemapper::Remappings::~Remappings() = default; + +BindingRemapper::BindingRemapper() = default; +BindingRemapper::~BindingRemapper() = default; + +Transform::Output BindingRemapper::Run(const Program* in, + const DataMap& datamap) { + ProgramBuilder out; + auto* remappings = datamap.Get(); + if (!remappings) { + out.Diagnostics().add_error( + "BindingRemapper did not find the remapping data"); + return Output(Program(std::move(out))); + } + CloneContext ctx(&out, in); + for (auto* var : in->AST().GlobalVariables()) { + if (auto binding_point = var->binding_point()) { + BindingPoint from{binding_point.group->value(), + binding_point.binding->value()}; + + // Replace any group or binding decorations. + // Note: This has to be performed *before* remapping access controls, as + // `ctx.Clone(var->decorations())` depend on these replacements. + auto bp_it = remappings->binding_points.find(from); + if (bp_it != remappings->binding_points.end()) { + BindingPoint to = bp_it->second; + auto* new_group = out.create(to.group); + auto* new_binding = out.create(to.binding); + ctx.Replace(binding_point.group, new_group); + ctx.Replace(binding_point.binding, new_binding); + } + + // Replace any access controls. + auto ac_it = remappings->access_controls.find(from); + if (ac_it != remappings->access_controls.end()) { + ast::AccessControl ac = ac_it->second; + auto* var_ty = in->Sem().Get(var)->Type(); + auto* ty = var_ty->UnwrapAliasIfNeeded(); + type::Type* inner_ty = nullptr; + if (auto* old_ac = ty->As()) { + inner_ty = ctx.Clone(old_ac->type()); + } else { + inner_ty = ctx.Clone(ty); + } + auto* new_ty = ctx.dst->create(ac, inner_ty); + auto* new_var = ctx.dst->create( + ctx.Clone(var->source()), ctx.Clone(var->symbol()), + var->declared_storage_class(), new_ty, var->is_const(), + ctx.Clone(var->constructor()), ctx.Clone(var->decorations())); + ctx.Replace(var, new_var); + } + } + } + ctx.Clone(); + return Output(Program(std::move(out))); +} + +} // namespace transform +} // namespace tint diff --git a/src/transform/binding_remapper.h b/src/transform/binding_remapper.h new file mode 100644 index 0000000000..b80d82373f --- /dev/null +++ b/src/transform/binding_remapper.h @@ -0,0 +1,72 @@ +// 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_BINDING_REMAPPER_H_ +#define SRC_TRANSFORM_BINDING_REMAPPER_H_ + +#include + +#include "src/ast/access_control.h" +#include "src/transform/binding_point.h" +#include "src/transform/transform.h" + +namespace tint { +namespace transform { + +/// BindingRemapper is a transform used to remap resource binding points and +/// access controls. +class BindingRemapper : public Transform { + public: + /// BindingPoints is a map of old binding point to new binding point + using BindingPoints = std::unordered_map; + + /// AccessControls is a map of old binding point to new access control + using AccessControls = std::unordered_map; + + /// Remappings is consumed by the BindingRemapper transform. + /// Data holds information about shader usage and constant buffer offsets. + struct Remappings : public Castable { + /// Constructor + /// @param bp a map of new binding points + /// @param ac a map of new access controls + Remappings(BindingPoints bp, AccessControls ac); + + /// Copy constructor + Remappings(const Remappings&); + + /// Destructor + ~Remappings() override; + + /// A map of old binding point to new binding point + BindingPoints const binding_points; + + /// A map of old binding point to new access controls + AccessControls const access_controls; + }; + + /// Constructor + BindingRemapper(); + ~BindingRemapper() override; + + /// Runs the transform on `program`, returning the transformation result. + /// @param program the source program to transform + /// @param data optional extra transform-specific input data + /// @returns the transformation result + Output Run(const Program* program, const DataMap& data = {}) override; +}; + +} // namespace transform +} // namespace tint + +#endif // SRC_TRANSFORM_BINDING_REMAPPER_H_ diff --git a/src/transform/binding_remapper_test.cc b/src/transform/binding_remapper_test.cc new file mode 100644 index 0000000000..cc04bf04e7 --- /dev/null +++ b/src/transform/binding_remapper_test.cc @@ -0,0 +1,279 @@ +// 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/binding_remapper.h" + +#include + +#include "src/transform/test_helper.h" + +namespace tint { +namespace transform { +namespace { + +using BindingRemapperTest = TransformTest; + +TEST_F(BindingRemapperTest, NoRemappings) { + auto* src = R"( +[[block]] +struct S { +}; + +[[group(2), binding(1)]] var a : S; + +[[group(3), binding(2)]] var b : S; + +[[stage(compute)]] +fn f() -> void { +} +)"; + + auto* expect = src; + + DataMap data; + data.Add(BindingRemapper::BindingPoints{}, + BindingRemapper::AccessControls{}); + auto got = Run(src, std::move(data)); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(BindingRemapperTest, RemapBindingPoints) { + auto* src = R"( +[[block]] +struct S { +}; + +[[group(2), binding(1)]] var a : S; + +[[group(3), binding(2)]] var b : S; + +[[stage(compute)]] +fn f() -> void { +} +)"; + + auto* expect = R"( +[[block]] +struct S { +}; + +[[group(1), binding(2)]] var a : S; + +[[group(3), binding(2)]] var b : S; + +[[stage(compute)]] +fn f() -> void { +} +)"; + + DataMap data; + data.Add( + BindingRemapper::BindingPoints{ + {{2, 1}, {1, 2}}, // Remap + {{4, 5}, {6, 7}}, // Not found + // Keep [[group(3), binding(2)]] as is + }, + BindingRemapper::AccessControls{}); + auto got = Run(src, std::move(data)); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(BindingRemapperTest, RemapAccessControls) { + auto* src = R"( +[[block]] +struct S { +}; + +[[group(2), binding(1)]] var a : [[access(read)]] +S; + +[[group(3), binding(2)]] var b : [[access(write)]] +S; + +[[group(4), binding(3)]] var c : S; + +[[stage(compute)]] +fn f() -> void { +} +)"; + + auto* expect = R"( +[[block]] +struct S { +}; + +[[group(2), binding(1)]] var a : [[access(write)]] +S; + +[[group(3), binding(2)]] var b : [[access(write)]] +S; + +[[group(4), binding(3)]] var c : [[access(read)]] +S; + +[[stage(compute)]] +fn f() -> void { +} +)"; + + DataMap data; + data.Add( + BindingRemapper::BindingPoints{}, + BindingRemapper::AccessControls{ + {{2, 1}, ast::AccessControl::kWriteOnly}, // Modify access control + // Keep [[group(3), binding(2)]] as is + {{4, 3}, ast::AccessControl::kReadOnly}, // Add access control + }); + auto got = Run(src, std::move(data)); + + EXPECT_EQ(expect, str(got)); +} + +// TODO(crbug.com/676): Possibly enable if the spec allows for access +// decorations in type aliases. If not, just remove. +TEST_F(BindingRemapperTest, DISABLED_RemapAccessControlsWithAliases) { + auto* src = R"( +[[block]] +struct S { +}; + +type ReadOnlyS = [[access(read)]] +S; + +type WriteOnlyS = [[access(write)]] +S; + +type A = S; + +[[group(2), binding(1)]] var a : ReadOnlyS; + +[[group(3), binding(2)]] var b : WriteOnlyS; + +[[group(4), binding(3)]] var c : A; + +[[stage(compute)]] +fn f() -> void { +} +)"; + + auto* expect = R"( +[[block]] +struct S { +}; + +type ReadOnlyS = [[access(read)]] +S; + +type WriteOnlyS = [[access(write)]] +S; + +type A = S; + +[[group(2), binding(1)]] var a : [[access(write)]] +S; + +[[group(3), binding(2)]] var b : WriteOnlyS; + +[[group(4), binding(3)]] var c : [[access(write)]] S; + +[[stage(compute)]] +fn f() -> void { +} +)"; + + DataMap data; + data.Add( + BindingRemapper::BindingPoints{}, + BindingRemapper::AccessControls{ + {{2, 1}, ast::AccessControl::kWriteOnly}, // Modify access control + // Keep [[group(3), binding(2)]] as is + {{4, 3}, ast::AccessControl::kReadOnly}, // Add access control + }); + auto got = Run(src, std::move(data)); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(BindingRemapperTest, RemapAll) { + auto* src = R"( +[[block]] +struct S { +}; + +[[group(2), binding(1)]] var a : [[access(read)]] +S; + +[[group(3), binding(2)]] var b : S; + +[[stage(compute)]] +fn f() -> void { +} +)"; + + auto* expect = R"( +[[block]] +struct S { +}; + +[[group(4), binding(5)]] var a : [[access(write)]] +S; + +[[group(6), binding(7)]] var b : [[access(write)]] +S; + +[[stage(compute)]] +fn f() -> void { +} +)"; + + DataMap data; + data.Add( + BindingRemapper::BindingPoints{ + {{2, 1}, {4, 5}}, + {{3, 2}, {6, 7}}, + }, + BindingRemapper::AccessControls{ + {{2, 1}, ast::AccessControl::kWriteOnly}, + {{3, 2}, ast::AccessControl::kWriteOnly}, + }); + auto got = Run(src, std::move(data)); + + EXPECT_EQ(expect, str(got)); +} + +TEST_F(BindingRemapperTest, NoData) { + auto* src = R"( +[[block]] +struct S { +}; + +[[group(2), binding(1)]] var a : S; +[[group(3), binding(2)]] var b : S; + +[[stage(compute)]] +fn f() -> void {} +)"; + + auto* expect = "error: BindingRemapper did not find the remapping data"; + + auto got = Run(src); + + EXPECT_EQ(expect, str(got)); +} + +} // namespace +} // namespace transform +} // namespace tint diff --git a/src/writer/spirv/scalar_constant.h b/src/writer/spirv/scalar_constant.h index 18dba915fd..5f4e94b624 100644 --- a/src/writer/spirv/scalar_constant.h +++ b/src/writer/spirv/scalar_constant.h @@ -20,6 +20,8 @@ #include #include +#include "src/utils/hash.h" + namespace tint { // Forward declarations @@ -105,8 +107,7 @@ class hash { const tint::writer::spirv::ScalarConstant& c) const { uint32_t value = 0; std::memcpy(&value, &c.value, sizeof(value)); - return (static_cast(value) << 2) | - (static_cast(c.kind) & 3); + return tint::utils::Hash(value, c.kind); } }; diff --git a/test/BUILD.gn b/test/BUILD.gn index b1d6b65005..7bc5663c12 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -189,6 +189,7 @@ source_set("tint_unittests_core_src") { "../src/symbol_table_test.cc", "../src/symbol_test.cc", "../src/traits_test.cc", + "../src/transform/binding_remapper_test.cc", "../src/transform/bound_array_accessors_test.cc", "../src/transform/emit_vertex_point_size_test.cc", "../src/transform/first_index_offset_test.cc",