From d59cedb5a5762a92c49aaa7f29a7afa1ad9ed931 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 25 Jan 2021 18:14:08 +0000 Subject: [PATCH] Add tint::Program as a wrapper of tint::ast::Module. `tint::Program` will become the new public API object for a parsed shader program. For now, have Program be a simple wrapper around ast::Module so we can migrate Dawn's use of the public tint API. Add new Program variants of public APIs for places that returned or took a Module. Remove Reset() methods from Generators, they aren't used, and make the migration harder. Change-Id: Ic5bee46ceb109ea591ba7fec33685220b244a1ae Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38540 Reviewed-by: dan sinclair Commit-Queue: Ben Clayton --- BUILD.gn | 1 + src/CMakeLists.txt | 1 + src/inspector/inspector.cc | 2 ++ src/inspector/inspector.h | 7 ++++++ src/program.h | 41 ++++++++++++++++++++++++++++++++ src/reader/reader.h | 4 ++++ src/transform/manager.h | 8 +++++++ src/transform/transform.cc | 20 ++++++++++++++++ src/transform/transform.h | 39 ++++++++++++++++++++++++++++-- src/type_determiner.cc | 3 +++ src/type_determiner.h | 7 ++++++ src/validator/validator.h | 6 +++++ src/writer/hlsl/generator.cc | 11 ++++----- src/writer/hlsl/generator.h | 11 ++++++--- src/writer/hlsl/generator_impl.h | 3 --- src/writer/msl/generator.cc | 10 ++++---- src/writer/msl/generator.h | 11 ++++++--- src/writer/spirv/generator.cc | 11 ++++----- src/writer/spirv/generator.h | 11 ++++++--- src/writer/text.cc | 2 -- src/writer/text.h | 3 --- src/writer/wgsl/generator.cc | 10 ++++---- src/writer/wgsl/generator.h | 11 ++++++--- src/writer/writer.cc | 2 -- src/writer/writer.h | 11 --------- 25 files changed, 186 insertions(+), 60 deletions(-) create mode 100644 src/program.h diff --git a/BUILD.gn b/BUILD.gn index a5b3291db0..156cb69ae5 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -373,6 +373,7 @@ source_set("libtint_core_src") { "src/inspector/scalar.h", "src/namer.cc", "src/namer.h", + "src/program.h", "src/reader/reader.cc", "src/reader/reader.h", "src/scope_stack.h", diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c3aa242d96..cb0edaf6d4 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -187,6 +187,7 @@ set(TINT_LIB_SRCS inspector/scalar.h namer.cc namer.h + program.h reader/reader.cc reader/reader.h scope_stack.h diff --git a/src/inspector/inspector.cc b/src/inspector/inspector.cc index dbbaa21eff..5a3c9e8f95 100644 --- a/src/inspector/inspector.cc +++ b/src/inspector/inspector.cc @@ -45,6 +45,8 @@ namespace inspector { Inspector::Inspector(const ast::Module& module) : module_(module) {} +Inspector::Inspector(const Program* program) : Inspector(program->module) {} + Inspector::~Inspector() = default; std::vector Inspector::GetEntryPoints() { diff --git a/src/inspector/inspector.h b/src/inspector/inspector.h index 552dab67bb..662726cf54 100644 --- a/src/inspector/inspector.h +++ b/src/inspector/inspector.h @@ -25,6 +25,7 @@ #include "src/ast/pipeline_stage.h" #include "src/inspector/entry_point.h" #include "src/inspector/scalar.h" +#include "src/program.h" namespace tint { namespace inspector { @@ -73,6 +74,12 @@ class Inspector { /// Constructor /// @param module Shader module to extract information from. explicit Inspector(const ast::Module& module); + + /// Constructor + /// @param program Shader program to extract information from. + explicit Inspector(const Program* program); + + /// Destructor ~Inspector(); /// @returns error messages from the Inspector diff --git a/src/program.h b/src/program.h new file mode 100644 index 0000000000..31c7f0328b --- /dev/null +++ b/src/program.h @@ -0,0 +1,41 @@ +// 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_PROGRAM_H_ +#define SRC_PROGRAM_H_ + +#include + +#include "src/ast/module.h" + +namespace tint { + +/// Program is (currently) a simple wrapper of to ast::Module. +/// This wrapper is used as a stepping stone to having Dawn use tint::Program +/// instead of tint::ast::Module. +class Program { + public: + /// The wrapped module + ast::Module module; + + /// @returns true if all required fields in the module are present. + bool IsValid() const { return module.IsValid(); } + + /// @return a deep copy of this program + Program Clone() const { return Program{module.Clone()}; } +}; + +} // namespace tint + +#endif // SRC_PROGRAM_H_ diff --git a/src/reader/reader.h b/src/reader/reader.h index 9c71184f6f..a852dd047c 100644 --- a/src/reader/reader.h +++ b/src/reader/reader.h @@ -21,6 +21,7 @@ #include "src/ast/module.h" #include "src/diagnostic/diagnostic.h" #include "src/diagnostic/formatter.h" +#include "src/program.h" namespace tint { namespace reader { @@ -49,6 +50,9 @@ class Reader { /// @returns the module. The module in the parser will be reset after this. virtual ast::Module module() = 0; + /// @returns the program. The module in the parser will be reset after this. + Program program() { return Program{module()}; } + protected: /// Constructor Reader(); diff --git a/src/transform/manager.h b/src/transform/manager.h index b31a81cc2e..8f838e4956 100644 --- a/src/transform/manager.h +++ b/src/transform/manager.h @@ -46,6 +46,14 @@ class Manager : public Transform { /// @returns the transformed module and diagnostics Output Run(ast::Module* module) override; + /// Runs the transform on `program`, returning the transformation result. + /// @note Users of Tint should register the transform with transform manager + /// and invoke its Run(), instead of directly calling the transform's Run(). + /// Calling Run() directly does not perform program state cleanup operations. + /// @param program the source program to transform + /// @returns the transformation result + Output Run(Program* program) { return Run(&program->module); } + private: std::vector> transforms_; }; diff --git a/src/transform/transform.cc b/src/transform/transform.cc index 29f346bc5a..9daa76304c 100644 --- a/src/transform/transform.cc +++ b/src/transform/transform.cc @@ -21,7 +21,27 @@ namespace tint { namespace transform { +Transform::Output::Output() = default; + +Transform::Output::Output(ast::Module&& mod) : program{std::move(mod)} {} + +Transform::Output::Output(ast::Module&& mod, diag::List&& d) + : program{std::move(mod)}, diagnostics(std::move(d)) {} + +Transform::Output::~Output() = default; + +Transform::Output::Output(Output&& rhs) + : program(std::move(rhs.program)), + diagnostics(std::move(rhs.diagnostics)) {} + +Transform::Output& Transform::Output::operator=(Output&& rhs) { + program = std::move(rhs.program); + diagnostics = std::move(rhs.diagnostics); + return *this; +} + Transform::Transform() = default; + Transform::~Transform() = default; ast::Function* Transform::CloneWithStatementsAtStart( diff --git a/src/transform/transform.h b/src/transform/transform.h index 4684a21f61..9c359f2dcf 100644 --- a/src/transform/transform.h +++ b/src/transform/transform.h @@ -21,6 +21,7 @@ #include "src/ast/module.h" #include "src/diagnostic/diagnostic.h" +#include "src/program.h" namespace tint { namespace transform { @@ -35,10 +36,36 @@ class Transform { /// The return type of Run() struct Output { - /// The transformed module. May be empty on error. - ast::Module module; + /// Constructor + Output(); + + /// Constructor + /// @param module the module to move into this Output + explicit Output(ast::Module&& module); + + /// Constructor + /// @param module the module to move into this Output + /// @param diags the list of diagnostics to move into this Output + Output(ast::Module&& module, diag::List&& diags); + + /// Move constructor + /// @param output the output to move into this Output + Output(Output&& output); + + /// Destructor + ~Output(); + + /// Move assignment operator + /// @param rhs the Output to move into this Output + /// @returns this Output + Output& operator=(Output&& rhs); + + /// The transformed program. May be empty on error. + Program program; /// Diagnostics raised while running the Transform. diag::List diagnostics; + /// The transformed module. May be empty on error. + ast::Module& module{program.module}; }; /// Runs the transform on `module`, returning the transformation result. @@ -49,6 +76,14 @@ class Transform { /// @returns the transformation result virtual Output Run(ast::Module* module) = 0; + /// Runs the transform on `program`, returning the transformation result. + /// @note Users of Tint should register the transform with transform manager + /// and invoke its Run(), instead of directly calling the transform's Run(). + /// Calling Run() directly does not perform program state cleanup operations. + /// @param program the source program to transform + /// @returns the transformation result + Output Run(Program* program) { return Run(&program->module); } + protected: /// Clones the function `in` adding `statements` to the beginning of the /// cloned function body. diff --git a/src/type_determiner.cc b/src/type_determiner.cc index 67875b5982..6752c31f98 100644 --- a/src/type_determiner.cc +++ b/src/type_determiner.cc @@ -61,6 +61,9 @@ namespace tint { TypeDeterminer::TypeDeterminer(ast::Module* mod) : mod_(mod) {} +TypeDeterminer::TypeDeterminer(Program* program) + : TypeDeterminer(&program->module) {} + TypeDeterminer::~TypeDeterminer() = default; void TypeDeterminer::set_error(const Source& src, const std::string& msg) { diff --git a/src/type_determiner.h b/src/type_determiner.h index f2aa2ba58d..3c38d6c0b6 100644 --- a/src/type_determiner.h +++ b/src/type_determiner.h @@ -20,6 +20,7 @@ #include #include "src/ast/module.h" +#include "src/program.h" #include "src/scope_stack.h" #include "src/type/storage_texture_type.h" @@ -45,6 +46,12 @@ class TypeDeterminer { /// Constructor /// @param mod the module to update with typing information explicit TypeDeterminer(ast::Module* mod); + + /// Constructor + /// @param program the module to update with typing information + explicit TypeDeterminer(Program* program); + + /// Destructor ~TypeDeterminer(); /// @returns error messages from the type determiner diff --git a/src/validator/validator.h b/src/validator/validator.h index 8b8f9a9b5e..9d3fbc7d01 100644 --- a/src/validator/validator.h +++ b/src/validator/validator.h @@ -24,6 +24,7 @@ #include "src/ast/statement.h" #include "src/diagnostic/diagnostic.h" #include "src/diagnostic/formatter.h" +#include "src/program.h" namespace tint { @@ -39,6 +40,11 @@ class Validator { /// @returns true if the validation was successful bool Validate(const ast::Module* module); + /// Runs the validator + /// @param program the program to validate + /// @returns true if the validation was successful + bool Validate(const Program* program) { return Validate(&program->module); } + /// @returns error messages from the validator std::string error() { diag::Formatter formatter{{false, false, false, false}}; diff --git a/src/writer/hlsl/generator.cc b/src/writer/hlsl/generator.cc index d521533a29..90d78c6b14 100644 --- a/src/writer/hlsl/generator.cc +++ b/src/writer/hlsl/generator.cc @@ -21,16 +21,13 @@ namespace writer { namespace hlsl { Generator::Generator(ast::Module module) - : Text(std::move(module)), + : module_(std::move(module)), impl_(std::make_unique(&module_)) {} -Generator::~Generator() = default; +Generator::Generator(Program* program) + : impl_(std::make_unique(&program->module)) {} -void Generator::Reset() { - set_error(""); - out_ = std::ostringstream(); - impl_ = std::make_unique(&module_); -} +Generator::~Generator() = default; bool Generator::Generate() { auto ret = impl_->Generate(out_); diff --git a/src/writer/hlsl/generator.h b/src/writer/hlsl/generator.h index 83f6d27d92..1c3b58cb00 100644 --- a/src/writer/hlsl/generator.h +++ b/src/writer/hlsl/generator.h @@ -19,6 +19,7 @@ #include #include +#include "src/program.h" #include "src/writer/hlsl/generator_impl.h" #include "src/writer/text.h" @@ -32,10 +33,13 @@ class Generator : public Text { /// Constructor /// @param module the module to convert explicit Generator(ast::Module module); - ~Generator() override; - /// Resets the generator - void Reset() override; + /// Constructor + /// @param program the program to convert + explicit Generator(Program* program); + + /// Destructor + ~Generator() override; /// Generates the result data /// @returns true on successful generation; false otherwise @@ -55,6 +59,7 @@ class Generator : public Text { std::string error() const; private: + ast::Module module_; std::ostringstream out_; std::unique_ptr impl_; }; diff --git a/src/writer/hlsl/generator_impl.h b/src/writer/hlsl/generator_impl.h index 433c3a1e0c..834fd7cce6 100644 --- a/src/writer/hlsl/generator_impl.h +++ b/src/writer/hlsl/generator_impl.h @@ -74,9 +74,6 @@ class GeneratorImpl { /// @returns the error std::string error() const { return error_; } - /// Resets the generator - void Reset(); - /// @param out the output stream /// @returns true on successful generation; false otherwise bool Generate(std::ostream& out); diff --git a/src/writer/msl/generator.cc b/src/writer/msl/generator.cc index 46fa0733f0..5c6428c8e6 100644 --- a/src/writer/msl/generator.cc +++ b/src/writer/msl/generator.cc @@ -21,15 +21,13 @@ namespace writer { namespace msl { Generator::Generator(ast::Module module) - : Text(std::move(module)), + : module_(std::move(module)), impl_(std::make_unique(&module_)) {} -Generator::~Generator() = default; +Generator::Generator(Program* program) + : impl_(std::make_unique(&program->module)) {} -void Generator::Reset() { - set_error(""); - impl_ = std::make_unique(&module_); -} +Generator::~Generator() = default; bool Generator::Generate() { auto ret = impl_->Generate(); diff --git a/src/writer/msl/generator.h b/src/writer/msl/generator.h index d0a9de2aee..7586a5a706 100644 --- a/src/writer/msl/generator.h +++ b/src/writer/msl/generator.h @@ -18,6 +18,7 @@ #include #include +#include "src/program.h" #include "src/writer/msl/generator_impl.h" #include "src/writer/text.h" @@ -31,10 +32,13 @@ class Generator : public Text { /// Constructor /// @param module the module to convert explicit Generator(ast::Module module); - ~Generator() override; - /// Resets the generator - void Reset() override; + /// Constructor + /// @param program the program to convert + explicit Generator(Program* program); + + /// Destructor + ~Generator() override; /// Generates the result data /// @returns true on successful generation; false otherwise @@ -54,6 +58,7 @@ class Generator : public Text { std::string error() const; private: + ast::Module module_; std::unique_ptr impl_; }; diff --git a/src/writer/spirv/generator.cc b/src/writer/spirv/generator.cc index c28cb130e5..ba367c51e9 100644 --- a/src/writer/spirv/generator.cc +++ b/src/writer/spirv/generator.cc @@ -21,16 +21,15 @@ namespace writer { namespace spirv { Generator::Generator(ast::Module module) - : writer::Writer(std::move(module)), + : module_(std::move(module)), builder_(std::make_unique(&module_)), writer_(std::make_unique()) {} -Generator::~Generator() = default; +Generator::Generator(Program* program) + : builder_(std::make_unique(&program->module)), + writer_(std::make_unique()) {} -void Generator::Reset() { - builder_ = std::make_unique(&module_); - writer_ = std::make_unique(); -} +Generator::~Generator() = default; bool Generator::Generate() { if (!builder_->Build()) { diff --git a/src/writer/spirv/generator.h b/src/writer/spirv/generator.h index f5968b2d2d..82bff48645 100644 --- a/src/writer/spirv/generator.h +++ b/src/writer/spirv/generator.h @@ -20,6 +20,7 @@ #include #include "src/ast/module.h" +#include "src/program.h" #include "src/writer/spirv/binary_writer.h" #include "src/writer/spirv/builder.h" #include "src/writer/writer.h" @@ -34,10 +35,13 @@ class Generator : public writer::Writer { /// Constructor /// @param module the module to convert explicit Generator(ast::Module module); - ~Generator() override; - /// Resets the generator - void Reset() override; + /// Constructor + /// @param program the program to convert + explicit Generator(Program* program); + + /// Destructor + ~Generator() override; /// Generates the result data /// @returns true on successful generation; false otherwise @@ -54,6 +58,7 @@ class Generator : public writer::Writer { const std::vector& result() const { return writer_->result(); } private: + ast::Module module_; std::unique_ptr builder_; std::unique_ptr writer_; }; diff --git a/src/writer/text.cc b/src/writer/text.cc index 319f32eb1e..28fcd9d2e2 100644 --- a/src/writer/text.cc +++ b/src/writer/text.cc @@ -19,8 +19,6 @@ namespace tint { namespace writer { -Text::Text(ast::Module module) : Writer(std::move(module)) {} - Text::~Text() = default; } // namespace writer diff --git a/src/writer/text.h b/src/writer/text.h index 7e4fffc05b..625e8e0ef6 100644 --- a/src/writer/text.h +++ b/src/writer/text.h @@ -25,9 +25,6 @@ namespace writer { /// Class to generate text source class Text : public Writer { public: - /// Constructor - /// @param module the module to convert - explicit Text(ast::Module module); ~Text() override; /// @returns the result data diff --git a/src/writer/wgsl/generator.cc b/src/writer/wgsl/generator.cc index e9d0a9b052..368b5eff98 100644 --- a/src/writer/wgsl/generator.cc +++ b/src/writer/wgsl/generator.cc @@ -21,15 +21,13 @@ namespace writer { namespace wgsl { Generator::Generator(ast::Module module) - : Text(std::move(module)), + : module_(std::move(module)), impl_(std::make_unique(&module_)) {} -Generator::~Generator() = default; +Generator::Generator(Program* program) + : impl_(std::make_unique(&program->module)) {} -void Generator::Reset() { - set_error(""); - impl_ = std::make_unique(&module_); -} +Generator::~Generator() = default; bool Generator::Generate() { auto ret = impl_->Generate(); diff --git a/src/writer/wgsl/generator.h b/src/writer/wgsl/generator.h index 2d3a234f8a..3ed31ecbd3 100644 --- a/src/writer/wgsl/generator.h +++ b/src/writer/wgsl/generator.h @@ -18,6 +18,7 @@ #include #include +#include "src/program.h" #include "src/writer/text.h" #include "src/writer/wgsl/generator_impl.h" @@ -31,10 +32,13 @@ class Generator : public Text { /// Constructor /// @param module the module to convert explicit Generator(ast::Module module); - ~Generator() override; - /// Resets the generator - void Reset() override; + /// Constructor + /// @param program the program to convert + explicit Generator(Program* program); + + /// Destructor + ~Generator() override; /// Generates the result data /// @returns true on successful generation; false otherwise @@ -54,6 +58,7 @@ class Generator : public Text { std::string error() const; private: + ast::Module module_; std::unique_ptr impl_; }; diff --git a/src/writer/writer.cc b/src/writer/writer.cc index 545ed3b372..7499c3c569 100644 --- a/src/writer/writer.cc +++ b/src/writer/writer.cc @@ -19,8 +19,6 @@ namespace tint { namespace writer { -Writer::Writer(ast::Module module) : module_(std::move(module)) {} - Writer::~Writer() = default; } // namespace writer diff --git a/src/writer/writer.h b/src/writer/writer.h index b38657be62..7aa7a97630 100644 --- a/src/writer/writer.h +++ b/src/writer/writer.h @@ -31,11 +31,6 @@ class Writer { /// @returns the writer error string const std::string& error() const { return error_; } - /// Resets the generator - //! @cond Doxygen_Suppress - virtual void Reset() = 0; - //! @endcond - /// Converts the module into the desired format /// @returns true on success; false on failure virtual bool Generate() = 0; @@ -48,18 +43,12 @@ class Writer { const std::string& name) = 0; protected: - /// Constructor - /// @param module the tint module to convert - explicit Writer(ast::Module module); - /// Sets the error string /// @param msg the error message void set_error(const std::string& msg) { error_ = msg; } /// An error message, if an error was encountered std::string error_; - /// The module being converted - ast::Module module_; }; } // namespace writer