Add transform::BindingRemapper
A transform to replace binding points and access control flags.
Required by Dawn for the HLSL and MSL backends
Fixed: tint:104
Fixed: tint:621
Fixed: tint:671
Change-Id: Ic6ccc4a8a7724697cc4af0fad25394a1973eeb22
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46262
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/include/tint/tint.h b/include/tint/tint.h
index 551c204..b6d0031 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 d71ba68..3139e79 100644
--- a/src/BUILD.gn
+++ b/src/BUILD.gn
@@ -396,6 +396,9 @@
"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 04b87e1..599c114 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -211,10 +211,13 @@
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_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 0000000..131da44
--- /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 <stdint.h>
+
+#include <functional>
+
+#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<tint::transform::BindingPoint> {
+ 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 0000000..4c8ed7b
--- /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 <utility>
+
+#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<Remappings>();
+ 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<ast::GroupDecoration>(to.group);
+ auto* new_binding = out.create<ast::BindingDecoration>(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<type::AccessControl>()) {
+ inner_ty = ctx.Clone(old_ac->type());
+ } else {
+ inner_ty = ctx.Clone(ty);
+ }
+ auto* new_ty = ctx.dst->create<type::AccessControl>(ac, inner_ty);
+ auto* new_var = ctx.dst->create<ast::Variable>(
+ 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 0000000..b80d823
--- /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 <unordered_map>
+
+#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<BindingPoint, BindingPoint>;
+
+ /// AccessControls is a map of old binding point to new access control
+ using AccessControls = std::unordered_map<BindingPoint, ast::AccessControl>;
+
+ /// Remappings is consumed by the BindingRemapper transform.
+ /// Data holds information about shader usage and constant buffer offsets.
+ struct Remappings : public Castable<Data, transform::Data> {
+ /// 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 0000000..cc04bf0
--- /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 <utility>
+
+#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<storage> a : S;
+
+[[group(3), binding(2)]] var<storage> b : S;
+
+[[stage(compute)]]
+fn f() -> void {
+}
+)";
+
+ auto* expect = src;
+
+ DataMap data;
+ data.Add<BindingRemapper::Remappings>(BindingRemapper::BindingPoints{},
+ BindingRemapper::AccessControls{});
+ auto got = Run<BindingRemapper>(src, std::move(data));
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(BindingRemapperTest, RemapBindingPoints) {
+ auto* src = R"(
+[[block]]
+struct S {
+};
+
+[[group(2), binding(1)]] var<storage> a : S;
+
+[[group(3), binding(2)]] var<storage> b : S;
+
+[[stage(compute)]]
+fn f() -> void {
+}
+)";
+
+ auto* expect = R"(
+[[block]]
+struct S {
+};
+
+[[group(1), binding(2)]] var<storage> a : S;
+
+[[group(3), binding(2)]] var<storage> b : S;
+
+[[stage(compute)]]
+fn f() -> void {
+}
+)";
+
+ DataMap data;
+ data.Add<BindingRemapper::Remappings>(
+ 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<BindingRemapper>(src, std::move(data));
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(BindingRemapperTest, RemapAccessControls) {
+ auto* src = R"(
+[[block]]
+struct S {
+};
+
+[[group(2), binding(1)]] var<storage> a : [[access(read)]]
+S;
+
+[[group(3), binding(2)]] var<storage> b : [[access(write)]]
+S;
+
+[[group(4), binding(3)]] var<storage> c : S;
+
+[[stage(compute)]]
+fn f() -> void {
+}
+)";
+
+ auto* expect = R"(
+[[block]]
+struct S {
+};
+
+[[group(2), binding(1)]] var<storage> a : [[access(write)]]
+S;
+
+[[group(3), binding(2)]] var<storage> b : [[access(write)]]
+S;
+
+[[group(4), binding(3)]] var<storage> c : [[access(read)]]
+S;
+
+[[stage(compute)]]
+fn f() -> void {
+}
+)";
+
+ DataMap data;
+ data.Add<BindingRemapper::Remappings>(
+ 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<BindingRemapper>(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<storage> a : ReadOnlyS;
+
+[[group(3), binding(2)]] var<storage> b : WriteOnlyS;
+
+[[group(4), binding(3)]] var<storage> 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<storage> a : [[access(write)]]
+S;
+
+[[group(3), binding(2)]] var<storage> b : WriteOnlyS;
+
+[[group(4), binding(3)]] var<storage> c : [[access(write)]] S;
+
+[[stage(compute)]]
+fn f() -> void {
+}
+)";
+
+ DataMap data;
+ data.Add<BindingRemapper::Remappings>(
+ 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<BindingRemapper>(src, std::move(data));
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(BindingRemapperTest, RemapAll) {
+ auto* src = R"(
+[[block]]
+struct S {
+};
+
+[[group(2), binding(1)]] var<storage> a : [[access(read)]]
+S;
+
+[[group(3), binding(2)]] var<storage> b : S;
+
+[[stage(compute)]]
+fn f() -> void {
+}
+)";
+
+ auto* expect = R"(
+[[block]]
+struct S {
+};
+
+[[group(4), binding(5)]] var<storage> a : [[access(write)]]
+S;
+
+[[group(6), binding(7)]] var<storage> b : [[access(write)]]
+S;
+
+[[stage(compute)]]
+fn f() -> void {
+}
+)";
+
+ DataMap data;
+ data.Add<BindingRemapper::Remappings>(
+ 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<BindingRemapper>(src, std::move(data));
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(BindingRemapperTest, NoData) {
+ auto* src = R"(
+[[block]]
+struct S {
+};
+
+[[group(2), binding(1)]] var<storage> a : S;
+[[group(3), binding(2)]] var<storage> b : S;
+
+[[stage(compute)]]
+fn f() -> void {}
+)";
+
+ auto* expect = "error: BindingRemapper did not find the remapping data";
+
+ auto got = Run<BindingRemapper>(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 18dba91..5f4e94b 100644
--- a/src/writer/spirv/scalar_constant.h
+++ b/src/writer/spirv/scalar_constant.h
@@ -20,6 +20,8 @@
#include <cstring>
#include <functional>
+#include "src/utils/hash.h"
+
namespace tint {
// Forward declarations
@@ -105,8 +107,7 @@
const tint::writer::spirv::ScalarConstant& c) const {
uint32_t value = 0;
std::memcpy(&value, &c.value, sizeof(value));
- return (static_cast<std::size_t>(value) << 2) |
- (static_cast<std::size_t>(c.kind) & 3);
+ return tint::utils::Hash(value, c.kind);
}
};
diff --git a/test/BUILD.gn b/test/BUILD.gn
index b1d6b65..7bc5663 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -189,6 +189,7 @@
"../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",
