[tint] Make binding space consistent for array length from uniform
MSL had the `bindpoint_to_size_index` keys in post-remap space, which goes against the principle that all inputs should be in WGSL binding space.
Also change the `ubo_binding` to use the target-language's output binding type. This requires forking the common `ArrayLengthFromUniformOptions` structure into the two places that use it.
Fixed: tint:2178
Change-Id: I02f849cef704a5c1dc045c3fea602eb7639f1ec4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/176940
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@google.com>
diff --git a/include/tint/tint.h b/include/tint/tint.h
index 23d1d3e..ac6dd66 100644
--- a/include/tint/tint.h
+++ b/include/tint/tint.h
@@ -35,7 +35,6 @@
// headers will need to be moved to include/tint/.
#include "src/tint/api/common/binding_point.h"
-#include "src/tint/api/options/array_length_from_uniform.h"
#include "src/tint/api/options/binding_remapper.h"
#include "src/tint/api/options/external_texture.h"
#include "src/tint/api/options/pixel_local.h"
diff --git a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
index 10390e1..2491045 100644
--- a/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
+++ b/src/dawn/native/d3d12/ShaderModuleD3D12.cpp
@@ -179,7 +179,7 @@
using tint::BindingPoint;
- tint::ArrayLengthFromUniformOptions arrayLengthFromUniform;
+ tint::hlsl::writer::ArrayLengthFromUniformOptions arrayLengthFromUniform;
arrayLengthFromUniform.ubo_binding = {layout->GetDynamicStorageBufferLengthsRegisterSpace(),
layout->GetDynamicStorageBufferLengthsShaderRegister()};
diff --git a/src/dawn/native/metal/ShaderModuleMTL.mm b/src/dawn/native/metal/ShaderModuleMTL.mm
index 1ec3209..02004cb 100644
--- a/src/dawn/native/metal/ShaderModuleMTL.mm
+++ b/src/dawn/native/metal/ShaderModuleMTL.mm
@@ -125,8 +125,8 @@
std::ostringstream errorStream;
errorStream << "Tint MSL failure:" << std::endl;
- tint::ArrayLengthFromUniformOptions arrayLengthFromUniform;
- arrayLengthFromUniform.ubo_binding = {0, kBufferLengthBufferSlot};
+ tint::msl::writer::ArrayLengthFromUniformOptions arrayLengthFromUniform;
+ arrayLengthFromUniform.ubo_binding = kBufferLengthBufferSlot;
tint::msl::writer::Bindings bindings;
@@ -167,7 +167,7 @@
// the array length uniform transform. This is used to compute the
// size of variable length arrays in storage buffers.
arrayLengthFromUniform.bindpoint_to_size_index.emplace(
- dstBindingPoint, dstBindingPoint.binding);
+ srcBindingPoint, dstBindingPoint.binding);
break;
case wgpu::BufferBindingType::Undefined:
DAWN_UNREACHABLE();
@@ -227,7 +227,7 @@
// Use the ShaderIndex as the indices for the buffer size lookups in the array
// length uniform transform.
- arrayLengthFromUniform.bindpoint_to_size_index.emplace(dstBindingPoint,
+ arrayLengthFromUniform.bindpoint_to_size_index.emplace(srcBindingPoint,
dstBindingPoint.binding);
}
}
diff --git a/src/tint/api/options/BUILD.bazel b/src/tint/api/options/BUILD.bazel
index c7eba70..7bf8d69 100644
--- a/src/tint/api/options/BUILD.bazel
+++ b/src/tint/api/options/BUILD.bazel
@@ -42,7 +42,6 @@
"options.cc",
],
hdrs = [
- "array_length_from_uniform.h",
"binding_remapper.h",
"depth_range_offsets.h",
"external_texture.h",
@@ -70,7 +69,6 @@
name = "test",
alwayslink = True,
srcs = [
- "array_length_from_uniform_test.cc",
"binding_remapper_test.cc",
"external_texture_test.cc",
"pixel_local_test.cc",
diff --git a/src/tint/api/options/BUILD.cmake b/src/tint/api/options/BUILD.cmake
index 1f0fffa..9629a54 100644
--- a/src/tint/api/options/BUILD.cmake
+++ b/src/tint/api/options/BUILD.cmake
@@ -39,7 +39,6 @@
# Kind: lib
################################################################################
tint_add_target(tint_api_options lib
- api/options/array_length_from_uniform.h
api/options/binding_remapper.h
api/options/depth_range_offsets.h
api/options/external_texture.h
@@ -68,7 +67,6 @@
# Kind: test
################################################################################
tint_add_target(tint_api_options_test test
- api/options/array_length_from_uniform_test.cc
api/options/binding_remapper_test.cc
api/options/external_texture_test.cc
api/options/pixel_local_test.cc
diff --git a/src/tint/api/options/BUILD.gn b/src/tint/api/options/BUILD.gn
index cc35c71..09a0971 100644
--- a/src/tint/api/options/BUILD.gn
+++ b/src/tint/api/options/BUILD.gn
@@ -44,7 +44,6 @@
libtint_source_set("options") {
sources = [
- "array_length_from_uniform.h",
"binding_remapper.h",
"depth_range_offsets.h",
"external_texture.h",
@@ -70,7 +69,6 @@
if (tint_build_unittests) {
tint_unittests_source_set("unittests") {
sources = [
- "array_length_from_uniform_test.cc",
"binding_remapper_test.cc",
"external_texture_test.cc",
"pixel_local_test.cc",
diff --git a/src/tint/api/options/array_length_from_uniform.h b/src/tint/api/options/array_length_from_uniform.h
deleted file mode 100644
index 821b7d3..0000000
--- a/src/tint/api/options/array_length_from_uniform.h
+++ /dev/null
@@ -1,53 +0,0 @@
-// Copyright 2021 The Dawn & Tint Authors
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are met:
-//
-// 1. Redistributions of source code must retain the above copyright notice, this
-// list of conditions and the following disclaimer.
-//
-// 2. Redistributions in binary form must reproduce the above copyright notice,
-// this list of conditions and the following disclaimer in the documentation
-// and/or other materials provided with the distribution.
-//
-// 3. Neither the name of the copyright holder nor the names of its
-// contributors may be used to endorse or promote products derived from
-// this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
-// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
-// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
-// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
-// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
-// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
-// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
-// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
-// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-#ifndef SRC_TINT_API_OPTIONS_ARRAY_LENGTH_FROM_UNIFORM_H_
-#define SRC_TINT_API_OPTIONS_ARRAY_LENGTH_FROM_UNIFORM_H_
-
-#include <unordered_map>
-
-#include "src/tint/api/common/binding_point.h"
-
-namespace tint {
-
-/// Options used to specify a mapping of binding points to indices into a UBO
-/// from which to load buffer sizes.
-struct ArrayLengthFromUniformOptions {
- /// The binding point to use to generate a uniform buffer from which to read
- /// buffer sizes.
- BindingPoint ubo_binding;
- /// The mapping from storage buffer binding points to the index into the
- /// uniform buffer where the length of the buffer is stored.
- std::unordered_map<BindingPoint, uint32_t> bindpoint_to_size_index;
-
- /// Reflect the fields of this class so that it can be used by tint::ForeachField()
- TINT_REFLECT(ArrayLengthFromUniformOptions, ubo_binding, bindpoint_to_size_index);
-};
-
-} // namespace tint
-
-#endif // SRC_TINT_API_OPTIONS_ARRAY_LENGTH_FROM_UNIFORM_H_
diff --git a/src/tint/api/options/array_length_from_uniform_test.cc b/src/tint/api/options/array_length_from_uniform_test.cc
deleted file mode 100644
index c62092a..0000000
--- a/src/tint/api/options/array_length_from_uniform_test.cc
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright 2024 The Dawn & Tint Authors
-//
-// Redistribution and use in source and binary forms, with or without
-// modification, are permitted provided that the following conditions are met:
-//
-// 1. Redistributions of source code must retain the above copyright notice, this
-// list of conditions and the following disclaimer.
-//
-// 2. Redistributions in binary form must reproduce the above copyright notice,
-// this list of conditions and the following disclaimer in the documentation
-// and/or other materials provided with the distribution.
-//
-// 3. Neither the name of the copyright holder nor the names of its
-// contributors may be used to endorse or promote products derived from
-// this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
-// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
-// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
-// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
-// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
-// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
-// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
-// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
-// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-#include "src/tint/api/options/array_length_from_uniform.h"
-
-#include <gtest/gtest.h>
-
-namespace tint {
-namespace {
-
-TEST(TintCheckAllFieldsReflected, ApiOptionsArrayLengthFromUniformTest) {
- TINT_ASSERT_ALL_FIELDS_REFLECTED(ArrayLengthFromUniformOptions);
-}
-
-} // namespace
-} // namespace tint
diff --git a/src/tint/api/tint.cc b/src/tint/api/tint.cc
index 3337b23..07254b2 100644
--- a/src/tint/api/tint.cc
+++ b/src/tint/api/tint.cc
@@ -32,7 +32,6 @@
// dependency from 'tint/api' to the libraries used to make up the Tint API.
////////////////////////////////////////////////////////////////////////////////
#include "src/tint/api/common/override_id.h"
-#include "src/tint/api/options/array_length_from_uniform.h"
#if TINT_BUILD_GLSL_WRITER
#include "src/tint/lang/glsl/writer/writer.h" // nogncheck
diff --git a/src/tint/cmd/loopy/main.cc b/src/tint/cmd/loopy/main.cc
index 89bbcb3..7703f7a 100644
--- a/src/tint/cmd/loopy/main.cc
+++ b/src/tint/cmd/loopy/main.cc
@@ -261,7 +261,7 @@
tint::msl::writer::Options gen_options;
gen_options.bindings = tint::msl::writer::GenerateBindings(program);
- gen_options.array_length_from_uniform.ubo_binding = tint::BindingPoint{0, 30};
+ gen_options.array_length_from_uniform.ubo_binding = 30;
gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(tint::BindingPoint{0, 0},
0);
gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(tint::BindingPoint{0, 1},
diff --git a/src/tint/cmd/tint/main.cc b/src/tint/cmd/tint/main.cc
index 947355e..f4913a9 100644
--- a/src/tint/cmd/tint/main.cc
+++ b/src/tint/cmd/tint/main.cc
@@ -829,7 +829,7 @@
gen_options.disable_workgroup_init = options.disable_workgroup_init;
gen_options.pixel_local_options = options.pixel_local_options;
gen_options.bindings = tint::msl::writer::GenerateBindings(*input_program);
- gen_options.array_length_from_uniform.ubo_binding = tint::BindingPoint{0, 30};
+ gen_options.array_length_from_uniform.ubo_binding = 30;
// Add array_length_from_uniform entries for all storage buffers with runtime sized arrays.
std::unordered_set<tint::BindingPoint> storage_bindings;
diff --git a/src/tint/lang/hlsl/writer/ast_printer/ast_printer.cc b/src/tint/lang/hlsl/writer/ast_printer/ast_printer.cc
index 387ddb9..fc2a4b8 100644
--- a/src/tint/lang/hlsl/writer/ast_printer/ast_printer.cc
+++ b/src/tint/lang/hlsl/writer/ast_printer/ast_printer.cc
@@ -33,6 +33,7 @@
#include <utility>
#include <vector>
+#include "src/tint/api/common/binding_point.h"
#include "src/tint/lang/core/constant/splat.h"
#include "src/tint/lang/core/constant/value.h"
#include "src/tint/lang/core/fluent_types.h"
@@ -334,12 +335,6 @@
manager.Add<ast::transform::SimplifyPointers>();
manager.Add<ast::transform::RemovePhonies>();
- // Build the config for the internal ArrayLengthFromUniform transform.
- ast::transform::ArrayLengthFromUniform::Config array_length_from_uniform_cfg(
- array_length_from_uniform_options.ubo_binding);
- array_length_from_uniform_cfg.bindpoint_to_size_index =
- std::move(array_length_from_uniform_options.bindpoint_to_size_index);
-
// DemoteToHelper must come after CanonicalizeEntryPointIO, PromoteSideEffectsToDecl, and
// ExpandCompoundAssignment.
// TODO(crbug.com/tint/1752): This is only necessary when FXC is being used.
@@ -348,6 +343,12 @@
// ArrayLengthFromUniform must come after SimplifyPointers as it assumes that the form of the
// array length argument is &var.array.
manager.Add<ast::transform::ArrayLengthFromUniform>();
+ // Build the config for the internal ArrayLengthFromUniform transform.
+ ast::transform::ArrayLengthFromUniform::Config array_length_from_uniform_cfg(
+ BindingPoint{array_length_from_uniform_options.ubo_binding.group,
+ array_length_from_uniform_options.ubo_binding.binding});
+ array_length_from_uniform_cfg.bindpoint_to_size_index =
+ std::move(array_length_from_uniform_options.bindpoint_to_size_index);
data.Add<ast::transform::ArrayLengthFromUniform::Config>(
std::move(array_length_from_uniform_cfg));
// DecomposeMemoryAccess must come after:
diff --git a/src/tint/lang/hlsl/writer/ast_printer/ast_printer.h b/src/tint/lang/hlsl/writer/ast_printer/ast_printer.h
index fa1a540..f17cf3b 100644
--- a/src/tint/lang/hlsl/writer/ast_printer/ast_printer.h
+++ b/src/tint/lang/hlsl/writer/ast_printer/ast_printer.h
@@ -35,7 +35,6 @@
#include <utility>
#include "src/tint/api/common/binding_point.h"
-#include "src/tint/api/options/array_length_from_uniform.h"
#include "src/tint/lang/core/builtin_value.h"
#include "src/tint/lang/hlsl/writer/ast_raise/decompose_memory_access.h"
#include "src/tint/lang/hlsl/writer/common/options.h"
diff --git a/src/tint/lang/hlsl/writer/common/options.h b/src/tint/lang/hlsl/writer/common/options.h
index 90bc6b3..6bf4719 100644
--- a/src/tint/lang/hlsl/writer/common/options.h
+++ b/src/tint/lang/hlsl/writer/common/options.h
@@ -34,7 +34,6 @@
#include <vector>
#include "src/tint/api/common/binding_point.h"
-#include "src/tint/api/options/array_length_from_uniform.h"
#include "src/tint/api/options/binding_remapper.h"
#include "src/tint/api/options/external_texture.h"
#include "src/tint/api/options/pixel_local.h"
@@ -140,6 +139,19 @@
/// D3D11_PS_INPUT_REGISTER_COUNT == D3D12_PS_INPUT_REGISTER_COUNT
constexpr uint32_t kMaxInterStageLocations = 30;
+/// Options used to specify a mapping of binding points to indices into a UBO
+/// from which to load buffer sizes.
+struct ArrayLengthFromUniformOptions {
+ /// The HLSL binding point to use to generate a uniform buffer from which to read buffer sizes.
+ binding::Uniform ubo_binding;
+ /// The mapping from the storage buffer binding points in WGSL binding-point space to the index
+ /// into the uniform buffer where the length of the buffer is stored.
+ std::unordered_map<BindingPoint, uint32_t> bindpoint_to_size_index;
+
+ /// Reflect the fields of this class so that it can be used by tint::ForeachField()
+ TINT_REFLECT(ArrayLengthFromUniformOptions, ubo_binding, bindpoint_to_size_index);
+};
+
/// Configuration options used for generating HLSL.
struct Options {
/// Constructor
diff --git a/src/tint/lang/hlsl/writer/common/options_test.cc b/src/tint/lang/hlsl/writer/common/options_test.cc
index 5feb66f..741a1b0 100644
--- a/src/tint/lang/hlsl/writer/common/options_test.cc
+++ b/src/tint/lang/hlsl/writer/common/options_test.cc
@@ -33,6 +33,7 @@
namespace {
TEST(TintCheckAllFieldsReflected, HlslWriterCommonOptionsTest) {
+ TINT_ASSERT_ALL_FIELDS_REFLECTED(ArrayLengthFromUniformOptions);
TINT_ASSERT_ALL_FIELDS_REFLECTED(binding::BindingInfo);
TINT_ASSERT_ALL_FIELDS_REFLECTED(binding::ExternalTexture);
TINT_ASSERT_ALL_FIELDS_REFLECTED(Bindings);
diff --git a/src/tint/lang/msl/writer/ast_printer/ast_printer.cc b/src/tint/lang/msl/writer/ast_printer/ast_printer.cc
index 9a574b7..a511c62 100644
--- a/src/tint/lang/msl/writer/ast_printer/ast_printer.cc
+++ b/src/tint/lang/msl/writer/ast_printer/ast_printer.cc
@@ -33,6 +33,7 @@
#include <utility>
#include <vector>
+#include "src/tint/api/common/binding_point.h"
#include "src/tint/lang/core/constant/splat.h"
#include "src/tint/lang/core/constant/value.h"
#include "src/tint/lang/core/fluent_types.h"
@@ -187,7 +188,9 @@
ExternalTextureOptions external_texture_options{};
RemapperData remapper_data{};
- PopulateRemapperAndMultiplanarOptions(options, remapper_data, external_texture_options);
+ ArrayLengthFromUniformOptions array_length_from_uniform_options{};
+ PopulateBindingRelatedOptions(options, remapper_data, external_texture_options,
+ array_length_from_uniform_options);
manager.Add<ast::transform::BindingRemapper>();
data.Add<ast::transform::BindingRemapper::Remappings>(
@@ -240,12 +243,13 @@
// ArrayLengthFromUniform must come after SimplifyPointers, as
// it assumes that the form of the array length argument is &var.array.
manager.Add<ast::transform::ArrayLengthFromUniform>();
-
- ast::transform::ArrayLengthFromUniform::Config array_length_cfg(
- std::move(options.array_length_from_uniform.ubo_binding));
- array_length_cfg.bindpoint_to_size_index =
- std::move(options.array_length_from_uniform.bindpoint_to_size_index);
- data.Add<ast::transform::ArrayLengthFromUniform::Config>(array_length_cfg);
+ // Build the config for the internal ArrayLengthFromUniform transform.
+ ast::transform::ArrayLengthFromUniform::Config array_length_from_uniform_cfg(
+ BindingPoint{0, array_length_from_uniform_options.ubo_binding});
+ array_length_from_uniform_cfg.bindpoint_to_size_index =
+ std::move(array_length_from_uniform_options.bindpoint_to_size_index);
+ data.Add<ast::transform::ArrayLengthFromUniform::Config>(
+ std::move(array_length_from_uniform_cfg));
// PackedVec3 must come after ExpandCompoundAssignment.
manager.Add<PackedVec3>();
diff --git a/src/tint/lang/msl/writer/ast_printer/ast_printer.h b/src/tint/lang/msl/writer/ast_printer/ast_printer.h
index 7b4fea1..f86bb39 100644
--- a/src/tint/lang/msl/writer/ast_printer/ast_printer.h
+++ b/src/tint/lang/msl/writer/ast_printer/ast_printer.h
@@ -34,7 +34,6 @@
#include <unordered_set>
#include <vector>
-#include "src/tint/api/options/array_length_from_uniform.h"
#include "src/tint/lang/core/builtin_value.h"
#include "src/tint/lang/msl/writer/common/options.h"
#include "src/tint/lang/wgsl/ast/assignment_statement.h"
diff --git a/src/tint/lang/msl/writer/ast_printer/sanitizer_test.cc b/src/tint/lang/msl/writer/ast_printer/sanitizer_test.cc
index 4745f0c..5bebc55 100644
--- a/src/tint/lang/msl/writer/ast_printer/sanitizer_test.cc
+++ b/src/tint/lang/msl/writer/ast_printer/sanitizer_test.cc
@@ -55,7 +55,7 @@
});
Options opts = DefaultOptions();
- opts.array_length_from_uniform.ubo_binding = BindingPoint{0, 30};
+ opts.array_length_from_uniform.ubo_binding = 30;
opts.array_length_from_uniform.bindpoint_to_size_index.emplace(BindingPoint{2, 1}, 1);
ASTPrinter& gen = SanitizeAndBuild(opts);
@@ -112,7 +112,7 @@
});
Options opts = DefaultOptions();
- opts.array_length_from_uniform.ubo_binding = BindingPoint{0, 30};
+ opts.array_length_from_uniform.ubo_binding = 30;
opts.array_length_from_uniform.bindpoint_to_size_index.emplace(BindingPoint{2, 1}, 1);
ASTPrinter& gen = SanitizeAndBuild(opts);
@@ -173,7 +173,7 @@
});
Options opts = DefaultOptions();
- opts.array_length_from_uniform.ubo_binding = BindingPoint{0, 30};
+ opts.array_length_from_uniform.ubo_binding = 30;
opts.array_length_from_uniform.bindpoint_to_size_index.emplace(BindingPoint{2, 1}, 1);
ASTPrinter& gen = SanitizeAndBuild(opts);
@@ -232,7 +232,7 @@
});
Options options;
- options.array_length_from_uniform.ubo_binding = {0, 29};
+ options.array_length_from_uniform.ubo_binding = 29;
options.array_length_from_uniform.bindpoint_to_size_index.emplace(BindingPoint{0, 1}, 7u);
options.array_length_from_uniform.bindpoint_to_size_index.emplace(BindingPoint{0, 2}, 2u);
ASTPrinter& gen = SanitizeAndBuild(options);
@@ -291,7 +291,7 @@
});
Options options;
- options.array_length_from_uniform.ubo_binding = {0, 29};
+ options.array_length_from_uniform.ubo_binding = 29;
options.array_length_from_uniform.bindpoint_to_size_index.emplace(BindingPoint{0, 2}, 2u);
ASTPrinter& gen = SanitizeAndBuild(options);
diff --git a/src/tint/lang/msl/writer/common/option_helpers.cc b/src/tint/lang/msl/writer/common/option_helpers.cc
index cb6a162..ab4857b 100644
--- a/src/tint/lang/msl/writer/common/option_helpers.cc
+++ b/src/tint/lang/msl/writer/common/option_helpers.cc
@@ -52,27 +52,21 @@
auto wgsl_seen = [&diagnostics, &seen_wgsl_bindings](const tint::BindingPoint& src,
const binding::BindingInfo& dst) -> bool {
- if (auto binding = seen_wgsl_bindings.Get(src)) {
- if (*binding != dst) {
- diagnostics.AddError(diag::System::Writer, Source{})
- << "found duplicate WGSL binding point: " << src;
- return true;
- }
+ if (auto binding = seen_wgsl_bindings.Add(src, dst); binding.value != dst) {
+ diagnostics.AddError(diag::System::Writer, Source{})
+ << "found duplicate WGSL binding point: " << src;
+ return true;
}
- seen_wgsl_bindings.Add(src, dst);
return false;
};
auto msl_seen = [&diagnostics](InfoToPointMap& map, const binding::BindingInfo& src,
const tint::BindingPoint& dst) -> bool {
- if (auto binding = map.Get(src)) {
- if (*binding != dst) {
- diagnostics.AddError(diag::System::Writer, Source{})
- << "found duplicate MSL binding point: [binding: " << src.binding << "]";
- return true;
- }
+ if (auto binding = map.Add(src, dst); binding.value != dst) {
+ diagnostics.AddError(diag::System::Writer, Source{})
+ << "found duplicate MSL binding point: [binding: " << src.binding << "]";
+ return true;
}
- map.Add(src, dst);
return false;
};
@@ -159,9 +153,11 @@
//
// When the data comes in we have a list of all WGSL origin (group,binding) pairs to MSL
// (binding) in the `uniform`, `storage`, `texture`, and `sampler` arrays.
-void PopulateRemapperAndMultiplanarOptions(const Options& options,
- RemapperData& remapper_data,
- ExternalTextureOptions& external_texture) {
+void PopulateBindingRelatedOptions(
+ const Options& options,
+ RemapperData& remapper_data,
+ ExternalTextureOptions& external_texture,
+ ArrayLengthFromUniformOptions& array_length_from_uniform_options) {
auto create_remappings = [&remapper_data](const auto& hsh) {
for (const auto& it : hsh) {
const BindingPoint& src_binding_point = it.first;
@@ -208,6 +204,24 @@
remapper_data.emplace(src_binding_point, plane0_binding_point);
}
+
+ // ArrayLengthFromUniformOptions bindpoints may need to be remapped
+ {
+ std::unordered_map<BindingPoint, uint32_t> bindpoint_to_size_index;
+ for (auto& [bindpoint, index] : options.array_length_from_uniform.bindpoint_to_size_index) {
+ auto it = remapper_data.find(bindpoint);
+ if (it != remapper_data.end()) {
+ bindpoint_to_size_index.emplace(it->second, index);
+ } else {
+ bindpoint_to_size_index.emplace(bindpoint, index);
+ }
+ }
+
+ array_length_from_uniform_options.ubo_binding =
+ options.array_length_from_uniform.ubo_binding;
+ array_length_from_uniform_options.bindpoint_to_size_index =
+ std::move(bindpoint_to_size_index);
+ }
}
} // namespace tint::msl::writer
diff --git a/src/tint/lang/msl/writer/common/option_helpers.h b/src/tint/lang/msl/writer/common/option_helpers.h
index 6f21fbd..f4d6dee 100644
--- a/src/tint/lang/msl/writer/common/option_helpers.h
+++ b/src/tint/lang/msl/writer/common/option_helpers.h
@@ -45,14 +45,17 @@
/// @returns success or failure
Result<SuccessType> ValidateBindingOptions(const Options& options);
-/// Populates data from the writer options for the remapper and external texture.
+/// Populates binding-related option from the writer options
/// @param options the writer options
/// @param remapper_data where to put the remapper data
/// @param external_texture where to store the external texture options
+/// @param array_length_from_uniform_options where to store the ArrayLengthFromUniform options
/// Note, these are populated together because there are dependencies between the two types of data.
-void PopulateRemapperAndMultiplanarOptions(const Options& options,
- RemapperData& remapper_data,
- ExternalTextureOptions& external_texture);
+void PopulateBindingRelatedOptions(
+ const Options& options,
+ RemapperData& remapper_data,
+ ExternalTextureOptions& external_texture,
+ ArrayLengthFromUniformOptions& array_length_from_uniform_options);
} // namespace tint::msl::writer
diff --git a/src/tint/lang/msl/writer/common/options.h b/src/tint/lang/msl/writer/common/options.h
index 083206c..adfbf72 100644
--- a/src/tint/lang/msl/writer/common/options.h
+++ b/src/tint/lang/msl/writer/common/options.h
@@ -31,7 +31,6 @@
#include <unordered_map>
#include "src/tint/api/common/binding_point.h"
-#include "src/tint/api/options/array_length_from_uniform.h"
#include "src/tint/api/options/pixel_local.h"
#include "src/tint/utils/reflection/reflection.h"
@@ -112,6 +111,19 @@
TINT_REFLECT(Bindings, uniform, storage, texture, storage_texture, sampler, external_texture);
};
+/// Options used to specify a mapping of binding points to indices into a UBO
+/// from which to load buffer sizes.
+struct ArrayLengthFromUniformOptions {
+ /// The MSL binding point to use to generate a uniform buffer from which to read buffer sizes.
+ uint32_t ubo_binding;
+ /// The mapping from the storage buffer binding points in WGSL binding-point space to the index
+ /// into the uniform buffer where the length of the buffer is stored.
+ std::unordered_map<BindingPoint, uint32_t> bindpoint_to_size_index;
+
+ /// Reflect the fields of this class so that it can be used by tint::ForeachField()
+ TINT_REFLECT(ArrayLengthFromUniformOptions, ubo_binding, bindpoint_to_size_index);
+};
+
/// Configuration options used for generating MSL.
struct Options {
/// Constructor
diff --git a/src/tint/lang/msl/writer/common/options_test.cc b/src/tint/lang/msl/writer/common/options_test.cc
index 6cd8632..1dc826f 100644
--- a/src/tint/lang/msl/writer/common/options_test.cc
+++ b/src/tint/lang/msl/writer/common/options_test.cc
@@ -33,6 +33,7 @@
namespace {
TEST(TintCheckAllFieldsReflected, MslWriterCommonOptionsTest) {
+ TINT_ASSERT_ALL_FIELDS_REFLECTED(ArrayLengthFromUniformOptions);
TINT_ASSERT_ALL_FIELDS_REFLECTED(binding::BindingInfo);
TINT_ASSERT_ALL_FIELDS_REFLECTED(binding::ExternalTexture);
TINT_ASSERT_ALL_FIELDS_REFLECTED(Bindings);
diff --git a/src/tint/lang/msl/writer/raise/raise.cc b/src/tint/lang/msl/writer/raise/raise.cc
index 63591e7..fab831d 100644
--- a/src/tint/lang/msl/writer/raise/raise.cc
+++ b/src/tint/lang/msl/writer/raise/raise.cc
@@ -56,7 +56,9 @@
ExternalTextureOptions external_texture_options{};
RemapperData remapper_data{};
- PopulateRemapperAndMultiplanarOptions(options, remapper_data, external_texture_options);
+ ArrayLengthFromUniformOptions array_length_from_uniform_options{};
+ PopulateBindingRelatedOptions(options, remapper_data, external_texture_options,
+ array_length_from_uniform_options);
RUN_TRANSFORM(core::ir::transform::BindingRemapper, remapper_data);
{
diff --git a/src/tint/lang/msl/writer/writer_bench.cc b/src/tint/lang/msl/writer/writer_bench.cc
index 455b501..47d07a1 100644
--- a/src/tint/lang/msl/writer/writer_bench.cc
+++ b/src/tint/lang/msl/writer/writer_bench.cc
@@ -45,7 +45,7 @@
auto& program = res->program;
tint::msl::writer::Options gen_options = {};
- gen_options.array_length_from_uniform.ubo_binding = tint::BindingPoint{0, 30};
+ gen_options.array_length_from_uniform.ubo_binding = 30;
gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(tint::BindingPoint{0, 0},
0);
gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(tint::BindingPoint{0, 1},
