From 2ce67252c8a09cbe4f1613ea5add266ffdcaf3d0 Mon Sep 17 00:00:00 2001 From: qining Date: Wed, 31 Aug 2016 12:44:49 -0400 Subject: [PATCH] Add forwarding so that passes' ctor can have args. Also removed the default argument value of `skip_nop` for function `SinglePassRunAndCheck()` and `SinglePassRunAndDisassemble()`. This is required to support variadic arguments. --- source/opt/pass_manager.h | 8 ++++--- test/opt/assembly_builder.h | 0 test/opt/pass_fixture.h | 33 ++++++++++++------------- test/opt/test_assembly_builder.cpp | 12 ++++++---- test/opt/test_freeze_spec_const.cpp | 2 +- test/opt/test_pass_manager.cpp | 48 +++++++++++++++++++++++++++++++++++++ test/opt/test_strip_debug_info.cpp | 9 ++++--- 7 files changed, 85 insertions(+), 27 deletions(-) mode change 100755 => 100644 test/opt/assembly_builder.h diff --git a/source/opt/pass_manager.h b/source/opt/pass_manager.h index a4ca586..9424370 100644 --- a/source/opt/pass_manager.h +++ b/source/opt/pass_manager.h @@ -50,9 +50,11 @@ class PassManager { void AddPass(std::unique_ptr pass) { passes_.push_back(std::move(pass)); } - template - void AddPass() { - passes_.emplace_back(new PassT); + // Uses the argument to construct a pass instance of type PassT, and adds the + // pass instance to this pass manger. + template + void AddPass(Args&&... args) { + passes_.emplace_back(new PassT(std::forward(args)...)); } // Returns the number of passes added. diff --git a/test/opt/assembly_builder.h b/test/opt/assembly_builder.h old mode 100755 new mode 100644 diff --git a/test/opt/pass_fixture.h b/test/opt/pass_fixture.h index 848de7d..dd560eb 100644 --- a/test/opt/pass_fixture.h +++ b/test/opt/pass_fixture.h @@ -34,9 +34,9 @@ #include #include "opt/libspirv.hpp" +#include "opt/make_unique.h" #include "opt/pass_manager.h" #include "opt/passes.h" -#include "opt/make_unique.h" namespace spvtools { @@ -57,10 +57,10 @@ class PassTest : public TestT { // disassebles the optimized binary. Returns a tuple of disassembly string // and the boolean value returned from pass Process() function. std::tuple OptimizeAndDisassemble( - opt::Pass* pass, const std::string& original, bool skip_nop = false) { + opt::Pass* pass, const std::string& original, bool skip_nop) { std::unique_ptr module = tools_.BuildModule(original); - EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n" << original - << std::endl; + EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n" + << original << std::endl; if (!module) { return std::make_tuple(std::string(), false); } @@ -71,17 +71,18 @@ class PassTest : public TestT { module->ToBinary(&binary, skip_nop); std::string optimized; EXPECT_EQ(SPV_SUCCESS, tools_.Disassemble(binary, &optimized)) - << "Disassembling failed for shader:\n" << original << std::endl; + << "Disassembling failed for shader:\n" + << original << std::endl; return std::make_tuple(optimized, modified); } // Runs a single pass of class |PassT| on the binary assembled from the // |assembly|, disassembles the optimized binary. Returns a tuple of // disassembly string and the boolean value from the pass Process() function. - template + template std::tuple SinglePassRunAndDisassemble( - const std::string& assembly, bool skip_nop = false) { - auto pass = MakeUnique(); + const std::string& assembly, bool skip_nop, Args&&... args) { + auto pass = MakeUnique(std::forward(args)...); return OptimizeAndDisassemble(pass.get(), assembly, skip_nop); } @@ -89,23 +90,23 @@ class PassTest : public TestT { // |original| assembly, and checks whether the optimized binary can be // disassembled to the |expected| assembly. This does *not* involve pass // manager. Callers are suggested to use SCOPED_TRACE() for better messages. - template + template void SinglePassRunAndCheck(const std::string& original, - const std::string& expected, - bool skip_nop = false) { + const std::string& expected, bool skip_nop, + Args&&... args) { std::string optimized; bool modified = false; - std::tie(optimized, modified) = - SinglePassRunAndDisassemble(original, skip_nop); + std::tie(optimized, modified) = SinglePassRunAndDisassemble( + original, skip_nop, std::forward(args)...); // Check whether the pass returns the correct modification indication. EXPECT_EQ(original != expected, modified); EXPECT_EQ(expected, optimized); } // Adds a pass to be run. - template - void AddPass() { - manager_->AddPass(); + template + void AddPass(Args&&... args) { + manager_->AddPass(std::forward(args)...); } // Renews the pass manager, including clearing all previously added passes. diff --git a/test/opt/test_assembly_builder.cpp b/test/opt/test_assembly_builder.cpp index 597229e..6da1e8f 100644 --- a/test/opt/test_assembly_builder.cpp +++ b/test/opt/test_assembly_builder.cpp @@ -57,7 +57,8 @@ TEST_F(AssemblyBuilderTest, MinimalShader) { }; SinglePassRunAndCheck(builder.GetCode(), - JoinAllInsts(expected)); + JoinAllInsts(expected), + /* skip_nop = */ false); } TEST_F(AssemblyBuilderTest, ShaderWithConstants) { @@ -170,7 +171,8 @@ TEST_F(AssemblyBuilderTest, ShaderWithConstants) { // clang-format on }; SinglePassRunAndCheck(builder.GetCode(), - JoinAllInsts(expected)); + JoinAllInsts(expected), + /* skip_nop = */ false); } TEST_F(AssemblyBuilderTest, SpecConstants) { @@ -250,7 +252,8 @@ TEST_F(AssemblyBuilderTest, SpecConstants) { }; SinglePassRunAndCheck(builder.GetCode(), - JoinAllInsts(expected)); + JoinAllInsts(expected), + /* skip_nop = */ false); } TEST_F(AssemblyBuilderTest, AppendNames) { @@ -283,7 +286,8 @@ TEST_F(AssemblyBuilderTest, AppendNames) { }; SinglePassRunAndCheck(builder.GetCode(), - JoinAllInsts(expected)); + JoinAllInsts(expected), + /* skip_nop = */ false); } } // anonymous namespace diff --git a/test/opt/test_freeze_spec_const.cpp b/test/opt/test_freeze_spec_const.cpp index 2be826b..ecb3c62 100644 --- a/test/opt/test_freeze_spec_const.cpp +++ b/test/opt/test_freeze_spec_const.cpp @@ -53,7 +53,7 @@ TEST_P(FreezeSpecConstantValueTypeTest, PrimaryType) { "OpCapability Shader", "OpMemoryModel Logical GLSL450", test_case.type_decl, test_case.expected_frozen_const}; SinglePassRunAndCheck( - JoinAllInsts(text), JoinAllInsts(expected)); + JoinAllInsts(text), JoinAllInsts(expected), /* skip_nop = */ false); } // Test each primary type. diff --git a/test/opt/test_pass_manager.cpp b/test/opt/test_pass_manager.cpp index 8b93fc5..51fcd55 100644 --- a/test/opt/test_pass_manager.cpp +++ b/test/opt/test_pass_manager.cpp @@ -26,6 +26,8 @@ #include "gmock/gmock.h" +#include + #include "module_utils.h" #include "opt/make_unique.h" #include "pass_fixture.h" @@ -36,6 +38,17 @@ using namespace spvtools; using spvtest::GetIdBound; using ::testing::Eq; +// A null pass whose construtors accept arguments +class NullPassWithArgs : public opt::NullPass { + public: + NullPassWithArgs(uint32_t) : NullPass() {} + NullPassWithArgs(std::string) : NullPass() {} + NullPassWithArgs(const std::vector&) : NullPass() {} + NullPassWithArgs(const std::vector&, uint32_t) : NullPass() {} + + const char* name() const override { return "null-with-args"; } +}; + TEST(PassManager, Interface) { opt::PassManager manager; EXPECT_EQ(0u, manager.NumPasses()); @@ -54,6 +67,19 @@ TEST(PassManager, Interface) { EXPECT_STREQ("strip-debug", manager.GetPass(0)->name()); EXPECT_STREQ("null", manager.GetPass(1)->name()); EXPECT_STREQ("strip-debug", manager.GetPass(2)->name()); + + manager.AddPass(1u); + manager.AddPass("null pass args"); + manager.AddPass(std::initializer_list{1, 2}); + manager.AddPass(std::initializer_list{1, 2}, 3); + EXPECT_EQ(7u, manager.NumPasses()); + EXPECT_STREQ("strip-debug", manager.GetPass(0)->name()); + EXPECT_STREQ("null", manager.GetPass(1)->name()); + EXPECT_STREQ("strip-debug", manager.GetPass(2)->name()); + EXPECT_STREQ("null-with-args", manager.GetPass(3)->name()); + EXPECT_STREQ("null-with-args", manager.GetPass(4)->name()); + EXPECT_STREQ("null-with-args", manager.GetPass(5)->name()); + EXPECT_STREQ("null-with-args", manager.GetPass(6)->name()); } // A pass that appends an OpNop instruction to the debug section. @@ -66,6 +92,24 @@ class AppendOpNopPass : public opt::Pass { } }; +// A pass that appends specified number of OpNop instructions to the debug +// section. +class AppendMultipleOpNopPass : public opt::Pass { + public: + AppendMultipleOpNopPass(uint32_t num_nop) : num_nop_(num_nop) {} + const char* name() const override { return "AppendOpNop"; } + bool Process(ir::Module* module) override { + for (uint32_t i = 0; i < num_nop_; i++) { + auto inst = MakeUnique(); + module->AddDebugInst(std::move(inst)); + } + return true; + } + + private: + uint32_t num_nop_; +}; + // A pass that duplicates the last instruction in the debug section. class DuplicateInstPass : public opt::Pass { const char* name() const override { return "DuplicateInst"; } @@ -94,6 +138,10 @@ TEST_F(PassManagerTest, Run) { AddPass(); AddPass(); RunAndCheck(text.c_str(), (text + "OpSource ESSL 310\nOpNop\n").c_str()); + + RenewPassManger(); + AddPass(3); + RunAndCheck(text.c_str(), (text + "OpNop\nOpNop\nOpNop\n").c_str()); } // A pass that appends an OpTypeVoid instruction that uses a given id. diff --git a/test/opt/test_strip_debug_info.cpp b/test/opt/test_strip_debug_info.cpp index d9b35e1..2e59243 100644 --- a/test/opt/test_strip_debug_info.cpp +++ b/test/opt/test_strip_debug_info.cpp @@ -61,7 +61,8 @@ TEST_F(StripLineDebugInfoTest, LineNoLine) { // clang-format on }; SinglePassRunAndCheck(JoinAllInsts(text), - JoinNonDebugInsts(text)); + JoinNonDebugInsts(text), + /* skip_nop = */ false); // Let's add more debug instruction before the "OpString" instruction. const std::vector more_text = { @@ -79,7 +80,8 @@ TEST_F(StripLineDebugInfoTest, LineNoLine) { }; text.insert(text.begin() + 4, more_text.cbegin(), more_text.cend()); SinglePassRunAndCheck(JoinAllInsts(text), - JoinNonDebugInsts(text)); + JoinNonDebugInsts(text), + /* skip_nop = */ false); } using StripDebugInfoTest = PassTest<::testing::TestWithParam>; @@ -89,7 +91,8 @@ TEST_P(StripDebugInfoTest, Kind) { "OpCapability Shader", "OpMemoryModel Logical GLSL450", GetParam(), }; SinglePassRunAndCheck(JoinAllInsts(text), - JoinNonDebugInsts(text)); + JoinNonDebugInsts(text), + /* skip_nop = */ false); } // Test each possible non-line debug instruction. -- 2.7.4