From b5c46c30ec440899910fdabcafa3b01027bb5e9e Mon Sep 17 00:00:00 2001 From: Antonio Maiorano Date: Mon, 11 Apr 2022 21:10:20 +0000 Subject: [PATCH] tint: spirv writer: add a GeneralImpl to be consistent with HLSL and MSL backends Also opportunistically optimize out the copying of the spir-v result vector by moving it instead. Bug: tint:1495 Change-Id: Ia2c3b3c09e7a23822eb8a782ce57b1fa11a0b54d Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/86204 Reviewed-by: David Neto Kokoro-Run: Antonio Maiorano Kokoro: Kokoro Commit-Queue: Antonio Maiorano --- src/tint/BUILD.gn | 2 + src/tint/CMakeLists.txt | 2 + src/tint/writer/spirv/binary_writer.h | 3 + src/tint/writer/spirv/builder.cc | 71 -------------- src/tint/writer/spirv/builder.h | 12 --- src/tint/writer/spirv/generator.cc | 22 ++--- src/tint/writer/spirv/generator_impl.cc | 120 ++++++++++++++++++++++++ src/tint/writer/spirv/generator_impl.h | 67 +++++++++++++ src/tint/writer/spirv/test_helper.h | 1 + 9 files changed, 203 insertions(+), 97 deletions(-) create mode 100644 src/tint/writer/spirv/generator_impl.cc create mode 100644 src/tint/writer/spirv/generator_impl.h diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn index 79beb3d9f7..956705503e 100644 --- a/src/tint/BUILD.gn +++ b/src/tint/BUILD.gn @@ -688,6 +688,8 @@ libtint_source_set("libtint_spv_writer_src") { "writer/spirv/function.h", "writer/spirv/generator.cc", "writer/spirv/generator.h", + "writer/spirv/generator_impl.cc", + "writer/spirv/generator_impl.h", "writer/spirv/instruction.cc", "writer/spirv/instruction.h", "writer/spirv/operand.cc", diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt index 046b814495..a6ad6f6224 100644 --- a/src/tint/CMakeLists.txt +++ b/src/tint/CMakeLists.txt @@ -519,6 +519,8 @@ if(${TINT_BUILD_SPV_WRITER}) writer/spirv/function.h writer/spirv/generator.cc writer/spirv/generator.h + writer/spirv/generator_impl.cc + writer/spirv/generator_impl.h writer/spirv/instruction.cc writer/spirv/instruction.h writer/spirv/operand.cc diff --git a/src/tint/writer/spirv/binary_writer.h b/src/tint/writer/spirv/binary_writer.h index 89789fd955..a071d7cabd 100644 --- a/src/tint/writer/spirv/binary_writer.h +++ b/src/tint/writer/spirv/binary_writer.h @@ -45,6 +45,9 @@ class BinaryWriter { /// @returns the assembled SPIR-V const std::vector& result() const { return out_; } + /// @returns the assembled SPIR-V + std::vector& result() { return out_; } + private: void process_instruction(const Instruction& inst); void process_op(const Operand& op); diff --git a/src/tint/writer/spirv/builder.cc b/src/tint/writer/spirv/builder.cc index 436f977ad1..c03334489d 100644 --- a/src/tint/writer/spirv/builder.cc +++ b/src/tint/writer/spirv/builder.cc @@ -41,26 +41,10 @@ #include "src/tint/sem/type_conversion.h" #include "src/tint/sem/variable.h" #include "src/tint/sem/vector_type.h" -#include "src/tint/transform/add_empty_entry_point.h" #include "src/tint/transform/add_spirv_block_attribute.h" -#include "src/tint/transform/builtin_polyfill.h" -#include "src/tint/transform/canonicalize_entry_point_io.h" -#include "src/tint/transform/expand_compound_assignment.h" -#include "src/tint/transform/fold_constants.h" -#include "src/tint/transform/for_loop_to_loop.h" -#include "src/tint/transform/manager.h" -#include "src/tint/transform/promote_side_effects_to_decl.h" -#include "src/tint/transform/remove_unreachable_statements.h" -#include "src/tint/transform/simplify_pointers.h" -#include "src/tint/transform/unshadow.h" -#include "src/tint/transform/unwind_discard_functions.h" -#include "src/tint/transform/var_for_dynamic_index.h" -#include "src/tint/transform/vectorize_scalar_matrix_constructors.h" -#include "src/tint/transform/zero_init_workgroup_memory.h" #include "src/tint/utils/defer.h" #include "src/tint/utils/map.h" #include "src/tint/writer/append_vector.h" -#include "src/tint/writer/generate_external_texture_bindings.h" namespace tint::writer::spirv { namespace { @@ -255,61 +239,6 @@ const sem::Type* ElementTypeOf(const sem::Type* ty) { } // namespace -SanitizedResult Sanitize(const Program* in, const Options& options) { - transform::Manager manager; - transform::DataMap data; - - { // Builtin polyfills - transform::BuiltinPolyfill::Builtins polyfills; - polyfills.count_leading_zeros = true; - polyfills.count_trailing_zeros = true; - polyfills.extract_bits = - transform::BuiltinPolyfill::Level::kClampParameters; - polyfills.first_leading_bit = true; - polyfills.first_trailing_bit = true; - polyfills.insert_bits = transform::BuiltinPolyfill::Level::kClampParameters; - data.Add(polyfills); - manager.Add(); - } - - if (options.generate_external_texture_bindings) { - auto new_bindings_map = GenerateExternalTextureBindings(in); - data.Add( - new_bindings_map); - } - manager.Add(); - - manager.Add(); - 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(); - } - manager.Add(); - manager.Add(); - manager.Add(); - manager.Add(); - manager.Add(); // Required for arrayLength() - manager.Add(); - manager.Add(); - manager.Add(); // Must come after - // ZeroInitWorkgroupMemory - manager.Add(); - manager.Add(); - manager.Add(); - manager.Add(); - - data.Add( - transform::CanonicalizeEntryPointIO::Config( - transform::CanonicalizeEntryPointIO::ShaderStyle::kSpirv, 0xFFFFFFFF, - options.emit_vertex_point_size)); - - SanitizedResult result; - result.program = std::move(manager.Run(in, data).program); - return result; -} - Builder::AccessorInfo::AccessorInfo() : source_id(0), source_type(nullptr) {} Builder::AccessorInfo::~AccessorInfo() {} diff --git a/src/tint/writer/spirv/builder.h b/src/tint/writer/spirv/builder.h index c36efcce36..7ad9cf242d 100644 --- a/src/tint/writer/spirv/builder.h +++ b/src/tint/writer/spirv/builder.h @@ -38,7 +38,6 @@ #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 @@ -51,17 +50,6 @@ class TypeConversion; namespace tint::writer::spirv { -/// The result of sanitizing a program for generation. -struct SanitizedResult { - /// The sanitized program. - Program program; -}; - -/// Sanitize a program in preparation for generating SPIR-V. -/// @program The program to sanitize -/// @param options The SPIR-V generator options. -SanitizedResult Sanitize(const Program* program, const Options& options); - /// Builder class to create SPIR-V instructions from a module. class Builder { public: diff --git a/src/tint/writer/spirv/generator.cc b/src/tint/writer/spirv/generator.cc index 938f8f7035..04f6e15129 100644 --- a/src/tint/writer/spirv/generator.cc +++ b/src/tint/writer/spirv/generator.cc @@ -14,7 +14,9 @@ #include "src/tint/writer/spirv/generator.h" -#include "src/tint/writer/spirv/binary_writer.h" +#include + +#include "src/tint/writer/spirv/generator_impl.h" namespace tint::writer::spirv { @@ -37,20 +39,12 @@ Result Generate(const Program* program, const Options& options) { bool zero_initialize_workgroup_memory = !options.disable_workgroup_init && options.use_zero_initialize_workgroup_memory_extension; - auto builder = std::make_unique(&sanitized_result.program, - zero_initialize_workgroup_memory); - auto writer = std::make_unique(); - if (!builder->Build()) { - result.success = false; - result.error = builder->error(); - return result; - } - writer->WriteHeader(builder->id_bound()); - writer->WriteBuilder(builder.get()); - - result.success = true; - result.spirv = writer->result(); + auto impl = std::make_unique(&sanitized_result.program, + zero_initialize_workgroup_memory); + result.success = impl->Generate(); + result.error = impl->error(); + result.spirv = std::move(impl->result()); return result; } diff --git a/src/tint/writer/spirv/generator_impl.cc b/src/tint/writer/spirv/generator_impl.cc new file mode 100644 index 0000000000..33188085b2 --- /dev/null +++ b/src/tint/writer/spirv/generator_impl.cc @@ -0,0 +1,120 @@ +// Copyright 2022 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/tint/writer/spirv/generator_impl.h" + +#include +#include + +#include "src/tint/transform/add_empty_entry_point.h" +#include "src/tint/transform/add_spirv_block_attribute.h" +#include "src/tint/transform/builtin_polyfill.h" +#include "src/tint/transform/canonicalize_entry_point_io.h" +#include "src/tint/transform/expand_compound_assignment.h" +#include "src/tint/transform/fold_constants.h" +#include "src/tint/transform/for_loop_to_loop.h" +#include "src/tint/transform/manager.h" +#include "src/tint/transform/promote_side_effects_to_decl.h" +#include "src/tint/transform/remove_unreachable_statements.h" +#include "src/tint/transform/simplify_pointers.h" +#include "src/tint/transform/unshadow.h" +#include "src/tint/transform/unwind_discard_functions.h" +#include "src/tint/transform/var_for_dynamic_index.h" +#include "src/tint/transform/vectorize_scalar_matrix_constructors.h" +#include "src/tint/transform/zero_init_workgroup_memory.h" +#include "src/tint/writer/generate_external_texture_bindings.h" + +namespace tint::writer::spirv { + +SanitizedResult Sanitize(const Program* in, const Options& options) { + transform::Manager manager; + transform::DataMap data; + + { // Builtin polyfills + transform::BuiltinPolyfill::Builtins polyfills; + polyfills.count_leading_zeros = true; + polyfills.count_trailing_zeros = true; + polyfills.extract_bits = + transform::BuiltinPolyfill::Level::kClampParameters; + polyfills.first_leading_bit = true; + polyfills.first_trailing_bit = true; + polyfills.insert_bits = transform::BuiltinPolyfill::Level::kClampParameters; + data.Add(polyfills); + manager.Add(); + } + + if (options.generate_external_texture_bindings) { + auto new_bindings_map = GenerateExternalTextureBindings(in); + data.Add( + new_bindings_map); + } + manager.Add(); + + manager.Add(); + 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(); + } + manager.Add(); + manager.Add(); + manager.Add(); + manager.Add(); + manager.Add(); // Required for arrayLength() + manager.Add(); + manager.Add(); + manager.Add(); // Must come after + // ZeroInitWorkgroupMemory + manager.Add(); + manager.Add(); + manager.Add(); + manager.Add(); + + data.Add( + transform::CanonicalizeEntryPointIO::Config( + transform::CanonicalizeEntryPointIO::ShaderStyle::kSpirv, 0xFFFFFFFF, + options.emit_vertex_point_size)); + + SanitizedResult result; + result.program = std::move(manager.Run(in, data).program); + return result; +} + +GeneratorImpl::GeneratorImpl(const Program* program, + bool zero_initialize_workgroup_memory) + : builder_(program, zero_initialize_workgroup_memory) {} + +bool GeneratorImpl::Generate() { + if (builder_.Build()) { + writer_.WriteHeader(builder_.id_bound()); + writer_.WriteBuilder(&builder_); + return true; + } + return false; +} + +const std::vector& GeneratorImpl::result() const { + return writer_.result(); +} + +std::vector& GeneratorImpl::result() { + return writer_.result(); +} + +std::string GeneratorImpl::error() const { + return builder_.error(); +} + +} // namespace tint::writer::spirv diff --git a/src/tint/writer/spirv/generator_impl.h b/src/tint/writer/spirv/generator_impl.h new file mode 100644 index 0000000000..2e02af1eb1 --- /dev/null +++ b/src/tint/writer/spirv/generator_impl.h @@ -0,0 +1,67 @@ +// Copyright 2022 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_TINT_WRITER_SPIRV_GENERATOR_IMPL_H_ +#define SRC_TINT_WRITER_SPIRV_GENERATOR_IMPL_H_ + +#include +#include + +#include "src/tint/program.h" +#include "src/tint/writer/spirv/binary_writer.h" +#include "src/tint/writer/spirv/builder.h" +#include "src/tint/writer/spirv/generator.h" + +namespace tint::writer::spirv { + +/// The result of sanitizing a program for generation. +struct SanitizedResult { + /// The sanitized program. + Program program; +}; + +/// Sanitize a program in preparation for generating SPIR-V. +/// @program The program to sanitize +/// @param options The SPIR-V generator options. +SanitizedResult Sanitize(const Program* program, const Options& options); + +/// Implementation class for SPIR-V generator +class GeneratorImpl { + public: + /// Constructor + /// @param program the program to generate + /// @param zero_initialize_workgroup_memory `true` to initialize all the + /// variables in the Workgroup storage class with OpConstantNull + GeneratorImpl(const Program* program, bool zero_initialize_workgroup_memory); + + /// @returns true on successful generation; false otherwise + bool Generate(); + + /// @returns the result data + const std::vector& result() const; + + /// @returns the result data + std::vector& result(); + + /// @returns the error + std::string error() const; + + private: + Builder builder_; + BinaryWriter writer_; +}; + +} // namespace tint::writer::spirv + +#endif // SRC_TINT_WRITER_SPIRV_GENERATOR_IMPL_H_ diff --git a/src/tint/writer/spirv/test_helper.h b/src/tint/writer/spirv/test_helper.h index ecf2a21f96..c8052b22b3 100644 --- a/src/tint/writer/spirv/test_helper.h +++ b/src/tint/writer/spirv/test_helper.h @@ -22,6 +22,7 @@ #include "gtest/gtest.h" #include "spirv-tools/libspirv.hpp" #include "src/tint/writer/spirv/binary_writer.h" +#include "src/tint/writer/spirv/generator_impl.h" namespace tint::writer::spirv {