From 97b729de106919ac8df0965623ba53e134766032 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Tue, 29 Sep 2020 20:28:21 +0000 Subject: [PATCH] [transform] Add Transformer base class This CL adds a Transformer base class from which the transformers will inherit. Bug: tint:206 Change-Id: I542eacb05d9a92af46d172a5803c245472c0e22c Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/29120 Commit-Queue: dan sinclair Reviewed-by: Ryan Harrison --- BUILD.gn | 2 + src/CMakeLists.txt | 2 + .../bound_array_accessors_transform.cc | 2 +- .../bound_array_accessors_transform.h | 13 ++--- src/transform/transformer.cc | 26 +++++++++ src/transform/transformer.h | 53 +++++++++++++++++++ src/transform/vertex_pulling_transform.cc | 10 ++-- src/transform/vertex_pulling_transform.h | 15 ++---- .../vertex_pulling_transform_test.cc | 8 +-- 9 files changed, 99 insertions(+), 32 deletions(-) create mode 100644 src/transform/transformer.cc create mode 100644 src/transform/transformer.h diff --git a/BUILD.gn b/BUILD.gn index 69cc327c69..0f40dd40e0 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -375,6 +375,8 @@ source_set("libtint_core_src") { "src/source.h", "src/transform/bound_array_accessors_transform.cc", "src/transform/bound_array_accessors_transform.h", + "src/transform/transformer.cc", + "src/transform/transformer.h", "src/transform/vertex_pulling_transform.cc", "src/transform/vertex_pulling_transform.h", "src/type_determiner.cc", diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 4325e8e571..c7730d6896 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -196,6 +196,8 @@ set(TINT_LIB_SRCS source.h transform/bound_array_accessors_transform.cc transform/bound_array_accessors_transform.h + transform/transformer.cc + transform/transformer.h transform/vertex_pulling_transform.cc transform/vertex_pulling_transform.h type_determiner.cc diff --git a/src/transform/bound_array_accessors_transform.cc b/src/transform/bound_array_accessors_transform.cc index 8ae5bd84ea..82ecd66efe 100644 --- a/src/transform/bound_array_accessors_transform.cc +++ b/src/transform/bound_array_accessors_transform.cc @@ -45,7 +45,7 @@ namespace transform { BoundArrayAccessorsTransform::BoundArrayAccessorsTransform(Context* ctx, ast::Module* mod) - : ctx_(ctx), mod_(mod) {} + : Transformer(ctx, mod) {} BoundArrayAccessorsTransform::~BoundArrayAccessorsTransform() = default; diff --git a/src/transform/bound_array_accessors_transform.h b/src/transform/bound_array_accessors_transform.h index 2a40225fe1..5fe2fbbc3e 100644 --- a/src/transform/bound_array_accessors_transform.h +++ b/src/transform/bound_array_accessors_transform.h @@ -21,6 +21,7 @@ #include "src/ast/statement.h" #include "src/context.h" #include "src/scope_stack.h" +#include "src/transform/transformer.h" #include @@ -31,19 +32,16 @@ namespace transform { /// the bounds of the array. Any access before the start of the array will clamp /// to zero and any access past the end of the array will clamp to /// (array length - 1). -class BoundArrayAccessorsTransform { +class BoundArrayAccessorsTransform : public Transformer { public: /// Constructor /// @param ctx the Tint context object /// @param mod the module transform explicit BoundArrayAccessorsTransform(Context* ctx, ast::Module* mod); - ~BoundArrayAccessorsTransform(); + ~BoundArrayAccessorsTransform() override; /// @returns true if the transformation was successful - bool Run(); - - /// @returns error messages - const std::string& error() { return error_; } + bool Run() override; private: bool ProcessStatement(ast::Statement* stmt); @@ -52,9 +50,6 @@ class BoundArrayAccessorsTransform { bool ProcessAccessExpression(ast::ArrayAccessorExpression* expr, uint32_t size); - Context* ctx_ = nullptr; - ast::Module* mod_ = nullptr; - std::string error_; ScopeStack scope_stack_; }; diff --git a/src/transform/transformer.cc b/src/transform/transformer.cc new file mode 100644 index 0000000000..c3b7770603 --- /dev/null +++ b/src/transform/transformer.cc @@ -0,0 +1,26 @@ +// 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/transformer.h" + +namespace tint { +namespace transform { + +Transformer::Transformer(Context* ctx, ast::Module* mod) + : ctx_(ctx), mod_(mod) {} + +Transformer::~Transformer() = default; + +} // namespace transform +} // namespace tint diff --git a/src/transform/transformer.h b/src/transform/transformer.h new file mode 100644 index 0000000000..004133edd7 --- /dev/null +++ b/src/transform/transformer.h @@ -0,0 +1,53 @@ +// 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. + +#ifndef SRC_TRANSFORM_TRANSFORMER_H_ +#define SRC_TRANSFORM_TRANSFORMER_H_ + +#include + +#include "src/ast/module.h" +#include "src/context.h" + +namespace tint { +namespace transform { + +/// Interface class for the transformers +class Transformer { + public: + /// Constructor + /// @param ctx the Tint context + /// @param mod the module to transform + Transformer(Context* ctx, ast::Module* mod); + virtual ~Transformer(); + + /// @returns true if the transformation was successful + virtual bool Run() = 0; + + /// @returns error messages + const std::string& error() { return error_; } + + protected: + /// The context + Context* ctx_ = nullptr; + /// The module + ast::Module* mod_ = nullptr; + /// Any error messages, or blank if no error + std::string error_; +}; + +} // namespace transform +} // namespace tint + +#endif // SRC_TRANSFORM_TRANSFORMER_H_ diff --git a/src/transform/vertex_pulling_transform.cc b/src/transform/vertex_pulling_transform.cc index ab668f99bc..fe1219298e 100644 --- a/src/transform/vertex_pulling_transform.cc +++ b/src/transform/vertex_pulling_transform.cc @@ -48,7 +48,7 @@ static const char kDefaultInstanceIndexName[] = "_tint_pulling_instance_index"; } // namespace VertexPullingTransform::VertexPullingTransform(Context* ctx, ast::Module* mod) - : ctx_(ctx), mod_(mod) {} + : Transformer(ctx, mod) {} VertexPullingTransform::~VertexPullingTransform() = default; @@ -68,7 +68,7 @@ void VertexPullingTransform::SetPullingBufferBindingSet(uint32_t number) { bool VertexPullingTransform::Run() { // Check SetVertexState was called if (vertex_state_ == nullptr) { - SetError("SetVertexState not called"); + error_ = "SetVertexState not called"; return false; } @@ -76,7 +76,7 @@ bool VertexPullingTransform::Run() { auto* func = mod_->FindFunctionByNameAndStage(entry_point_name_, ast::PipelineStage::kVertex); if (func == nullptr) { - SetError("Vertex stage entry point not found"); + error_ = "Vertex stage entry point not found"; return false; } @@ -98,10 +98,6 @@ bool VertexPullingTransform::Run() { return true; } -void VertexPullingTransform::SetError(const std::string& error) { - error_ = error; -} - std::string VertexPullingTransform::GetVertexBufferName(uint32_t index) { return kVertexBufferNamePrefix + std::to_string(index); } diff --git a/src/transform/vertex_pulling_transform.h b/src/transform/vertex_pulling_transform.h index c1d9195fbf..805f7514ed 100644 --- a/src/transform/vertex_pulling_transform.h +++ b/src/transform/vertex_pulling_transform.h @@ -21,6 +21,7 @@ #include "src/ast/statement.h" #include "src/ast/variable.h" #include "src/context.h" +#include "src/transform/transformer.h" #include #include @@ -142,13 +143,13 @@ struct VertexStateDescriptor { /// To be clear, there won't be types such as f16 or u8 anywhere in WGSL code, /// but these are types that the data may arrive as. We need to convert these /// smaller types into the base types such as f32 and u32 for the shader to use. -class VertexPullingTransform { +class VertexPullingTransform : public Transformer { public: /// Constructor /// @param ctx the tint context /// @param mod the module to convert to vertex pulling VertexPullingTransform(Context* ctx, ast::Module* mod); - ~VertexPullingTransform(); + ~VertexPullingTransform() override; /// Sets the vertex state descriptor, containing info about attributes /// @param vertex_state the vertex state descriptor @@ -164,14 +165,9 @@ class VertexPullingTransform { void SetPullingBufferBindingSet(uint32_t number); /// @returns true if the transformation was successful - bool Run(); - - /// @returns error messages - const std::string& GetError() { return error_; } + bool Run() override; private: - void SetError(const std::string& error); - /// Generate the vertex buffer binding name /// @param index index to append to buffer name std::string GetVertexBufferName(uint32_t index); @@ -256,10 +252,7 @@ class VertexPullingTransform { ast::type::Type* GetI32Type(); ast::type::Type* GetF32Type(); - Context* ctx_ = nullptr; - ast::Module* mod_ = nullptr; std::string entry_point_name_; - std::string error_; std::string vertex_index_name_; std::string instance_index_name_; diff --git a/src/transform/vertex_pulling_transform_test.cc b/src/transform/vertex_pulling_transform_test.cc index a30065d76c..666416f930 100644 --- a/src/transform/vertex_pulling_transform_test.cc +++ b/src/transform/vertex_pulling_transform_test.cc @@ -88,13 +88,13 @@ class VertexPullingTransformTest : public VertexPullingTransformHelper, TEST_F(VertexPullingTransformTest, Error_NoVertexState) { EXPECT_FALSE(transform()->Run()); - EXPECT_EQ(transform()->GetError(), "SetVertexState not called"); + EXPECT_EQ(transform()->error(), "SetVertexState not called"); } TEST_F(VertexPullingTransformTest, Error_NoEntryPoint) { transform()->SetVertexState(std::make_unique()); EXPECT_FALSE(transform()->Run()); - EXPECT_EQ(transform()->GetError(), "Vertex stage entry point not found"); + EXPECT_EQ(transform()->error(), "Vertex stage entry point not found"); } TEST_F(VertexPullingTransformTest, Error_InvalidEntryPoint) { @@ -103,7 +103,7 @@ TEST_F(VertexPullingTransformTest, Error_InvalidEntryPoint) { transform()->SetEntryPoint("_"); EXPECT_FALSE(transform()->Run()); - EXPECT_EQ(transform()->GetError(), "Vertex stage entry point not found"); + EXPECT_EQ(transform()->error(), "Vertex stage entry point not found"); } TEST_F(VertexPullingTransformTest, Error_EntryPointWrongStage) { @@ -116,7 +116,7 @@ TEST_F(VertexPullingTransformTest, Error_EntryPointWrongStage) { InitTransform({}); EXPECT_FALSE(transform()->Run()); - EXPECT_EQ(transform()->GetError(), "Vertex stage entry point not found"); + EXPECT_EQ(transform()->error(), "Vertex stage entry point not found"); } TEST_F(VertexPullingTransformTest, BasicModule) {