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>

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"

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",

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/binding_point.h
-  transform/emit_vertex_point_size.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

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_

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

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_

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

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 @@ class hash<tint::writer::spirv::ScalarConstant> {
       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) |
+    return tint::utils::Hash(value, c.kind);
-           (static_cast<std::size_t>(c.kind) & 3);
   }
 };
 

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",