[tint][spirv-writer] Add a KeepBindingArrayAsPointer transform.
KeepBindingArrayAsPointer is a transform that ensures that binding_arrays
are never stored by value but only used via a pointer to them. This is
used to produce SPIR-V that's more similar to what drivers typically
ingest where OpTypeArray<OpTypeImage> is always kept as a pointer.
Note that it doesn't handle function parameters so DirectVariableAccess
(DVA) for handles must have run prior to this transform.
This mismatch between Tint IR and SPIR-V at the time of writing is
because Tint IR disallows handle address space pointers as function
arguments, while "idiomatic" SPIR-V that drivers are used to use pointers
to pass handle types as function arguments.
Bug: 393558555
Change-Id: I2a37899b938dd773a6d991d6ebf58f30a3a6a5ca
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/247054
Reviewed-by: James Price <jrprice@google.com>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/tint/lang/spirv/writer/raise/BUILD.bazel b/src/tint/lang/spirv/writer/raise/BUILD.bazel
index 1bd116c..a4285df 100644
--- a/src/tint/lang/spirv/writer/raise/BUILD.bazel
+++ b/src/tint/lang/spirv/writer/raise/BUILD.bazel
@@ -43,6 +43,7 @@
"expand_implicit_splats.cc",
"fork_explicit_layout_types.cc",
"handle_matrix_arithmetic.cc",
+ "keep_binding_array_as_pointer.cc",
"merge_return.cc",
"pass_matrix_by_pointer.cc",
"raise.cc",
@@ -55,6 +56,7 @@
"expand_implicit_splats.h",
"fork_explicit_layout_types.h",
"handle_matrix_arithmetic.h",
+ "keep_binding_array_as_pointer.h",
"merge_return.h",
"pass_matrix_by_pointer.h",
"raise.h",
@@ -107,6 +109,7 @@
"expand_implicit_splats_test.cc",
"fork_explicit_layout_types_test.cc",
"handle_matrix_arithmetic_test.cc",
+ "keep_binding_array_as_pointer_test.cc",
"merge_return_test.cc",
"pass_matrix_by_pointer_test.cc",
"remove_unreachable_in_loop_continuing_test.cc",
diff --git a/src/tint/lang/spirv/writer/raise/BUILD.cmake b/src/tint/lang/spirv/writer/raise/BUILD.cmake
index 93f2fef..04f8809 100644
--- a/src/tint/lang/spirv/writer/raise/BUILD.cmake
+++ b/src/tint/lang/spirv/writer/raise/BUILD.cmake
@@ -49,6 +49,8 @@
lang/spirv/writer/raise/fork_explicit_layout_types.h
lang/spirv/writer/raise/handle_matrix_arithmetic.cc
lang/spirv/writer/raise/handle_matrix_arithmetic.h
+ lang/spirv/writer/raise/keep_binding_array_as_pointer.cc
+ lang/spirv/writer/raise/keep_binding_array_as_pointer.h
lang/spirv/writer/raise/merge_return.cc
lang/spirv/writer/raise/merge_return.h
lang/spirv/writer/raise/pass_matrix_by_pointer.cc
@@ -115,6 +117,7 @@
lang/spirv/writer/raise/expand_implicit_splats_test.cc
lang/spirv/writer/raise/fork_explicit_layout_types_test.cc
lang/spirv/writer/raise/handle_matrix_arithmetic_test.cc
+ lang/spirv/writer/raise/keep_binding_array_as_pointer_test.cc
lang/spirv/writer/raise/merge_return_test.cc
lang/spirv/writer/raise/pass_matrix_by_pointer_test.cc
lang/spirv/writer/raise/remove_unreachable_in_loop_continuing_test.cc
diff --git a/src/tint/lang/spirv/writer/raise/BUILD.gn b/src/tint/lang/spirv/writer/raise/BUILD.gn
index afbcfe5..246b677 100644
--- a/src/tint/lang/spirv/writer/raise/BUILD.gn
+++ b/src/tint/lang/spirv/writer/raise/BUILD.gn
@@ -53,6 +53,8 @@
"fork_explicit_layout_types.h",
"handle_matrix_arithmetic.cc",
"handle_matrix_arithmetic.h",
+ "keep_binding_array_as_pointer.cc",
+ "keep_binding_array_as_pointer.h",
"merge_return.cc",
"merge_return.h",
"pass_matrix_by_pointer.cc",
@@ -108,6 +110,7 @@
"expand_implicit_splats_test.cc",
"fork_explicit_layout_types_test.cc",
"handle_matrix_arithmetic_test.cc",
+ "keep_binding_array_as_pointer_test.cc",
"merge_return_test.cc",
"pass_matrix_by_pointer_test.cc",
"remove_unreachable_in_loop_continuing_test.cc",
diff --git a/src/tint/lang/spirv/writer/raise/keep_binding_array_as_pointer.cc b/src/tint/lang/spirv/writer/raise/keep_binding_array_as_pointer.cc
new file mode 100644
index 0000000..ef47bc2
--- /dev/null
+++ b/src/tint/lang/spirv/writer/raise/keep_binding_array_as_pointer.cc
@@ -0,0 +1,104 @@
+// Copyright 2025 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/lang/spirv/writer/raise/keep_binding_array_as_pointer.h"
+
+#include <utility>
+
+#include "src/tint/lang/core/ir/builder.h"
+#include "src/tint/lang/core/ir/module.h"
+#include "src/tint/lang/core/ir/validator.h"
+#include "src/tint/lang/core/type/binding_array.h"
+
+using namespace tint::core::number_suffixes; // NOLINT
+using namespace tint::core::fluent_types; // NOLINT
+
+namespace tint::spirv::writer::raise {
+
+namespace {
+
+/// PIMPL state for the transform.
+struct State {
+ /// The IR module.
+ core::ir::Module& ir;
+
+ /// The IR builder.
+ core::ir::Builder b{ir};
+
+ /// The type manager.
+ core::type::Manager& ty{ir.Types()};
+
+ /// Process the module.
+ void Process() {
+ // Find the access instructions that need replacing.
+ for (auto* inst : ir.Instructions()) {
+ auto* load = inst->As<core::ir::Load>();
+ if (load == nullptr) {
+ continue;
+ }
+
+ auto* ba_type = load->Result()->Type()->As<core::type::BindingArray>();
+ if (ba_type == nullptr) {
+ continue;
+ }
+ TINT_ASSERT(ba_type->IsHandle());
+
+ auto* ba_ptr = load->From();
+ auto* element_ptr_ty = ty.ptr<handle>(ba_type->ElemType());
+
+ load->Result()->ForEachUseUnsorted([&](core::ir::Usage use) {
+ Switch(
+ use.instruction,
+ [&](core::ir::Access* access) {
+ b.InsertBefore(access, [&]() {
+ Vector<core::ir::Value*, 1> indices_copy = access->Indices();
+ auto* element_ptr = b.Access(element_ptr_ty, ba_ptr, indices_copy);
+ b.LoadWithResult(access->DetachResult(), element_ptr);
+ access->Destroy();
+ });
+ },
+ TINT_ICE_ON_NO_MATCH);
+ });
+ load->Destroy();
+ }
+ }
+};
+
+} // namespace
+
+Result<SuccessType> KeepBindingArrayAsPointer(core::ir::Module& ir) {
+ auto result = ValidateAndDumpIfNeeded(ir, "spirv.KeepBindingArrayAsPointer");
+ if (result != Success) {
+ return result;
+ }
+
+ State{ir}.Process();
+
+ return Success;
+}
+
+} // namespace tint::spirv::writer::raise
diff --git a/src/tint/lang/spirv/writer/raise/keep_binding_array_as_pointer.h b/src/tint/lang/spirv/writer/raise/keep_binding_array_as_pointer.h
new file mode 100644
index 0000000..8d3951c
--- /dev/null
+++ b/src/tint/lang/spirv/writer/raise/keep_binding_array_as_pointer.h
@@ -0,0 +1,57 @@
+// Copyright 2025 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_LANG_SPIRV_WRITER_RAISE_KEEP_BINDING_ARRAY_AS_POINTER_H_
+#define SRC_TINT_LANG_SPIRV_WRITER_RAISE_KEEP_BINDING_ARRAY_AS_POINTER_H_
+
+#include "src/tint/utils/result.h"
+
+// Forward declarations.
+namespace tint::core::ir {
+class Module;
+}
+
+namespace tint::spirv::writer::raise {
+
+/// KeepBindingArrayAsPointer is a transform that ensures that binding_arrays are never stored by
+/// value but only used via a pointer to them. This is used to produce SPIR-V that's more similar to
+/// what drivers typically ingest where OpTypeArray<OpTypeImage> is always kept as a pointer.
+///
+/// Note that it doesn't handle function parameters so DirectVariableAccess (DVA) for handles must
+/// have run prior to this transform.
+///
+/// This mismatch between Tint IR and SPIR-V at the time of writing is because Tint IR disallows
+/// handle address space pointers as function arguments, while "idiomatic" SPIR-V that drivers are
+/// used to use pointers to pass handle types as function arguments.
+///
+/// @param module the module to transform
+/// @returns success or failure
+Result<SuccessType> KeepBindingArrayAsPointer(core::ir::Module& module);
+
+} // namespace tint::spirv::writer::raise
+
+#endif // SRC_TINT_LANG_SPIRV_WRITER_RAISE_KEEP_BINDING_ARRAY_AS_POINTER_H_
diff --git a/src/tint/lang/spirv/writer/raise/keep_binding_array_as_pointer_test.cc b/src/tint/lang/spirv/writer/raise/keep_binding_array_as_pointer_test.cc
new file mode 100644
index 0000000..b13368a
--- /dev/null
+++ b/src/tint/lang/spirv/writer/raise/keep_binding_array_as_pointer_test.cc
@@ -0,0 +1,128 @@
+// Copyright 2025 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/lang/spirv/writer/raise/keep_binding_array_as_pointer.h"
+
+#include "src/tint/lang/core/ir/transform/helper_test.h"
+#include "src/tint/lang/core/type/binding_array.h"
+#include "src/tint/lang/core/type/sampled_texture.h"
+
+namespace tint::spirv::writer::raise {
+namespace {
+
+using namespace tint::core::fluent_types; // NOLINT
+using namespace tint::core::number_suffixes; // NOLINT
+
+using SpirvWriter_KeepBindingArrayAsPointer = core::ir::transform::TransformTest;
+
+TEST_F(SpirvWriter_KeepBindingArrayAsPointer, SkippedForPtrToBindingArray) {
+ auto* texture_type = ty.sampled_texture(core::type::TextureDimension::k2d, ty.f32());
+ auto* var_ts = b.Var("ts", ty.ptr<handle>(ty.binding_array(texture_type, 3u)));
+ var_ts->SetBindingPoint(0, 0);
+ b.ir.root_block->Append(var_ts);
+
+ auto* fn = b.Function("f", ty.void_());
+ b.Append(fn->Block(), [&] {
+ auto* t_ptr = b.Access(ty.ptr<handle>(texture_type), var_ts, 0_i);
+ auto* t = b.Load(t_ptr);
+ b.Call(ty.vec4<f32>(), core::BuiltinFn::kTextureLoad, t, b.Zero(ty.vec2<u32>()), 0_u);
+ b.Return(fn);
+ });
+
+ auto* src = R"(
+$B1: { # root
+ %ts:ptr<handle, binding_array<texture_2d<f32>, 3>, read> = var undef @binding_point(0, 0)
+}
+
+%f = func():void {
+ $B2: {
+ %3:ptr<handle, texture_2d<f32>, read> = access %ts, 0i
+ %4:texture_2d<f32> = load %3
+ %5:vec4<f32> = textureLoad %4, vec2<u32>(0u), 0u
+ ret
+ }
+}
+)";
+
+ EXPECT_EQ(src, str());
+
+ Run(KeepBindingArrayAsPointer);
+ EXPECT_EQ(src, str());
+}
+
+TEST_F(SpirvWriter_KeepBindingArrayAsPointer, LoadOfBindingArrayIsReplaced) {
+ auto* texture_type = ty.sampled_texture(core::type::TextureDimension::k2d, ty.f32());
+ auto* var_ts = b.Var("ts", ty.ptr<handle>(ty.binding_array(texture_type, 3u)));
+ var_ts->SetBindingPoint(0, 0);
+ b.ir.root_block->Append(var_ts);
+
+ auto* fn = b.Function("f", ty.void_());
+ b.Append(fn->Block(), [&] {
+ auto* ts = b.Load(var_ts);
+ auto* t = b.Access(texture_type, ts, 0_i);
+ b.Call(ty.vec4<f32>(), core::BuiltinFn::kTextureLoad, t, b.Zero(ty.vec2<u32>()), 0_u);
+ b.Return(fn);
+ });
+
+ auto* src = R"(
+$B1: { # root
+ %ts:ptr<handle, binding_array<texture_2d<f32>, 3>, read> = var undef @binding_point(0, 0)
+}
+
+%f = func():void {
+ $B2: {
+ %3:binding_array<texture_2d<f32>, 3> = load %ts
+ %4:texture_2d<f32> = access %3, 0i
+ %5:vec4<f32> = textureLoad %4, vec2<u32>(0u), 0u
+ ret
+ }
+}
+)";
+
+ EXPECT_EQ(src, str());
+
+ auto* expected = R"(
+$B1: { # root
+ %ts:ptr<handle, binding_array<texture_2d<f32>, 3>, read> = var undef @binding_point(0, 0)
+}
+
+%f = func():void {
+ $B2: {
+ %3:ptr<handle, texture_2d<f32>, read> = access %ts, 0i
+ %4:texture_2d<f32> = load %3
+ %5:vec4<f32> = textureLoad %4, vec2<u32>(0u), 0u
+ ret
+ }
+}
+)";
+
+ Run(KeepBindingArrayAsPointer);
+ EXPECT_EQ(expected, str());
+}
+
+} // namespace
+} // namespace tint::spirv::writer::raise
diff --git a/src/tint/lang/spirv/writer/raise/raise.cc b/src/tint/lang/spirv/writer/raise/raise.cc
index 85fcd0e..7506b7e 100644
--- a/src/tint/lang/spirv/writer/raise/raise.cc
+++ b/src/tint/lang/spirv/writer/raise/raise.cc
@@ -53,6 +53,7 @@
#include "src/tint/lang/spirv/writer/raise/expand_implicit_splats.h"
#include "src/tint/lang/spirv/writer/raise/fork_explicit_layout_types.h"
#include "src/tint/lang/spirv/writer/raise/handle_matrix_arithmetic.h"
+#include "src/tint/lang/spirv/writer/raise/keep_binding_array_as_pointer.h"
#include "src/tint/lang/spirv/writer/raise/merge_return.h"
#include "src/tint/lang/spirv/writer/raise/pass_matrix_by_pointer.h"
#include "src/tint/lang/spirv/writer/raise/remove_unreachable_in_loop_continuing.h"
@@ -148,6 +149,13 @@
dva_options.transform_handle = options.dva_transform_handle;
RUN_TRANSFORM(core::ir::transform::DirectVariableAccess, module, dva_options);
+ // Fixup loads of binding_arrays of handles that may have been introduced by
+ // DirectVariableAccess (DVA). Vulkan drivers that need DVA of handle expect binding_arrays to
+ // stay as pointer and many mishandle by-value binding_arrays.
+ if (options.dva_transform_handle) {
+ RUN_TRANSFORM(raise::KeepBindingArrayAsPointer, module);
+ }
+
if (options.pass_matrix_by_pointer) {
// PassMatrixByPointer must come after PreservePadding+DirectVariableAccess.
RUN_TRANSFORM(raise::PassMatrixByPointer, module);
