From af8a8ac3d61a0f2b319b812af689510445960b07 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 29 Mar 2021 21:03:59 +0000 Subject: [PATCH] transform: Add Data as an input Like 42264, this is an attempt to keep transforms immutable, and to keep mutable I/O information in Data objects. Bug: tint:389 Change-Id: Ie417872d3e97f3db3904245a30aa74fdcce76610 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46260 Commit-Queue: Ben Clayton Reviewed-by: James Price --- src/transform/bound_array_accessors.cc | 2 +- src/transform/bound_array_accessors.h | 3 ++- src/transform/emit_vertex_point_size.cc | 2 +- src/transform/emit_vertex_point_size.h | 3 ++- src/transform/first_index_offset.cc | 3 +-- src/transform/first_index_offset.h | 4 +++- src/transform/first_index_offset_test.cc | 10 +++++----- src/transform/hlsl.cc | 2 +- src/transform/hlsl.h | 3 ++- src/transform/manager.cc | 4 ++-- src/transform/manager.h | 3 ++- src/transform/msl.cc | 2 +- src/transform/msl.h | 3 ++- src/transform/renamer.cc | 2 +- src/transform/renamer.h | 4 +++- src/transform/spirv.cc | 2 +- src/transform/spirv.h | 3 ++- src/transform/test_helper.h | 21 ++++++++++++--------- src/transform/transform.h | 15 ++++++++++++--- src/transform/vertex_pulling.cc | 2 +- src/transform/vertex_pulling.h | 3 ++- 21 files changed, 59 insertions(+), 37 deletions(-) diff --git a/src/transform/bound_array_accessors.cc b/src/transform/bound_array_accessors.cc index bbb50d7f1d..5eecfec70c 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) { +Transform::Output BoundArrayAccessors::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); ctx.ReplaceAll([&](ast::ArrayAccessorExpression* expr) { diff --git a/src/transform/bound_array_accessors.h b/src/transform/bound_array_accessors.h index d0d0a9fb29..c2330ad476 100644 --- a/src/transform/bound_array_accessors.h +++ b/src/transform/bound_array_accessors.h @@ -34,8 +34,9 @@ class BoundArrayAccessors : public Transform { /// 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) override; + Output Run(const Program* program, const DataMap& data = {}) override; private: ast::ArrayAccessorExpression* Transform(ast::ArrayAccessorExpression* expr, diff --git a/src/transform/emit_vertex_point_size.cc b/src/transform/emit_vertex_point_size.cc index ef2eb62f7f..944659fc14 100644 --- a/src/transform/emit_vertex_point_size.cc +++ b/src/transform/emit_vertex_point_size.cc @@ -30,7 +30,7 @@ const char kPointSizeVar[] = "tint_pointsize"; EmitVertexPointSize::EmitVertexPointSize() = default; EmitVertexPointSize::~EmitVertexPointSize() = default; -Transform::Output EmitVertexPointSize::Run(const Program* in) { +Transform::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/emit_vertex_point_size.h b/src/transform/emit_vertex_point_size.h index 210cd5106a..0e597a1a37 100644 --- a/src/transform/emit_vertex_point_size.h +++ b/src/transform/emit_vertex_point_size.h @@ -34,8 +34,9 @@ class EmitVertexPointSize : public Transform { /// 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) override; + Output Run(const Program* program, const DataMap& data = {}) override; }; } // namespace transform diff --git a/src/transform/first_index_offset.cc b/src/transform/first_index_offset.cc index cdc0288adb..3766e013b4 100644 --- a/src/transform/first_index_offset.cc +++ b/src/transform/first_index_offset.cc @@ -67,7 +67,7 @@ FirstIndexOffset::FirstIndexOffset(uint32_t binding, uint32_t group) FirstIndexOffset::~FirstIndexOffset() = default; -Transform::Output FirstIndexOffset::Run(const Program* in) { +Transform::Output FirstIndexOffset::Run(const Program* in, const DataMap&) { ProgramBuilder out; // First do a quick check to see if the transform has already been applied. @@ -191,7 +191,6 @@ ast::Variable* FirstIndexOffset::State::AddUniformBuffer() { dst->create(Source{}, group), }); - dst->AST().AddConstructedType(struct_type); dst->AST().AddGlobalVariable(idx_var); diff --git a/src/transform/first_index_offset.h b/src/transform/first_index_offset.h index dd187d2702..ed092fb5dc 100644 --- a/src/transform/first_index_offset.h +++ b/src/transform/first_index_offset.h @@ -60,6 +60,7 @@ namespace transform { /// class FirstIndexOffset : public Transform { public: + /// Data is outputted by the FirstIndexOffset transform. /// Data holds information about shader usage and constant buffer offsets. struct Data : public Castable { /// Constructor @@ -96,8 +97,9 @@ class FirstIndexOffset : public Transform { /// 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) override; + Output Run(const Program* program, const DataMap& data = {}) override; private: struct State { diff --git a/src/transform/first_index_offset_test.cc b/src/transform/first_index_offset_test.cc index 53d8d0fa5e..2c76e82694 100644 --- a/src/transform/first_index_offset_test.cc +++ b/src/transform/first_index_offset_test.cc @@ -64,7 +64,7 @@ TEST_F(FirstIndexOffsetTest, EmptyModule) { auto* src = ""; auto* expect = ""; - auto got = Run(src, 0, 0); + auto got = Run(src, std::make_unique(0, 0)); EXPECT_EQ(expect, str(got)); @@ -112,7 +112,7 @@ fn entry() -> void { } )"; - auto got = Run(src, 1, 2); + auto got = Run(src, std::make_unique(1, 2)); EXPECT_EQ(expect, str(got)); @@ -160,7 +160,7 @@ fn entry() -> void { } )"; - auto got = Run(src, 1, 7); + auto got = Run(src, std::make_unique(1, 7)); EXPECT_EQ(expect, str(got)); @@ -213,7 +213,7 @@ fn entry() -> void { } )"; - auto got = Run(src, 1, 2); + auto got = Run(src, std::make_unique(1, 2)); EXPECT_EQ(expect, str(got)); @@ -269,7 +269,7 @@ fn entry() -> void { } )"; - auto got = Run(src, 1, 2); + auto got = Run(src, std::make_unique(1, 2)); EXPECT_EQ(expect, str(got)); diff --git a/src/transform/hlsl.cc b/src/transform/hlsl.cc index 8ed06ab907..87e8c6e270 100644 --- a/src/transform/hlsl.cc +++ b/src/transform/hlsl.cc @@ -29,7 +29,7 @@ namespace transform { Hlsl::Hlsl() = default; Hlsl::~Hlsl() = default; -Transform::Output Hlsl::Run(const Program* in) { +Transform::Output Hlsl::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); PromoteArrayInitializerToConstVar(ctx); diff --git a/src/transform/hlsl.h b/src/transform/hlsl.h index 7213404ad9..bf6a124647 100644 --- a/src/transform/hlsl.h +++ b/src/transform/hlsl.h @@ -35,8 +35,9 @@ class Hlsl : public Transform { /// Runs the transform on `program`, returning the transformation result. /// @param program the source program to transform + /// @param data optional extra transform-specific data /// @returns the transformation result - Output Run(const Program* program) override; + Output Run(const Program* program, const DataMap& data = {}) override; private: /// Hoists the array initializer to a constant variable, declared just before diff --git a/src/transform/manager.cc b/src/transform/manager.cc index 4f4fb73c4f..ef6f79e631 100644 --- a/src/transform/manager.cc +++ b/src/transform/manager.cc @@ -20,11 +20,11 @@ namespace transform { Manager::Manager() = default; Manager::~Manager() = default; -Transform::Output Manager::Run(const Program* program) { +Transform::Output Manager::Run(const Program* program, const DataMap& data) { Output out; if (!transforms_.empty()) { for (auto& transform : transforms_) { - auto res = transform->Run(program); + auto res = transform->Run(program, data); out.program = std::move(res.program); out.data.Add(std::move(res.data)); if (!out.program.IsValid()) { diff --git a/src/transform/manager.h b/src/transform/manager.h index 1351b0a40c..8e860052de 100644 --- a/src/transform/manager.h +++ b/src/transform/manager.h @@ -42,8 +42,9 @@ class Manager : public Transform { /// Runs the transforms on `program`, returning the transformation result. /// @param program the source program to transform + /// @param data optional extra transform-specific input data /// @returns the transformed program and diagnostics - Output Run(const Program* program) override; + Output Run(const Program* program, const DataMap& data = {}) override; private: std::vector> transforms_; diff --git a/src/transform/msl.cc b/src/transform/msl.cc index 6245fc7f88..ffcb6b102e 100644 --- a/src/transform/msl.cc +++ b/src/transform/msl.cc @@ -266,7 +266,7 @@ const char* kReservedKeywords[] = {"access", Msl::Msl() = default; Msl::~Msl() = default; -Transform::Output Msl::Run(const Program* in) { +Transform::Output Msl::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); RenameReservedKeywords(&ctx, kReservedKeywords); diff --git a/src/transform/msl.h b/src/transform/msl.h index 38a2cd1ab5..13abdca57e 100644 --- a/src/transform/msl.h +++ b/src/transform/msl.h @@ -31,8 +31,9 @@ class Msl : public Transform { /// 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) override; + Output Run(const Program* program, const DataMap& data = {}) override; private: /// Hoist location-decorated entry point parameters out to struct members. diff --git a/src/transform/renamer.cc b/src/transform/renamer.cc index b6f31a0e1b..9277136643 100644 --- a/src/transform/renamer.cc +++ b/src/transform/renamer.cc @@ -39,7 +39,7 @@ Renamer::Renamer(const Config& config) : cfg_(config) {} Renamer::~Renamer() = default; -Transform::Output Renamer::Run(const Program* in) { +Transform::Output Renamer::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); diff --git a/src/transform/renamer.h b/src/transform/renamer.h index 2b5e21975b..4670482846 100644 --- a/src/transform/renamer.h +++ b/src/transform/renamer.h @@ -26,6 +26,7 @@ namespace transform { /// Renamer is a Transform that renames all the symbols in a program. class Renamer : public Transform { public: + /// Data is outputted by the Renamer transform. /// Data holds information about shader usage and constant buffer offsets. struct Data : public Castable { /// Remappings is a map of old symbol name to new symbol name @@ -70,8 +71,9 @@ class Renamer : public Transform { /// 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) override; + Output Run(const Program* program, const DataMap& data = {}) override; private: Config const cfg_; diff --git a/src/transform/spirv.cc b/src/transform/spirv.cc index 06bd5f63fc..1eb7a6aadc 100644 --- a/src/transform/spirv.cc +++ b/src/transform/spirv.cc @@ -29,7 +29,7 @@ namespace transform { Spirv::Spirv() = default; Spirv::~Spirv() = default; -Transform::Output Spirv::Run(const Program* in) { +Transform::Output Spirv::Run(const Program* in, const DataMap&) { ProgramBuilder out; CloneContext ctx(&out, in); HandleEntryPointIOTypes(ctx); diff --git a/src/transform/spirv.h b/src/transform/spirv.h index e2d1f12ba8..40bc54ee8d 100644 --- a/src/transform/spirv.h +++ b/src/transform/spirv.h @@ -37,8 +37,9 @@ class Spirv : public Transform { /// 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) override; + Output Run(const Program* program, const DataMap& data = {}) override; private: /// Hoist entry point parameters, return values, and struct members out to diff --git a/src/transform/test_helper.h b/src/transform/test_helper.h index f8fc8b5306..4dc38d95fe 100644 --- a/src/transform/test_helper.h +++ b/src/transform/test_helper.h @@ -36,10 +36,12 @@ class TransformTestBase : public BASE { /// `transforms`. /// @param in the input WGSL source /// @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) { + std::vector> transforms, + DataMap data = {}) { auto file = std::make_unique("test", in); auto program = reader::wgsl::Parse(file.get()); @@ -54,30 +56,31 @@ class TransformTestBase : public BASE { for (auto& transform : transforms) { manager.append(std::move(transform)); } - return manager.Run(&program); + return manager.Run(&program, data); } /// Transforms and returns the WGSL source `in`, transformed using /// `transform`. /// @param transform the transform to apply /// @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) { + std::unique_ptr transform, + DataMap data = {}) { std::vector> transforms; transforms.emplace_back(std::move(transform)); - return Run(std::move(in), std::move(transforms)); + return Run(std::move(in), std::move(transforms), std::move(data)); } /// Transforms and returns the WGSL source `in`, transformed using /// a transform of type `TRANSFORM`. /// @param in the input WGSL source - /// @param args the TRANSFORM constructor arguments + /// @param data the optional DataMap to pass to Transform::Run() /// @return the transformed output - template - Transform::Output Run(std::string in, ARGS&&... args) { - return Run(std::move(in), - std::make_unique(std::forward(args)...)); + template + Transform::Output Run(std::string in, DataMap data = {}) { + return Run(std::move(in), std::make_unique(), std::move(data)); } /// @param output the output of the transform diff --git a/src/transform/transform.h b/src/transform/transform.h index 8b475bb0ad..e87d2c5049 100644 --- a/src/transform/transform.h +++ b/src/transform/transform.h @@ -24,8 +24,8 @@ namespace tint { namespace transform { -/// Data is the base class for transforms that emit extra output information -/// along with a Program. +/// Data is the base class for transforms that accept extra input or emit extra +/// output information along with a Program. class Data : public Castable { public: /// Constructor @@ -67,6 +67,14 @@ class DataMap { map_[&TypeInfo::Of()] = std::move(data); } + /// Creates the data of type `T` with the provided arguments and adds it into + /// DataMap keyed by the ClassID of type T. + /// @param args the arguments forwarded to the constructor for type T + template + void Add(ARGS&&... args) { + Put(std::make_unique(std::forward(args)...)); + } + /// @returns a pointer to the Data placed into the DataMap with a call to /// Put() template @@ -137,8 +145,9 @@ class Transform { /// 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 - virtual Output Run(const Program* program) = 0; + virtual Output Run(const Program* program, const DataMap& data = {}) = 0; protected: /// Clones the function `in` adding `statements` to the beginning of the diff --git a/src/transform/vertex_pulling.cc b/src/transform/vertex_pulling.cc index 638eb53818..9abcbd9c27 100644 --- a/src/transform/vertex_pulling.cc +++ b/src/transform/vertex_pulling.cc @@ -40,7 +40,7 @@ VertexPulling::VertexPulling(const Config& config) : cfg(config) {} VertexPulling::~VertexPulling() = default; -Transform::Output VertexPulling::Run(const Program* in) { +Transform::Output VertexPulling::Run(const Program* in, const DataMap&) { ProgramBuilder out; // Find entry point diff --git a/src/transform/vertex_pulling.h b/src/transform/vertex_pulling.h index bf219899b9..05b3a839db 100644 --- a/src/transform/vertex_pulling.h +++ b/src/transform/vertex_pulling.h @@ -162,8 +162,9 @@ class VertexPulling : public Transform { /// 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) override; + Output Run(const Program* program, const DataMap& data = {}) override; private: Config cfg;