tint: Simplify backend Sanitize functions by passing in Options

Reduces the amount of variable duplication and copying we do.

Bug: tint:1495
Change-Id: I7999eadf09dc899361926e01dea715e9edc124c9
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/86203
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>

src/tint/writer/hlsl/generator.cc

@@ -31,10 +31,7 @@ Result Generate(const Program* program, const Options& options) {
   Result result;
 
   // Sanitize the program.
-  auto sanitized_result = Sanitize(program, options.root_constant_binding_point,
+  auto sanitized_result = Sanitize(program, options);
-                                   options.disable_workgroup_init,
-                                   options.generate_external_texture_bindings,
-                                   options.array_length_from_uniform);
   if (!sanitized_result.program.IsValid()) {
     result.success = false;
     result.error = sanitized_result.program.Diagnostics().str();

src/tint/writer/hlsl/generator_impl.cc

@@ -134,12 +134,7 @@ SanitizedResult::SanitizedResult() = default;
 SanitizedResult::~SanitizedResult() = default;
 SanitizedResult::SanitizedResult(SanitizedResult&&) = default;
 
-SanitizedResult Sanitize(
+SanitizedResult Sanitize(const Program* in, const Options& options) {
-    const Program* in,
-    sem::BindingPoint root_constant_binding_point,
-    bool disable_workgroup_init,
-    bool generate_external_texture_bindings,
-    const ArrayLengthFromUniformOptions& array_length_from_uniform) {
   transform::Manager manager;
   transform::DataMap data;
 
@@ -158,12 +153,13 @@ SanitizedResult Sanitize(
   }
 
   // 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);
   array_length_from_uniform_cfg.bindpoint_to_size_index =
       array_length_from_uniform.bindpoint_to_size_index;
 
-  if (generate_external_texture_bindings) {
+  if (options.generate_external_texture_bindings) {
     auto new_bindings_map = GenerateExternalTextureBindings(in);
     data.Add<transform::MultiplanarExternalTexture::NewBindingPoints>(
         new_bindings_map);
@@ -186,7 +182,7 @@ SanitizedResult Sanitize(
   manager.Add<transform::FoldTrivialSingleUseLets>();
   manager.Add<transform::LoopToForLoop>();
 
-  if (!disable_workgroup_init) {
+  if (!options.disable_workgroup_init) {
     // ZeroInitWorkgroupMemory must come before CanonicalizeEntryPointIO as
     // ZeroInitWorkgroupMemory may inject new builtin parameters.
     manager.Add<transform::ZeroInitWorkgroupMemory>();
@@ -227,7 +223,7 @@ SanitizedResult Sanitize(
   data.Add<transform::CanonicalizeEntryPointIO::Config>(
       transform::CanonicalizeEntryPointIO::ShaderStyle::kHlsl);
   data.Add<transform::NumWorkgroupsFromUniform::Config>(
-      root_constant_binding_point);
+      options.root_constant_binding_point);
 
   auto out = manager.Run(in, data);
 

src/tint/writer/hlsl/generator_impl.h

@@ -37,6 +37,7 @@
 #include "src/tint/transform/decompose_memory_access.h"
 #include "src/tint/utils/hash.h"
 #include "src/tint/writer/array_length_from_uniform_options.h"
+#include "src/tint/writer/hlsl/generator.h"
 #include "src/tint/writer/text_generator.h"
 
 // Forward declarations
@@ -66,16 +67,10 @@ struct SanitizedResult {
 };
 
 /// Sanitize a program in preparation for generating HLSL.
-/// @param root_constant_binding_point the binding point to use for information
+/// @program The program to sanitize
-/// that will be passed via root constants
+/// @param options The HLSL generator options.
-/// @param disable_workgroup_init `true` to disable workgroup memory zero
 /// @returns the sanitized program and any supplementary information
-SanitizedResult Sanitize(
+SanitizedResult Sanitize(const Program* program, const Options& options);
-    const Program* program,
-    sem::BindingPoint root_constant_binding_point = {},
-    bool disable_workgroup_init = false,
-    bool generate_external_texture_bindings = false,
-    const ArrayLengthFromUniformOptions& array_length_from_uniform = {});
 
 /// Implementation class for HLSL generator
 class GeneratorImpl : public TextGenerator {

src/tint/writer/hlsl/test_helper.h

@@ -76,11 +76,7 @@ class TestHelperBase : public BODY, public ProgramBuilder {
           << formatter.format(program->Diagnostics());
     }();
 
-    auto sanitized_result =
+    auto sanitized_result = Sanitize(program.get(), options);
-        Sanitize(program.get(), options.root_constant_binding_point,
-                 options.disable_workgroup_init,
-                 options.generate_external_texture_bindings,
-                 options.array_length_from_uniform);
     [&]() {
       ASSERT_TRUE(sanitized_result.program.IsValid())
           << formatter.format(sanitized_result.program.Diagnostics());

src/tint/writer/msl/generator.cc

@@ -33,11 +33,7 @@ Result Generate(const Program* program, const Options& options) {
   Result result;
 
   // Sanitize the program.
-  auto sanitized_result = Sanitize(
+  auto sanitized_result = Sanitize(program, options);
-      program, options.buffer_size_ubo_index, options.fixed_sample_mask,
-      options.emit_vertex_point_size, options.disable_workgroup_init,
-      options.generate_external_texture_bindings,
-      options.array_length_from_uniform);
   if (!sanitized_result.program.IsValid()) {
     result.success = false;
     result.error = sanitized_result.program.Diagnostics().str();

src/tint/writer/msl/generator_impl.cc

@@ -119,14 +119,7 @@ SanitizedResult::SanitizedResult() = default;
 SanitizedResult::~SanitizedResult() = default;
 SanitizedResult::SanitizedResult(SanitizedResult&&) = default;
 
-SanitizedResult Sanitize(
+SanitizedResult Sanitize(const Program* in, const Options& options) {
-    const Program* in,
-    uint32_t buffer_size_ubo_index,
-    uint32_t fixed_sample_mask,
-    bool emit_vertex_point_size,
-    bool disable_workgroup_init,
-    bool generate_external_texture_bindings,
-    const ArrayLengthFromUniformOptions& array_length_from_uniform) {
   transform::Manager manager;
   transform::DataMap data;
 
@@ -142,6 +135,7 @@ SanitizedResult Sanitize(
   }
 
   // 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()) {
@@ -152,7 +146,7 @@ SanitizedResult Sanitize(
     // 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, buffer_size_ubo_index});
+        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);
@@ -165,10 +159,10 @@ SanitizedResult Sanitize(
 
   // Build the configs for the internal CanonicalizeEntryPointIO transform.
   auto entry_point_io_cfg = transform::CanonicalizeEntryPointIO::Config(
-      transform::CanonicalizeEntryPointIO::ShaderStyle::kMsl, fixed_sample_mask,
+      transform::CanonicalizeEntryPointIO::ShaderStyle::kMsl,
-      emit_vertex_point_size);
+      options.fixed_sample_mask, options.emit_vertex_point_size);
 
-  if (generate_external_texture_bindings) {
+  if (options.generate_external_texture_bindings) {
     auto new_bindings_map = GenerateExternalTextureBindings(in);
     data.Add<transform::MultiplanarExternalTexture::NewBindingPoints>(
         new_bindings_map);
@@ -177,7 +171,7 @@ SanitizedResult Sanitize(
 
   manager.Add<transform::Unshadow>();
 
-  if (!disable_workgroup_init) {
+  if (!options.disable_workgroup_init) {
     // ZeroInitWorkgroupMemory must come before CanonicalizeEntryPointIO as
     // ZeroInitWorkgroupMemory may inject new builtin parameters.
     manager.Add<transform::ZeroInitWorkgroupMemory>();

src/tint/writer/msl/generator_impl.h

@@ -39,6 +39,7 @@
 #include "src/tint/scope_stack.h"
 #include "src/tint/sem/struct.h"
 #include "src/tint/writer/array_length_from_uniform_options.h"
+#include "src/tint/writer/msl/generator.h"
 #include "src/tint/writer/text_generator.h"
 
 // Forward declarations
@@ -70,19 +71,10 @@ struct SanitizedResult {
 };
 
 /// Sanitize a program in preparation for generating MSL.
-/// @param buffer_size_ubo_index the index to use for the buffer size UBO
+/// @program The program to sanitize
-/// @param fixed_sample_mask the fixed sample mask to use for fragment shaders
+/// @param options The MSL generator options.
-/// @param emit_vertex_point_size `true` to emit a vertex point size builtin
-/// @param disable_workgroup_init `true` to disable workgroup memory zero
 /// @returns the sanitized program and any supplementary information
-SanitizedResult Sanitize(
+SanitizedResult Sanitize(const Program* program, const Options& options);
-    const Program* program,
-    uint32_t buffer_size_ubo_index,
-    uint32_t fixed_sample_mask = 0xFFFFFFFF,
-    bool emit_vertex_point_size = false,
-    bool disable_workgroup_init = false,
-    bool generate_external_texture_bindings = false,
-    const ArrayLengthFromUniformOptions& array_length_from_uniform = {});
 
 /// Implementation class for MSL generator
 class GeneratorImpl : public TextGenerator {

src/tint/writer/msl/test_helper.h

@@ -74,11 +74,7 @@ class TestHelperBase : public BASE, public ProgramBuilder {
           << diag::Formatter().format(program->Diagnostics());
     }();
 
-    auto result = Sanitize(
+    auto result = Sanitize(program.get(), options);
-        program.get(), options.buffer_size_ubo_index, options.fixed_sample_mask,
-        options.emit_vertex_point_size, options.disable_workgroup_init,
-        options.generate_external_texture_bindings,
-        options.array_length_from_uniform);
     [&]() {
       ASSERT_TRUE(result.program.IsValid())
           << diag::Formatter().format(result.program.Diagnostics());

src/tint/writer/spirv/builder.cc

@@ -255,10 +255,7 @@ const sem::Type* ElementTypeOf(const sem::Type* ty) {
 
 }  // namespace
 
-SanitizedResult Sanitize(const Program* in,
+SanitizedResult Sanitize(const Program* in, const Options& options) {
-                         bool emit_vertex_point_size,
-                         bool disable_workgroup_init,
-                         bool generate_external_texture_bindings) {
   transform::Manager manager;
   transform::DataMap data;
 
@@ -275,7 +272,7 @@ SanitizedResult Sanitize(const Program* in,
     manager.Add<transform::BuiltinPolyfill>();
   }
 
-  if (generate_external_texture_bindings) {
+  if (options.generate_external_texture_bindings) {
     auto new_bindings_map = GenerateExternalTextureBindings(in);
     data.Add<transform::MultiplanarExternalTexture::NewBindingPoints>(
         new_bindings_map);
@@ -283,7 +280,10 @@ SanitizedResult Sanitize(const Program* in,
   manager.Add<transform::MultiplanarExternalTexture>();
 
   manager.Add<transform::Unshadow>();
-  if (!disable_workgroup_init) {
+  bool disable_workgroup_init_in_sanitizer =
+      options.disable_workgroup_init ||
+      options.use_zero_initialize_workgroup_memory_extension;
+  if (!disable_workgroup_init_in_sanitizer) {
     manager.Add<transform::ZeroInitWorkgroupMemory>();
   }
   manager.Add<transform::RemoveUnreachableStatements>();
@@ -303,7 +303,7 @@ SanitizedResult Sanitize(const Program* in,
   data.Add<transform::CanonicalizeEntryPointIO::Config>(
       transform::CanonicalizeEntryPointIO::Config(
           transform::CanonicalizeEntryPointIO::ShaderStyle::kSpirv, 0xFFFFFFFF,
-          emit_vertex_point_size));
+          options.emit_vertex_point_size));
 
   SanitizedResult result;
   result.program = std::move(manager.Run(in, data).program);

src/tint/writer/spirv/builder.h

@@ -38,6 +38,7 @@
 #include "src/tint/sem/builtin.h"
 #include "src/tint/sem/storage_texture_type.h"
 #include "src/tint/writer/spirv/function.h"
+#include "src/tint/writer/spirv/generator.h"
 #include "src/tint/writer/spirv/scalar_constant.h"
 
 // Forward declarations
@@ -57,13 +58,9 @@ struct SanitizedResult {
 };
 
 /// Sanitize a program in preparation for generating SPIR-V.
-/// @param emit_vertex_point_size `true` to emit a vertex point size builtin
+/// @program The program to sanitize
-/// @param disable_workgroup_init `true` to disable workgroup memory zero
+/// @param options The SPIR-V generator options.
-/// @returns the sanitized program and any supplementary information
+SanitizedResult Sanitize(const Program* program, const Options& options);
-SanitizedResult Sanitize(const Program* program,
-                         bool emit_vertex_point_size = false,
-                         bool disable_workgroup_init = false,
-                         bool generate_external_texture_bindings = false);
 
 /// Builder class to create SPIR-V instructions from a module.
 class Builder {

src/tint/writer/spirv/builder_entry_point_test.cc

@@ -221,30 +221,32 @@ TEST_F(BuilderTest, EntryPoint_SharedStruct) {
 
   EXPECT_EQ(DumpBuilder(b), R"(OpCapability Shader
 OpMemoryModel Logical GLSL450
-OpEntryPoint Vertex %22 "vert_main" %1 %5
+OpEntryPoint Vertex %23 "vert_main" %1 %5 %9
-OpEntryPoint Fragment %32 "frag_main" %9 %11 %13
+OpEntryPoint Fragment %34 "frag_main" %10 %12 %14
-OpExecutionMode %32 OriginUpperLeft
+OpExecutionMode %34 OriginUpperLeft
-OpExecutionMode %32 DepthReplacing
+OpExecutionMode %34 DepthReplacing
 OpName %1 "value_1"
 OpName %5 "pos_1"
-OpName %9 "value_2"
+OpName %9 "vertex_point_size"
-OpName %11 "pos_2"
+OpName %10 "value_2"
-OpName %13 "value_3"
+OpName %12 "pos_2"
-OpName %15 "Interface"
+OpName %14 "value_3"
-OpMemberName %15 0 "value"
+OpName %16 "Interface"
-OpMemberName %15 1 "pos"
+OpMemberName %16 0 "value"
-OpName %16 "vert_main_inner"
+OpMemberName %16 1 "pos"
-OpName %22 "vert_main"
+OpName %17 "vert_main_inner"
-OpName %28 "frag_main_inner"
+OpName %23 "vert_main"
-OpName %29 "inputs"
+OpName %30 "frag_main_inner"
-OpName %32 "frag_main"
+OpName %31 "inputs"
+OpName %34 "frag_main"
 OpDecorate %1 Location 1
 OpDecorate %5 BuiltIn Position
-OpDecorate %9 Location 1
+OpDecorate %9 BuiltIn PointSize
-OpDecorate %11 BuiltIn FragCoord
+OpDecorate %10 Location 1
-OpDecorate %13 BuiltIn FragDepth
+OpDecorate %12 BuiltIn FragCoord
-OpMemberDecorate %15 0 Offset 0
+OpDecorate %14 BuiltIn FragDepth
-OpMemberDecorate %15 1 Offset 16
+OpMemberDecorate %16 0 Offset 0
+OpMemberDecorate %16 1 Offset 16
 %3 = OpTypeFloat 32
 %2 = OpTypePointer Output %3
 %4 = OpConstantNull %3
@@ -253,44 +255,47 @@ OpMemberDecorate %15 1 Offset 16
 %6 = OpTypePointer Output %7
 %8 = OpConstantNull %7
 %5 = OpVariable %6 Output %8
-%10 = OpTypePointer Input %3
+%9 = OpVariable %2 Output %4
-%9 = OpVariable %10 Input
+%11 = OpTypePointer Input %3
-%12 = OpTypePointer Input %7
+%10 = OpVariable %11 Input
-%11 = OpVariable %12 Input
+%13 = OpTypePointer Input %7
-%13 = OpVariable %2 Output %4
+%12 = OpVariable %13 Input
-%15 = OpTypeStruct %3 %7
+%14 = OpVariable %2 Output %4
-%14 = OpTypeFunction %15
+%16 = OpTypeStruct %3 %7
-%18 = OpConstant %3 42
+%15 = OpTypeFunction %16
-%19 = OpConstantComposite %15 %18 %8
+%19 = OpConstant %3 42
-%21 = OpTypeVoid
+%20 = OpConstantComposite %16 %19 %8
-%20 = OpTypeFunction %21
+%22 = OpTypeVoid
-%27 = OpTypeFunction %3 %15
+%21 = OpTypeFunction %22
-%16 = OpFunction %15 None %14
+%28 = OpConstant %3 1
-%17 = OpLabel
+%29 = OpTypeFunction %3 %16
-OpReturnValue %19
+%17 = OpFunction %16 None %15
+%18 = OpLabel
+OpReturnValue %20
 OpFunctionEnd
-%22 = OpFunction %21 None %20
+%23 = OpFunction %22 None %21
-%23 = OpLabel
+%24 = OpLabel
-%24 = OpFunctionCall %15 %16
+%25 = OpFunctionCall %16 %17
-%25 = OpCompositeExtract %3 %24 0
+%26 = OpCompositeExtract %3 %25 0
-OpStore %1 %25
+OpStore %1 %26
-%26 = OpCompositeExtract %7 %24 1
+%27 = OpCompositeExtract %7 %25 1
-OpStore %5 %26
+OpStore %5 %27
+OpStore %9 %28
 OpReturn
 OpFunctionEnd
-%28 = OpFunction %3 None %27
+%30 = OpFunction %3 None %29
-%29 = OpFunctionParameter %15
+%31 = OpFunctionParameter %16
-%30 = OpLabel
+%32 = OpLabel
-%31 = OpCompositeExtract %3 %29 0
+%33 = OpCompositeExtract %3 %31 0
-OpReturnValue %31
+OpReturnValue %33
 OpFunctionEnd
-%32 = OpFunction %21 None %20
+%34 = OpFunction %22 None %21
-%33 = OpLabel
+%35 = OpLabel
-%35 = OpLoad %3 %9
+%37 = OpLoad %3 %10
-%36 = OpLoad %7 %11
+%38 = OpLoad %7 %12
-%37 = OpCompositeConstruct %15 %35 %36
+%39 = OpCompositeConstruct %16 %37 %38
-%34 = OpFunctionCall %3 %28 %37
+%36 = OpFunctionCall %3 %30 %39
-OpStore %13 %34
+OpStore %14 %36
 OpReturn
 OpFunctionEnd
 )");

src/tint/writer/spirv/generator.cc

@@ -26,12 +26,7 @@ Result Generate(const Program* program, const Options& options) {
   Result result;
 
   // Sanitize the program.
-  bool disable_workgroup_init_in_sanitizer =
+  auto sanitized_result = Sanitize(program, options);
-      options.disable_workgroup_init ||
-      options.use_zero_initialize_workgroup_memory_extension;
-  auto sanitized_result = Sanitize(program, options.emit_vertex_point_size,
-                                   disable_workgroup_init_in_sanitizer,
-                                   options.generate_external_texture_bindings);
   if (!sanitized_result.program.IsValid()) {
     result.success = false;
     result.error = sanitized_result.program.Diagnostics().str();

src/tint/writer/spirv/test_helper.h

@@ -55,10 +55,11 @@ class TestHelperBase : public ProgramBuilder, public BASE {
 
   /// Builds the program, runs the program through the transform::Spirv
   /// sanitizer and returns a spirv::Builder from the sanitized program.
+  /// @param options The SPIR-V generator options.
   /// @note The spirv::Builder is only built once. Multiple calls to Build()
   /// will return the same spirv::Builder without rebuilding.
   /// @return the built spirv::Builder
-  spirv::Builder& SanitizeAndBuild() {
+  spirv::Builder& SanitizeAndBuild(const Options& options = {}) {
     if (spirv_builder) {
       return *spirv_builder;
     }
@@ -71,7 +72,7 @@ class TestHelperBase : public ProgramBuilder, public BASE {
       ASSERT_TRUE(program->IsValid())
           << diag::Formatter().format(program->Diagnostics());
     }();
-    auto result = Sanitize(program.get());
+    auto result = Sanitize(program.get(), options);
     [&]() {
       ASSERT_TRUE(result.program.IsValid())
           << diag::Formatter().format(result.program.Diagnostics());