msl: Remove the sanitizer transform
Invoke the required transforms directly in the MSL backend.
Change-Id: Id8026b1a64415fbe363f8f8a5790e8216cd12c68
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/63620
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
diff --git a/src/BUILD.gn b/src/BUILD.gn
index baa0cbb..9ad333d 100644
--- a/src/BUILD.gn
+++ b/src/BUILD.gn
@@ -670,8 +670,6 @@
libtint_source_set("libtint_msl_writer_src") {
sources = [
- "transform/msl.cc",
- "transform/msl.h",
"writer/msl/generator.cc",
"writer/msl/generator.h",
"writer/msl/generator_impl.cc",
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 06e7392..8dd1896 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -477,8 +477,6 @@
if(${TINT_BUILD_MSL_WRITER})
list(APPEND TINT_LIB_SRCS
- transform/msl.cc
- transform/msl.h
writer/msl/generator.cc
writer/msl/generator.h
writer/msl/generator_impl.cc
@@ -955,9 +953,6 @@
endif()
if(${TINT_BUILD_MSL_WRITER})
- if(${TINT_BUILD_WGSL_READER} AND ${TINT_BUILD_WGSL_WRITER})
- list(APPEND TINT_TEST_SRCS transform/msl_test.cc)
- endif()
list(APPEND TINT_TEST_SRCS
writer/msl/generator_impl_array_accessor_test.cc
writer/msl/generator_impl_assign_test.cc
diff --git a/src/transform/msl.cc b/src/transform/msl.cc
deleted file mode 100644
index 66c36be..0000000
--- a/src/transform/msl.cc
+++ /dev/null
@@ -1,134 +0,0 @@
-// Copyright 2020 The Tint Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "src/transform/msl.h"
-
-#include <memory>
-#include <unordered_map>
-#include <utility>
-#include <vector>
-
-#include "src/ast/disable_validation_decoration.h"
-#include "src/program_builder.h"
-#include "src/sem/call.h"
-#include "src/sem/function.h"
-#include "src/sem/statement.h"
-#include "src/sem/variable.h"
-#include "src/transform/array_length_from_uniform.h"
-#include "src/transform/canonicalize_entry_point_io.h"
-#include "src/transform/external_texture_transform.h"
-#include "src/transform/inline_pointer_lets.h"
-#include "src/transform/manager.h"
-#include "src/transform/module_scope_var_to_entry_point_param.h"
-#include "src/transform/pad_array_elements.h"
-#include "src/transform/promote_initializers_to_const_var.h"
-#include "src/transform/simplify.h"
-#include "src/transform/wrap_arrays_in_structs.h"
-#include "src/transform/zero_init_workgroup_memory.h"
-
-TINT_INSTANTIATE_TYPEINFO(tint::transform::Msl);
-TINT_INSTANTIATE_TYPEINFO(tint::transform::Msl::Config);
-TINT_INSTANTIATE_TYPEINFO(tint::transform::Msl::Result);
-
-namespace tint {
-namespace transform {
-
-Msl::Msl() = default;
-Msl::~Msl() = default;
-
-Output Msl::Run(const Program* in, const DataMap& inputs) {
- Manager manager;
- DataMap internal_inputs;
-
- auto* cfg = inputs.Get<Config>();
-
- // Build the configs for the internal transforms.
- uint32_t buffer_size_ubo_index = kDefaultBufferSizeUniformIndex;
- uint32_t fixed_sample_mask = 0xFFFFFFFF;
- bool emit_point_size = false;
- if (cfg) {
- buffer_size_ubo_index = cfg->buffer_size_ubo_index;
- fixed_sample_mask = cfg->fixed_sample_mask;
- emit_point_size = cfg->emit_vertex_point_size;
- }
- auto array_length_from_uniform_cfg = ArrayLengthFromUniform::Config(
- sem::BindingPoint{0, buffer_size_ubo_index});
- auto entry_point_io_cfg = CanonicalizeEntryPointIO::Config(
- CanonicalizeEntryPointIO::ShaderStyle::kMsl, fixed_sample_mask,
- emit_point_size);
-
- // 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->StorageClass() == ast::StorageClass::kStorage) {
- array_length_from_uniform_cfg.bindpoint_to_size_index.emplace(
- global->BindingPoint(), global->BindingPoint().binding);
- }
- }
-
- if (!cfg || !cfg->disable_workgroup_init) {
- // ZeroInitWorkgroupMemory must come before CanonicalizeEntryPointIO as
- // ZeroInitWorkgroupMemory may inject new builtin parameters.
- manager.Add<ZeroInitWorkgroupMemory>();
- }
- manager.Add<CanonicalizeEntryPointIO>();
- manager.Add<ExternalTextureTransform>();
- manager.Add<PromoteInitializersToConstVar>();
- manager.Add<WrapArraysInStructs>();
- manager.Add<PadArrayElements>();
- manager.Add<ModuleScopeVarToEntryPointParam>();
- manager.Add<InlinePointerLets>();
- manager.Add<Simplify>();
- // ArrayLengthFromUniform must come after InlinePointerLets and Simplify, as
- // it assumes that the form of the array length argument is &var.array.
- manager.Add<ArrayLengthFromUniform>();
- internal_inputs.Add<ArrayLengthFromUniform::Config>(
- std::move(array_length_from_uniform_cfg));
- internal_inputs.Add<CanonicalizeEntryPointIO::Config>(
- std::move(entry_point_io_cfg));
- auto out = manager.Run(in, internal_inputs);
- if (!out.program.IsValid()) {
- return out;
- }
-
- ProgramBuilder builder;
- CloneContext ctx(&builder, &out.program);
- // TODO(jrprice): Move the sanitizer into the backend.
- ctx.Clone();
-
- auto result = std::make_unique<Result>(
- out.data.Get<ArrayLengthFromUniform::Result>()->needs_buffer_sizes);
-
- builder.SetTransformApplied(this);
- return Output{Program(std::move(builder)), std::move(result)};
-}
-
-Msl::Config::Config(uint32_t buffer_size_ubo_idx,
- uint32_t sample_mask,
- bool emit_point_size,
- bool disable_wi)
- : buffer_size_ubo_index(buffer_size_ubo_idx),
- fixed_sample_mask(sample_mask),
- emit_vertex_point_size(emit_point_size),
- disable_workgroup_init(disable_wi) {}
-Msl::Config::Config(const Config&) = default;
-Msl::Config::~Config() = default;
-
-Msl::Result::Result(bool needs_buffer_sizes)
- : needs_storage_buffer_sizes(needs_buffer_sizes) {}
-Msl::Result::Result(const Result&) = default;
-Msl::Result::~Result() = default;
-
-} // namespace transform
-} // namespace tint
diff --git a/src/transform/msl.h b/src/transform/msl.h
deleted file mode 100644
index 07e7a39..0000000
--- a/src/transform/msl.h
+++ /dev/null
@@ -1,94 +0,0 @@
-// Copyright 2021 The Tint Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#ifndef SRC_TRANSFORM_MSL_H_
-#define SRC_TRANSFORM_MSL_H_
-
-#include "src/transform/transform.h"
-
-namespace tint {
-namespace transform {
-
-/// Msl is a transform used to sanitize a Program for use with the Msl writer.
-/// Passing a non-sanitized Program to the Msl writer will result in undefined
-/// behavior.
-class Msl : public Castable<Msl, Transform> {
- public:
- /// The default buffer slot to use for the storage buffer size buffer.
- const uint32_t kDefaultBufferSizeUniformIndex = 30;
-
- /// Configuration options for the Msl sanitizer transform.
- struct Config : public Castable<Data, transform::Data> {
- /// Constructor
- /// @param buffer_size_ubo_idx the index to use for the buffer size UBO
- /// @param sample_mask the fixed sample mask to use for fragment shaders
- /// @param emit_point_size `true` to emit a vertex point size builtin
- /// @param disable_workgroup_init `true` to disable workgroup memory zero
- /// initialization
- Config(uint32_t buffer_size_ubo_idx,
- uint32_t sample_mask = 0xFFFFFFFF,
- bool emit_point_size = false,
- bool disable_workgroup_init = false);
-
- /// Copy constructor
- Config(const Config&);
-
- /// Destructor
- ~Config() override;
-
- /// The index to use when generating a UBO to receive storage buffer sizes.
- uint32_t buffer_size_ubo_index = 0;
-
- /// The fixed sample mask to combine with fragment shader outputs.
- uint32_t fixed_sample_mask = 0xFFFFFFFF;
-
- /// Set to `true` to generate a [[point_size]] attribute which is set to 1.0
- /// for all vertex shaders in the module.
- bool emit_vertex_point_size = false;
-
- /// Set to `true` to disable workgroup memory zero initialization
- bool disable_workgroup_init = false;
- };
-
- /// Information produced by the sanitizer that users may need to act on.
- struct Result : public Castable<Result, transform::Data> {
- /// Constructor
- /// @param needs_buffer_sizes True if the shader needs a UBO of buffer sizes
- explicit Result(bool needs_buffer_sizes);
-
- /// Copy constructor
- Result(const Result&);
-
- /// Destructor
- ~Result() override;
-
- /// True if the shader needs a UBO of buffer sizes.
- bool const needs_storage_buffer_sizes;
- };
-
- /// Constructor
- Msl();
- ~Msl() override;
-
- /// Runs the transform on `program`, returning the transformation result.
- /// @param program the source program to transform
- /// @param data optional extra transform-specific input data
- /// @returns the transformation result
- Output Run(const Program* program, const DataMap& data = {}) override;
-};
-
-} // namespace transform
-} // namespace tint
-
-#endif // SRC_TRANSFORM_MSL_H_
diff --git a/src/transform/msl_test.cc b/src/transform/msl_test.cc
deleted file mode 100644
index b3db820..0000000
--- a/src/transform/msl_test.cc
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright 2021 The Tint Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "src/transform/msl.h"
-
-#include "src/transform/test_helper.h"
-
-namespace tint {
-namespace transform {
-namespace {
-
-using MslTest = TransformTest;
-
-// TODO(jrprice): Remove this file when we remove the sanitizers.
-
-} // namespace
-} // namespace transform
-} // namespace tint
diff --git a/src/writer/msl/generator.cc b/src/writer/msl/generator.cc
index 61c314a..c898204 100644
--- a/src/writer/msl/generator.cc
+++ b/src/writer/msl/generator.cc
@@ -14,7 +14,6 @@
#include "src/writer/msl/generator.h"
-#include "src/transform/msl.h"
#include "src/writer/msl/generator_impl.h"
namespace tint {
@@ -28,24 +27,20 @@
Result Generate(const Program* program, const Options& options) {
Result result;
- // Run the MSL sanitizer.
- transform::Msl sanitizer;
- transform::DataMap transform_input;
- transform_input.Add<transform::Msl::Config>(
- options.buffer_size_ubo_index, options.fixed_sample_mask,
+ // Sanitize the program.
+ auto sanitized_result = Sanitize(
+ program, options.buffer_size_ubo_index, options.fixed_sample_mask,
options.emit_vertex_point_size, options.disable_workgroup_init);
- auto output = sanitizer.Run(program, transform_input);
- if (!output.program.IsValid()) {
+ if (!sanitized_result.program.IsValid()) {
result.success = false;
- result.error = output.program.Diagnostics().str();
+ result.error = sanitized_result.program.Diagnostics().str();
return result;
}
- auto* transform_output = output.data.Get<transform::Msl::Result>();
result.needs_storage_buffer_sizes =
- transform_output->needs_storage_buffer_sizes;
+ sanitized_result.needs_storage_buffer_sizes;
// Generate the MSL code.
- auto impl = std::make_unique<GeneratorImpl>(&output.program);
+ auto impl = std::make_unique<GeneratorImpl>(&sanitized_result.program);
result.success = impl->Generate();
result.error = impl->error();
result.msl = impl->result();
diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc
index 3d01434..87d7b50 100644
--- a/src/writer/msl/generator_impl.cc
+++ b/src/writer/msl/generator_impl.cc
@@ -55,7 +55,17 @@
#include "src/sem/variable.h"
#include "src/sem/vector_type.h"
#include "src/sem/void_type.h"
-#include "src/transform/msl.h"
+#include "src/transform/array_length_from_uniform.h"
+#include "src/transform/canonicalize_entry_point_io.h"
+#include "src/transform/external_texture_transform.h"
+#include "src/transform/inline_pointer_lets.h"
+#include "src/transform/manager.h"
+#include "src/transform/module_scope_var_to_entry_point_param.h"
+#include "src/transform/pad_array_elements.h"
+#include "src/transform/promote_initializers_to_const_var.h"
+#include "src/transform/simplify.h"
+#include "src/transform/wrap_arrays_in_structs.h"
+#include "src/transform/zero_init_workgroup_memory.h"
#include "src/utils/defer.h"
#include "src/utils/get_or_create.h"
#include "src/utils/scoped_assignment.h"
@@ -103,19 +113,66 @@
};
} // namespace
+SanitizedResult Sanitize(const Program* in,
+ uint32_t buffer_size_ubo_index,
+ uint32_t fixed_sample_mask,
+ bool emit_vertex_point_size,
+ bool disable_workgroup_init) {
+ transform::Manager manager;
+ transform::DataMap internal_inputs;
+
+ // Build the configs for the internal transforms.
+ auto array_length_from_uniform_cfg =
+ transform::ArrayLengthFromUniform::Config(
+ sem::BindingPoint{0, buffer_size_ubo_index});
+ auto entry_point_io_cfg = transform::CanonicalizeEntryPointIO::Config(
+ transform::CanonicalizeEntryPointIO::ShaderStyle::kMsl, fixed_sample_mask,
+ emit_vertex_point_size);
+
+ // 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->StorageClass() == ast::StorageClass::kStorage) {
+ array_length_from_uniform_cfg.bindpoint_to_size_index.emplace(
+ global->BindingPoint(), global->BindingPoint().binding);
+ }
+ }
+
+ if (!disable_workgroup_init) {
+ // ZeroInitWorkgroupMemory must come before CanonicalizeEntryPointIO as
+ // ZeroInitWorkgroupMemory may inject new builtin parameters.
+ manager.Add<transform::ZeroInitWorkgroupMemory>();
+ }
+ manager.Add<transform::CanonicalizeEntryPointIO>();
+ manager.Add<transform::ExternalTextureTransform>();
+ manager.Add<transform::PromoteInitializersToConstVar>();
+ manager.Add<transform::WrapArraysInStructs>();
+ manager.Add<transform::PadArrayElements>();
+ manager.Add<transform::ModuleScopeVarToEntryPointParam>();
+ manager.Add<transform::InlinePointerLets>();
+ manager.Add<transform::Simplify>();
+ // ArrayLengthFromUniform must come after InlinePointerLets and Simplify, as
+ // it assumes that the form of the array length argument is &var.array.
+ manager.Add<transform::ArrayLengthFromUniform>();
+ internal_inputs.Add<transform::ArrayLengthFromUniform::Config>(
+ std::move(array_length_from_uniform_cfg));
+ internal_inputs.Add<transform::CanonicalizeEntryPointIO::Config>(
+ std::move(entry_point_io_cfg));
+ auto out = manager.Run(in, internal_inputs);
+ if (!out.program.IsValid()) {
+ return {std::move(out.program)};
+ }
+
+ return {std::move(out.program),
+ out.data.Get<transform::ArrayLengthFromUniform::Result>()
+ ->needs_buffer_sizes};
+}
+
GeneratorImpl::GeneratorImpl(const Program* program) : TextGenerator(program) {}
GeneratorImpl::~GeneratorImpl() = default;
bool GeneratorImpl::Generate() {
- if (!program_->HasTransformApplied<transform::Msl>()) {
- diagnostics_.add_error(
- diag::System::Writer,
- "MSL writer requires the transform::Msl sanitizer to have been "
- "applied to the input program");
- return false;
- }
-
line() << "#include <metal_stdlib>";
line();
line() << "using namespace metal;";
diff --git a/src/writer/msl/generator_impl.h b/src/writer/msl/generator_impl.h
index 9c65c15..b2be21d 100644
--- a/src/writer/msl/generator_impl.h
+++ b/src/writer/msl/generator_impl.h
@@ -50,6 +50,26 @@
namespace writer {
namespace msl {
+/// The result of sanitizing a program for generation.
+struct SanitizedResult {
+ /// The sanitized program.
+ Program program;
+ /// True if the shader needs a UBO of buffer sizes.
+ bool needs_storage_buffer_sizes = false;
+};
+
+/// Sanitize a program in preparation for generating MSL.
+/// @param buffer_size_ubo_index the index to use for the buffer size UBO
+/// @param fixed_sample_mask the fixed sample mask to use for fragment shaders
+/// @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(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);
+
/// Implementation class for MSL generator
class GeneratorImpl : public TextGenerator {
public:
diff --git a/src/writer/msl/generator_impl_test.cc b/src/writer/msl/generator_impl_test.cc
index 5ac5b73..f5abc64 100644
--- a/src/writer/msl/generator_impl_test.cc
+++ b/src/writer/msl/generator_impl_test.cc
@@ -22,16 +22,6 @@
using MslGeneratorImplTest = TestHelper;
-TEST_F(MslGeneratorImplTest, ErrorIfSanitizerNotRun) {
- auto program = std::make_unique<Program>(std::move(*this));
- GeneratorImpl gen(program.get());
- EXPECT_FALSE(gen.Generate());
- EXPECT_EQ(
- gen.error(),
- "error: MSL writer requires the transform::Msl sanitizer to have been "
- "applied to the input program");
-}
-
TEST_F(MslGeneratorImplTest, Generate) {
Func("my_func", ast::VariableList{}, ty.void_(), ast::StatementList{},
ast::DecorationList{
diff --git a/src/writer/msl/test_helper.h b/src/writer/msl/test_helper.h
index 7400467..762b2b6 100644
--- a/src/writer/msl/test_helper.h
+++ b/src/writer/msl/test_helper.h
@@ -21,7 +21,6 @@
#include "gtest/gtest.h"
#include "src/program_builder.h"
-#include "src/transform/msl.h"
#include "src/writer/msl/generator_impl.h"
namespace tint {
@@ -43,9 +42,6 @@
if (gen_) {
return *gen_;
}
- // Fake that the MSL sanitizer has been applied, so that we can unit test
- // the writer without it erroring.
- SetTransformApplied<transform::Msl>();
[&]() {
ASSERT_TRUE(IsValid()) << "Builder program is not valid\n"
<< diag::Formatter().format(Diagnostics());
@@ -78,7 +74,7 @@
<< diag::Formatter().format(program->Diagnostics());
}();
- auto result = transform::Msl().Run(program.get());
+ auto result = Sanitize(program.get(), 30);
[&]() {
ASSERT_TRUE(result.program.IsValid())
<< diag::Formatter().format(result.program.Diagnostics());
diff --git a/test/BUILD.gn b/test/BUILD.gn
index ad410e7..8d3ee1f 100644
--- a/test/BUILD.gn
+++ b/test/BUILD.gn
@@ -548,7 +548,6 @@
tint_unittests_source_set("tint_unittests_msl_writer_src") {
sources = [
- "../src/transform/msl_test.cc",
"../src/writer/msl/generator_impl_array_accessor_test.cc",
"../src/writer/msl/generator_impl_assign_test.cc",
"../src/writer/msl/generator_impl_binary_test.cc",
