Move MSL configuration for `ArrayLengthFromUniform` transform.

The configuration for the `ArrayLengthFromUniform` transform was pulled
out to generator options in a previous CL. The HLSL backend was updated
to pass this information into the generator. The MSL backend was using
the deprecated path of having the transform determine the values.

This CL updates the MSL backend to pass the information into the
generator and removes the deprecated code from the transform.

Bug: tint:1855 chromium:1421379

Change-Id: I679c57914d575a758a9ff03b9db27a051d55fe17
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/123880
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/dawn/native/metal/ShaderModuleMTL.mm

@@ -38,6 +38,7 @@ using OptionalVertexPullingTransformConfig = std::optional<tint::transform::Vert
 #define MSL_COMPILATION_REQUEST_MEMBERS(X)                                                  \
     X(SingleShaderStage, stage)                                                             \
     X(const tint::Program*, inputProgram)                                                   \
+    X(tint::writer::ArrayLengthFromUniformOptions, arrayLengthFromUniform)                  \
     X(tint::transform::BindingRemapper::BindingPoints, bindingPoints)                       \
     X(tint::transform::MultiplanarExternalTexture::BindingsMap, externalTextureBindings)    \
     X(OptionalVertexPullingTransformConfig, vertexPullingTransformConfig)                   \
@@ -114,6 +115,9 @@ ResultOrError<CacheResult<MslCompilation>> TranslateToMSL(
     using BindingPoint = tint::writer::BindingPoint;
     BindingRemapper::BindingPoints bindingPoints;
 
+    tint::writer::ArrayLengthFromUniformOptions arrayLengthFromUniform;
+    arrayLengthFromUniform.ubo_binding = {0, kBufferLengthBufferSlot};
+
     for (BindGroupIndex group : IterateBitSet(layout->GetBindGroupLayoutsMask())) {
         const BindGroupLayoutBase::BindingMap& bindingMap =
             layout->GetBindGroupLayout(group)->GetBindingMap();
@@ -133,6 +137,16 @@ ResultOrError<CacheResult<MslCompilation>> TranslateToMSL(
             if (srcBindingPoint != dstBindingPoint) {
                 bindingPoints.emplace(srcBindingPoint, dstBindingPoint);
             }
+
+            // Use the ShaderIndex as the indices for the buffer size lookups in the array length
+            // uniform transform. This is used to compute the size of variable length arrays in
+            // storage buffers.
+            if (bindingInfo.buffer.type == wgpu::BufferBindingType::Storage ||
+                bindingInfo.buffer.type == wgpu::BufferBindingType::ReadOnlyStorage ||
+                bindingInfo.buffer.type == kInternalStorageBufferBinding) {
+                arrayLengthFromUniform.bindpoint_to_size_index.emplace(dstBindingPoint,
+                                                                       dstBindingPoint.binding);
+            }
         }
     }
 
@@ -154,6 +168,11 @@ ResultOrError<CacheResult<MslCompilation>> TranslateToMSL(
             if (srcBindingPoint != dstBindingPoint) {
                 bindingPoints.emplace(srcBindingPoint, dstBindingPoint);
             }
+
+            // Use the ShaderIndex as the indices for the buffer size lookups in the array
+            // length uniform transform.
+            arrayLengthFromUniform.bindpoint_to_size_index.emplace(dstBindingPoint,
+                                                                   dstBindingPoint.binding);
         }
     }
 
