From 856a3688f731bc7cd804fc92fd76118ee869595c Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Fri, 16 Apr 2021 08:35:24 +0000 Subject: [PATCH] transform: Minor changes to cleanup Dawn usage. Move transform::Transform::Output to transform::Output. There's no need for this to be an nested class, it stutters, and it also prevents Dawn from forward declaring it. Add move assignment operator to DataMap. Change-Id: Ibe1af03abc1a872790d20ee6ec8cf18a511ea0b4 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47772 Kokoro: Kokoro Reviewed-by: Corentin Wallez Commit-Queue: Ben Clayton --- src/transform/binding_remapper.cc | 3 +- src/transform/bound_array_accessors.cc | 2 +- src/transform/calculate_array_length.cc | 2 +- src/transform/canonicalize_entry_point_io.cc | 3 +- src/transform/decompose_storage_access.cc | 3 +- src/transform/emit_vertex_point_size.cc | 2 +- src/transform/first_index_offset.cc | 3 +- src/transform/hlsl.cc | 2 +- src/transform/manager.cc | 2 +- src/transform/msl.cc | 2 +- src/transform/renamer.cc | 2 +- src/transform/spirv.cc | 2 +- src/transform/test_helper.h | 19 +++---- src/transform/transform.cc | 5 +- src/transform/transform.h | 58 +++++++++++--------- src/transform/vertex_pulling.cc | 2 +- 16 files changed, 58 insertions(+), 54 deletions(-) diff --git a/src/transform/binding_remapper.cc b/src/transform/binding_remapper.cc index aed0b0aee8..8cc1f9c953 100644 --- a/src/transform/binding_remapper.cc +++ b/src/transform/binding_remapper.cc @@ -34,8 +34,7 @@ BindingRemapper::Remappings::~Remappings() = default; BindingRemapper::BindingRemapper() = default; BindingRemapper::~BindingRemapper() = default; -Transform::Output BindingRemapper::Run(const Program* in, - const DataMap& datamap) { +Output BindingRemapper::Run(const Program* in, const DataMap& datamap) { ProgramBuilder out; auto* remappings = datamap.Get(); if (!remappings) { diff --git a/src/transform/bound_array_accessors.cc b/src/transform/bound_array_accessors.cc index 0ba5a49631..3f6416b42c 100644 --- a/src/transform/bound_array_accessors.cc +++ b/src/transform/bound_array_accessors.cc @@ -26,7 +26,7 @@ namespace transform { BoundArrayAccessors::BoundArrayAccessors() = default; BoundArrayAccessors::~BoundArrayAccessors() = default; -Transform::Output BoundArrayAccessors::Run(const Program* in, const DataMap&) { +Output BoundArrayAccessors::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); diff --git a/src/transform/calculate_array_length.cc b/src/transform/calculate_array_length.cc index 7d92631785..3d37e86ffa 100644 --- a/src/transform/calculate_array_length.cc +++ b/src/transform/calculate_array_length.cc @@ -68,7 +68,7 @@ CalculateArrayLength::BufferSizeIntrinsic::Clone(CloneContext* ctx) const { CalculateArrayLength::CalculateArrayLength() = default; CalculateArrayLength::~CalculateArrayLength() = default; -Transform::Output CalculateArrayLength::Run(const Program* in, const DataMap&) { +Output CalculateArrayLength::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); diff --git a/src/transform/canonicalize_entry_point_io.cc b/src/transform/canonicalize_entry_point_io.cc index 22bab4a7eb..622b6b2fa4 100644 --- a/src/transform/canonicalize_entry_point_io.cc +++ b/src/transform/canonicalize_entry_point_io.cc @@ -58,8 +58,7 @@ bool StructMemberComparator(const ast::StructMember* a, } // namespace -Transform::Output CanonicalizeEntryPointIO::Run(const Program* in, - const DataMap&) { +Output CanonicalizeEntryPointIO::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); diff --git a/src/transform/decompose_storage_access.cc b/src/transform/decompose_storage_access.cc index 92ebe5ff26..d154514663 100644 --- a/src/transform/decompose_storage_access.cc +++ b/src/transform/decompose_storage_access.cc @@ -609,8 +609,7 @@ DecomposeStorageAccess::Intrinsic* DecomposeStorageAccess::Intrinsic::Clone( DecomposeStorageAccess::DecomposeStorageAccess() = default; DecomposeStorageAccess::~DecomposeStorageAccess() = default; -Transform::Output DecomposeStorageAccess::Run(const Program* in, - const DataMap&) { +Output DecomposeStorageAccess::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); diff --git a/src/transform/emit_vertex_point_size.cc b/src/transform/emit_vertex_point_size.cc index c4e0464002..676d400a3e 100644 --- a/src/transform/emit_vertex_point_size.cc +++ b/src/transform/emit_vertex_point_size.cc @@ -25,7 +25,7 @@ namespace transform { EmitVertexPointSize::EmitVertexPointSize() = default; EmitVertexPointSize::~EmitVertexPointSize() = default; -Transform::Output EmitVertexPointSize::Run(const Program* in, const DataMap&) { +Output EmitVertexPointSize::Run(const Program* in, const DataMap&) { if (!in->AST().Functions().HasStage(ast::PipelineStage::kVertex)) { // If the module doesn't have any vertex stages, then there's nothing to do. return Output(Program(in->Clone())); diff --git a/src/transform/first_index_offset.cc b/src/transform/first_index_offset.cc index a1adf66d38..7a969951a1 100644 --- a/src/transform/first_index_offset.cc +++ b/src/transform/first_index_offset.cc @@ -60,8 +60,7 @@ FirstIndexOffset::FirstIndexOffset(uint32_t binding, uint32_t group) FirstIndexOffset::~FirstIndexOffset() = default; -Transform::Output FirstIndexOffset::Run(const Program* in, - const DataMap& data) { +Output FirstIndexOffset::Run(const Program* in, const DataMap& data) { // Get the uniform buffer binding point uint32_t ub_binding = binding_; uint32_t ub_group = group_; diff --git a/src/transform/hlsl.cc b/src/transform/hlsl.cc index 3b4bb8008e..5a5b6db1c9 100644 --- a/src/transform/hlsl.cc +++ b/src/transform/hlsl.cc @@ -33,7 +33,7 @@ namespace transform { Hlsl::Hlsl() = default; Hlsl::~Hlsl() = default; -Transform::Output Hlsl::Run(const Program* in, const DataMap& data) { +Output Hlsl::Run(const Program* in, const DataMap& data) { Manager manager; manager.Add(); manager.Add(); diff --git a/src/transform/manager.cc b/src/transform/manager.cc index ef6f79e631..10e3f58420 100644 --- a/src/transform/manager.cc +++ b/src/transform/manager.cc @@ -20,7 +20,7 @@ namespace transform { Manager::Manager() = default; Manager::~Manager() = default; -Transform::Output Manager::Run(const Program* program, const DataMap& data) { +Output Manager::Run(const Program* program, const DataMap& data) { Output out; if (!transforms_.empty()) { for (auto& transform : transforms_) { diff --git a/src/transform/msl.cc b/src/transform/msl.cc index 5f01358b90..c1033fafbf 100644 --- a/src/transform/msl.cc +++ b/src/transform/msl.cc @@ -25,7 +25,7 @@ namespace transform { Msl::Msl() = default; Msl::~Msl() = default; -Transform::Output Msl::Run(const Program* in, const DataMap& data) { +Output Msl::Run(const Program* in, const DataMap& data) { Manager manager; manager.Add(); auto out = manager.Run(in, data); diff --git a/src/transform/renamer.cc b/src/transform/renamer.cc index 07f773d264..9ed474320f 100644 --- a/src/transform/renamer.cc +++ b/src/transform/renamer.cc @@ -846,7 +846,7 @@ Renamer::Renamer(const Config& config) : cfg_(config) {} Renamer::~Renamer() = default; -Transform::Output Renamer::Run(const Program* in, const DataMap&) { +Output Renamer::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); diff --git a/src/transform/spirv.cc b/src/transform/spirv.cc index 2d651e16c9..1faad8f39d 100644 --- a/src/transform/spirv.cc +++ b/src/transform/spirv.cc @@ -31,7 +31,7 @@ namespace transform { Spirv::Spirv() = default; Spirv::~Spirv() = default; -Transform::Output Spirv::Run(const Program* in, const DataMap&) { +Output Spirv::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); HandleEntryPointIOTypes(ctx); diff --git a/src/transform/test_helper.h b/src/transform/test_helper.h index 52f3a77aeb..dcd11b0517 100644 --- a/src/transform/test_helper.h +++ b/src/transform/test_helper.h @@ -38,10 +38,9 @@ class TransformTestBase : public BASE { /// @param transforms the list of transforms to apply /// @param data the optional DataMap to pass to Transform::Run() /// @return the transformed output - Transform::Output Run( - std::string in, - std::vector> transforms, - const DataMap& data = {}) { + Output Run(std::string in, + std::vector> transforms, + const DataMap& data = {}) { auto file = std::make_unique("test", in); auto program = reader::wgsl::Parse(file.get()); @@ -49,7 +48,7 @@ class TransformTestBase : public BASE { files_.emplace_back(std::move(file)); if (!program.IsValid()) { - return Transform::Output(std::move(program)); + return Output(std::move(program)); } Manager manager; @@ -65,9 +64,9 @@ class TransformTestBase : public BASE { /// @param in the input WGSL source /// @param data the optional DataMap to pass to Transform::Run() /// @return the transformed output - Transform::Output Run(std::string in, - std::unique_ptr transform, - const DataMap& data = {}) { + Output Run(std::string in, + std::unique_ptr transform, + const DataMap& data = {}) { std::vector> transforms; transforms.emplace_back(std::move(transform)); return Run(std::move(in), std::move(transforms), data); @@ -79,14 +78,14 @@ class TransformTestBase : public BASE { /// @param data the optional DataMap to pass to Transform::Run() /// @return the transformed output template - Transform::Output Run(std::string in, const DataMap& data = {}) { + Output Run(std::string in, const DataMap& data = {}) { return Run(std::move(in), std::make_unique(), data); } /// @param output the output of the transform /// @returns the output program as a WGSL string, or an error string if the /// program is not valid. - std::string str(const Transform::Output& output) { + std::string str(const Output& output) { diag::Formatter::Style style; style.print_newline_at_end = false; diff --git a/src/transform/transform.cc b/src/transform/transform.cc index a9c3816174..503e5a5880 100644 --- a/src/transform/transform.cc +++ b/src/transform/transform.cc @@ -31,9 +31,10 @@ Data& Data::operator=(const Data&) = default; DataMap::DataMap() = default; DataMap::DataMap(DataMap&&) = default; DataMap::~DataMap() = default; +DataMap& DataMap::operator=(DataMap&&) = default; -Transform::Output::Output() = default; -Transform::Output::Output(Program&& p) : program(std::move(p)) {} +Output::Output() = default; +Output::Output(Program&& p) : program(std::move(p)) {} Transform::Transform() = default; Transform::~Transform() = default; diff --git a/src/transform/transform.h b/src/transform/transform.h index 21e9f10012..533f0f8015 100644 --- a/src/transform/transform.h +++ b/src/transform/transform.h @@ -62,6 +62,11 @@ class DataMap { /// Destructor ~DataMap(); + /// Move assignment operator + /// @param rhs the DataMap to move into this DataMap + /// @return this DataMap + DataMap& operator=(DataMap&& rhs); + /// Adds the data into DataMap keyed by the ClassID of type T. /// @param data the data to add to the DataMap template @@ -114,39 +119,42 @@ class DataMap { std::unordered_map> map_; }; +/// The return type of Run() +class Output { + public: + /// Constructor + Output(); + + /// Constructor + /// @param program the program to move into this Output + explicit Output(Program&& program); + + /// Constructor + /// @param program_ the program to move into this Output + /// @param data_ a variadic list of additional data unique_ptrs produced by + /// the transform + template + Output(Program&& program_, DATA... data_) + : program(std::move(program_)), data(std::forward(data_)...) {} + + /// The transformed program. May be empty on error. + Program program; + + /// Extra output generated by the transforms. + DataMap data; +}; + /// Interface for Program transforms class Transform { public: + /// [DEPRECATED]: Use transform::Output + using Output = transform::Output; + /// Constructor Transform(); /// Destructor virtual ~Transform(); - /// The return type of Run() - class Output { - public: - /// Constructor - Output(); - - /// Constructor - /// @param program the program to move into this Output - explicit Output(Program&& program); - - /// Constructor - /// @param program_ the program to move into this Output - /// @param data_ a variadic list of additional data unique_ptrs produced by - /// the transform - template - Output(Program&& program_, DATA... data_) - : program(std::move(program_)), data(std::forward(data_)...) {} - - /// The transformed program. May be empty on error. - Program program; - - /// Extra output generated by the transforms. - DataMap data; - }; - /// Runs the transform on `program`, returning the transformation result. /// @param program the source program to transform /// @param data optional extra transform-specific input data diff --git a/src/transform/vertex_pulling.cc b/src/transform/vertex_pulling.cc index af4f00b7c4..3657ea59cf 100644 --- a/src/transform/vertex_pulling.cc +++ b/src/transform/vertex_pulling.cc @@ -390,7 +390,7 @@ VertexPulling::VertexPulling(const Config& config) : cfg_(config) {} VertexPulling::~VertexPulling() = default; -Transform::Output VertexPulling::Run(const Program* in, const DataMap& data) { +Output VertexPulling::Run(const Program* in, const DataMap& data) { ProgramBuilder out; auto cfg = cfg_;