diff --git a/src/BUILD.gn b/src/BUILD.gn index baa0cbbbcd..9ad333dc03 100644 --- a/src/BUILD.gn +++ b/src/BUILD.gn @@ -670,8 +670,6 @@ libtint_source_set("libtint_wgsl_writer_src") { 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 06e7392f24..8dd18966c4 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -477,8 +477,6 @@ endif() 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 @@ if(${TINT_BUILD_TESTS}) 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 66c36be48c..0000000000 --- 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 -#include -#include -#include - -#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(); - - // 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(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(); - } - manager.Add(); - manager.Add(); - manager.Add(); - manager.Add(); - manager.Add(); - manager.Add(); - manager.Add(); - manager.Add(); - // ArrayLengthFromUniform must come after InlinePointerLets and Simplify, as - // it assumes that the form of the array length argument is &var.array. - manager.Add(); - internal_inputs.Add( - std::move(array_length_from_uniform_cfg)); - internal_inputs.Add( - 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( - out.data.Get()->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 07e7a393d6..0000000000 --- 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 { - 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 { - /// 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 { - /// 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 b3db8201fa..0000000000 --- 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 61c314adba..c898204a11 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::Result(const Result&) = default; Result Generate(const Program* program, const Options& options) { Result result; - // Run the MSL sanitizer. - transform::Msl sanitizer; - transform::DataMap transform_input; - transform_input.Add( - 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(); 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(&output.program); + auto impl = std::make_unique(&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 3d014342ae..87d7b50e56 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 @@ class ScopedBitCast { }; } // 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(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(); + } + manager.Add(); + manager.Add(); + manager.Add(); + manager.Add(); + manager.Add(); + manager.Add(); + manager.Add(); + manager.Add(); + // ArrayLengthFromUniform must come after InlinePointerLets and Simplify, as + // it assumes that the form of the array length argument is &var.array. + manager.Add(); + internal_inputs.Add( + std::move(array_length_from_uniform_cfg)); + internal_inputs.Add( + 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() + ->needs_buffer_sizes}; +} + GeneratorImpl::GeneratorImpl(const Program* program) : TextGenerator(program) {} GeneratorImpl::~GeneratorImpl() = default; bool GeneratorImpl::Generate() { - if (!program_->HasTransformApplied()) { - 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 "; line(); line() << "using namespace metal;"; diff --git a/src/writer/msl/generator_impl.h b/src/writer/msl/generator_impl.h index 9c65c158e3..b2be21d02f 100644 --- a/src/writer/msl/generator_impl.h +++ b/src/writer/msl/generator_impl.h @@ -50,6 +50,26 @@ class Intrinsic; 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 5ac5b733d9..f5abc64f7b 100644 --- a/src/writer/msl/generator_impl_test.cc +++ b/src/writer/msl/generator_impl_test.cc @@ -22,16 +22,6 @@ namespace { using MslGeneratorImplTest = TestHelper; -TEST_F(MslGeneratorImplTest, ErrorIfSanitizerNotRun) { - auto program = std::make_unique(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 7400467347..762b2b6528 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 @@ class TestHelperBase : public BASE, public ProgramBuilder { if (gen_) { return *gen_; } - // Fake that the MSL sanitizer has been applied, so that we can unit test - // the writer without it erroring. - SetTransformApplied(); [&]() { ASSERT_TRUE(IsValid()) << "Builder program is not valid\n" << diag::Formatter().format(Diagnostics()); @@ -78,7 +74,7 @@ class TestHelperBase : public BASE, public ProgramBuilder { << 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 ad410e75d6..8d3ee1f078 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -548,7 +548,6 @@ tint_unittests_source_set("tint_unittests_wgsl_writer_src") { 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",