@@ -177,6 +196,7 @@ ResultOrError<CacheResult<MslCompilation>> TranslateToMSL(
     req.isRobustnessEnabled = device->IsRobustnessEnabled();
     req.disableSymbolRenaming = device->IsToggleEnabled(Toggle::DisableSymbolRenaming);
     req.tracePlatform = UnsafeUnkeyedValue(device->GetPlatform());
+    req.arrayLengthFromUniform = std::move(arrayLengthFromUniform);
 
     const CombinedLimits& limits = device->GetLimits();
     req.limits = LimitsForCompilationRequest::Create(limits.v1);
@@ -264,6 +284,8 @@ ResultOrError<CacheResult<MslCompilation>> TranslateToMSL(
             options.fixed_sample_mask = r.sampleMask;
             options.disable_workgroup_init = r.disableWorkgroupInit;
             options.emit_vertex_point_size = r.emitVertexPointSize;
+            options.array_length_from_uniform = r.arrayLengthFromUniform;
+
             TRACE_EVENT0(r.tracePlatform.UnsafeGetValue(), General, "tint::writer::msl::Generate");
             auto result = tint::writer::msl::Generate(&program, options);
             DAWN_INVALID_IF(!result.success, "An error occured while generating MSL: %s.",

src/tint/cmd/main.cc

@@ -660,6 +660,11 @@ bool GenerateMsl(const tint::Program* program, const Options& options) {
     gen_options.disable_workgroup_init = options.disable_workgroup_init;
     gen_options.external_texture_options.bindings_map =
         tint::cmd::GenerateExternalTextureBindings(input_program);
+    gen_options.array_length_from_uniform.ubo_binding = tint::writer::BindingPoint{0, 30};
+    gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
+        tint::writer::BindingPoint{0, 0}, 0);
+    gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
+        tint::writer::BindingPoint{0, 1}, 1);
     auto result = tint::writer::msl::Generate(input_program, gen_options);
     if (!result.success) {
         tint::cmd::PrintWGSL(std::cerr, *program);

src/tint/writer/msl/generator_impl.cc

@@ -173,29 +173,6 @@ SanitizedResult Sanitize(const Program* in, const Options& options) {
     // ExpandCompoundAssignment must come before BuiltinPolyfill
     manager.Add<transform::ExpandCompoundAssignment>();
 
-    // Build the config for the internal ArrayLengthFromUniform transform.
-    auto& array_length_from_uniform = options.array_length_from_uniform;
-    transform::ArrayLengthFromUniform::Config array_length_from_uniform_cfg(
-        array_length_from_uniform.ubo_binding);
-    if (!array_length_from_uniform.bindpoint_to_size_index.empty()) {
-        // If |array_length_from_uniform| bindings are provided, use that config.
-        array_length_from_uniform_cfg.bindpoint_to_size_index =
-            array_length_from_uniform.bindpoint_to_size_index;
-    } else {
-        // If the binding map is empty, use the deprecated |buffer_size_ubo_index|
-        // and automatically choose indices using the binding numbers.
-        array_length_from_uniform_cfg = transform::ArrayLengthFromUniform::Config(
-            sem::BindingPoint{0, options.buffer_size_ubo_index});
-        // Use the SSBO binding numbers as the indices for the buffer size lookups.
-        for (auto* var : in->AST().GlobalVariables()) {
-            auto* global = in->Sem().Get<sem::GlobalVariable>(var);
-            if (global && global->AddressSpace() == builtin::AddressSpace::kStorage) {
-                array_length_from_uniform_cfg.bindpoint_to_size_index.emplace(
-                    global->BindingPoint(), global->BindingPoint().binding);
-            }
-        }
-    }
-
     // Build the configs for the internal CanonicalizeEntryPointIO transform.
     auto entry_point_io_cfg = transform::CanonicalizeEntryPointIO::Config(
         transform::CanonicalizeEntryPointIO::ShaderStyle::kMsl, options.fixed_sample_mask,
@@ -210,6 +187,7 @@ SanitizedResult Sanitize(const Program* in, const Options& options) {
     if (!options.disable_robustness) {
         // Robustness must come after PromoteSideEffectsToDecl
         // Robustness must come before BuiltinPolyfill and CanonicalizeEntryPointIO
+        // Robustness must come before ArrayLengthFromUniform
         manager.Add<transform::Robustness>();
     }
 
@@ -260,10 +238,16 @@ SanitizedResult Sanitize(const Program* in, const Options& options) {
     // ArrayLengthFromUniform must come after SimplifyPointers, as
     // it assumes that the form of the array length argument is &var.array.
     manager.Add<transform::ArrayLengthFromUniform>();
+
+    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<transform::ArrayLengthFromUniform::Config>(array_length_cfg);
+
     // PackedVec3 must come after ExpandCompoundAssignment.
     manager.Add<transform::PackedVec3>();
     manager.Add<transform::ModuleScopeVarToEntryPointParam>();
-    data.Add<transform::ArrayLengthFromUniform::Config>(std::move(array_length_from_uniform_cfg));
     data.Add<transform::CanonicalizeEntryPointIO::Config>(std::move(entry_point_io_cfg));
     auto out = manager.Run(in, data);
 

src/tint/writer/msl/generator_impl_sanitizer_test.cc

@@ -39,7 +39,10 @@ TEST_F(MslSanitizerTest, Call_ArrayLength) {
              Stage(ast::PipelineStage::kFragment),
          });
 
-    GeneratorImpl& gen = SanitizeAndBuild();
+    Options opts = DefaultOptions();
+    opts.array_length_from_uniform.ubo_binding = sem::BindingPoint{0, 30};
+    opts.array_length_from_uniform.bindpoint_to_size_index.emplace(sem::BindingPoint{2, 1}, 1);
+    GeneratorImpl& gen = SanitizeAndBuild(opts);
 
     ASSERT_TRUE(gen.Generate()) << gen.error();
 
@@ -93,7 +96,10 @@ TEST_F(MslSanitizerTest, Call_ArrayLength_OtherMembersInStruct) {
              Stage(ast::PipelineStage::kFragment),
          });
 
-    GeneratorImpl& gen = SanitizeAndBuild();
+    Options opts = DefaultOptions();
+    opts.array_length_from_uniform.ubo_binding = sem::BindingPoint{0, 30};
+    opts.array_length_from_uniform.bindpoint_to_size_index.emplace(sem::BindingPoint{2, 1}, 1);
+    GeneratorImpl& gen = SanitizeAndBuild(opts);
 
     ASSERT_TRUE(gen.Generate()) << gen.error();
 
@@ -151,7 +157,10 @@ TEST_F(MslSanitizerTest, Call_ArrayLength_ViaLets) {
              Stage(ast::PipelineStage::kFragment),
          });
 
-    GeneratorImpl& gen = SanitizeAndBuild();
+    Options opts = DefaultOptions();
+    opts.array_length_from_uniform.ubo_binding = sem::BindingPoint{0, 30};
+    opts.array_length_from_uniform.bindpoint_to_size_index.emplace(sem::BindingPoint{2, 1}, 1);
+    GeneratorImpl& gen = SanitizeAndBuild(opts);
 
     ASSERT_TRUE(gen.Generate()) << gen.error();