From ad22d567e54af729bbb5ee6b80f7efb66bc6c5e9 Mon Sep 17 00:00:00 2001
From: Ben Clayton <bclayton@google.com>
Date: Thu, 25 May 2023 09:56:04 +0000
Subject: [PATCH] [tint][bench]: Fix MSL benchmarks

These required binding point remapping so that the @group is always zero.

Also: Use a unique_ptr for the file in ProgramAndFile. Previously the diagnostics were referring to the std::moved file, causing segfaults if the benchmark program errors and printed diagnostics.
Change-Id: Id0e41665b97b3fc73a6cdd5848c5f505cd77e805
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/134280
Kokoro: Ben Clayton <bclayton@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
---
 src/tint/bench/benchmark.cc            |  4 +-
 src/tint/bench/benchmark.h             |  2 +-
 src/tint/writer/msl/generator_bench.cc | 52 ++++++++++++++++----------
 3 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/src/tint/bench/benchmark.cc b/src/tint/bench/benchmark.cc
index 2338681297..4804a712bf 100644
--- a/src/tint/bench/benchmark.cc
+++ b/src/tint/bench/benchmark.cc
@@ -120,8 +120,8 @@ std::variant<ProgramAndFile, Error> LoadProgram(std::string name) {
     if (auto err = std::get_if<bench::Error>(&res)) {
         return *err;
     }
-    auto& file = std::get<Source::File>(res);
-    auto program = reader::wgsl::Parse(&file);
+    auto file = std::make_unique<Source::File>(std::move(std::get<Source::File>(res)));
+    auto program = reader::wgsl::Parse(file.get());
     if (program.Diagnostics().contains_errors()) {
         return Error{program.Diagnostics().str()};
     }
diff --git a/src/tint/bench/benchmark.h b/src/tint/bench/benchmark.h
index 4a0b29e4b6..acb37ce5ff 100644
--- a/src/tint/bench/benchmark.h
+++ b/src/tint/bench/benchmark.h
@@ -36,7 +36,7 @@ struct ProgramAndFile {
     /// The tint program parsed from file.
     Program program;
     /// The source file
-    Source::File file;
+    std::unique_ptr<Source::File> file;
 };
 
 /// LoadInputFile attempts to load a benchmark input file with the given file
diff --git a/src/tint/writer/msl/generator_bench.cc b/src/tint/writer/msl/generator_bench.cc
index a6242ddb3e..98d5ece965 100644
--- a/src/tint/writer/msl/generator_bench.cc
+++ b/src/tint/writer/msl/generator_bench.cc
@@ -14,7 +14,9 @@
 
 #include <string>
 
+#include "src/tint/ast/module.h"
 #include "src/tint/bench/benchmark.h"
+#include "src/tint/sem/variable.h"
 
 namespace tint::writer::msl {
 namespace {
@@ -26,26 +28,38 @@ void GenerateMSL(benchmark::State& state, std::string input_name) {
         return;
     }
     auto& program = std::get<bench::ProgramAndFile>(res).program;
-    for (auto _ : state) {
-        tint::writer::msl::Options gen_options = {};
-        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);
-        gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
-            tint::writer::BindingPoint{0, 2}, 2);
-        gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
-            tint::writer::BindingPoint{0, 3}, 3);
-        gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
-            tint::writer::BindingPoint{0, 4}, 4);
-        gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
-            tint::writer::BindingPoint{0, 5}, 5);
-        gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
-            tint::writer::BindingPoint{0, 6}, 6);
-        gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
-            tint::writer::BindingPoint{0, 7}, 7);
 
+    tint::writer::msl::Options gen_options = {};
+    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);
+    gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
+        tint::writer::BindingPoint{0, 2}, 2);
+    gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
+        tint::writer::BindingPoint{0, 3}, 3);
+    gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
+        tint::writer::BindingPoint{0, 4}, 4);
+    gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
+        tint::writer::BindingPoint{0, 5}, 5);
+    gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
+        tint::writer::BindingPoint{0, 6}, 6);
+    gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(
+        tint::writer::BindingPoint{0, 7}, 7);
+
+    uint32_t next_binding_point = 0;
+    for (auto* var : program.AST().GlobalVariables()) {
+        if (auto* var_sem = program.Sem().Get(var)->As<sem::GlobalVariable>()) {
+            if (auto bp = var_sem->BindingPoint()) {
+                gen_options.binding_remapper_options.binding_points[*bp] = sem::BindingPoint{
+                    0,                     // group
+                    next_binding_point++,  // binding
+                };
+            }
+        }
+    }
+    for (auto _ : state) {
         auto res = Generate(&program, gen_options);
         if (!res.error.empty()) {
             state.SkipWithError(res.error.c_str());