From 2cbb2cce3ea1401bf8982c58d0d7dc8a8a0d4f33 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Mon, 12 Sep 2016 12:39:44 -0400 Subject: [PATCH] Change interface of Pass::Process() to return possible failures. --- source/opt/CMakeLists.txt | 1 + source/opt/eliminate_dead_constant_pass.cpp | 5 +- source/opt/eliminate_dead_constant_pass.h | 2 +- .../fold_spec_constant_op_and_composite_pass.cpp | 5 +- .../opt/fold_spec_constant_op_and_composite_pass.h | 6 +-- source/opt/freeze_spec_constant_value_pass.cpp | 4 +- source/opt/freeze_spec_constant_value_pass.h | 2 +- source/opt/null_pass.h | 2 +- source/opt/pass.h | 17 +++++-- source/opt/pass_manager.cpp | 35 +++++++++++++ source/opt/pass_manager.h | 58 +++++++++++++--------- .../opt/set_spec_constant_default_value_pass.cpp | 4 +- source/opt/set_spec_constant_default_value_pass.h | 2 +- source/opt/strip_debug_info_pass.cpp | 4 +- source/opt/strip_debug_info_pass.h | 2 +- source/opt/unify_const_pass.cpp | 4 +- source/opt/unify_const_pass.h | 2 +- test/opt/pass_fixture.h | 18 ++++--- test/opt/test_fold_spec_const_op_composite.cpp | 9 ++-- test/opt/test_line_debug_info.cpp | 13 +++-- test/opt/test_pass_manager.cpp | 22 ++++---- test/opt/test_unify_const.cpp | 31 ++++++------ tools/opt/opt.cpp | 1 + 23 files changed, 159 insertions(+), 90 deletions(-) create mode 100644 source/opt/pass_manager.cpp diff --git a/source/opt/CMakeLists.txt b/source/opt/CMakeLists.txt index b9a586f..aa85eb2 100644 --- a/source/opt/CMakeLists.txt +++ b/source/opt/CMakeLists.txt @@ -47,6 +47,7 @@ add_library(SPIRV-Tools-opt libspirv.cpp module.cpp set_spec_constant_default_value_pass.cpp + pass_manager.cpp strip_debug_info_pass.cpp types.cpp type_manager.cpp diff --git a/source/opt/eliminate_dead_constant_pass.cpp b/source/opt/eliminate_dead_constant_pass.cpp index c7a921f..9d7a148 100644 --- a/source/opt/eliminate_dead_constant_pass.cpp +++ b/source/opt/eliminate_dead_constant_pass.cpp @@ -25,7 +25,7 @@ namespace spvtools { namespace opt { -bool EliminateDeadConstantPass::Process(ir::Module* module) { +Pass::Status EliminateDeadConstantPass::Process(ir::Module* module) { analysis::DefUseManager def_use(consumer(), module); std::unordered_set working_list; // Traverse all the instructions to get the initial set of dead constants as @@ -109,7 +109,8 @@ bool EliminateDeadConstantPass::Process(ir::Module* module) { for (auto* da : dead_others) { da->ToNop(); } - return !dead_consts.empty(); + return dead_consts.empty() ? Status::SuccessWithoutChange + : Status::SuccessWithChange; } } // namespace opt diff --git a/source/opt/eliminate_dead_constant_pass.h b/source/opt/eliminate_dead_constant_pass.h index bc24eba..80d8a9a 100644 --- a/source/opt/eliminate_dead_constant_pass.h +++ b/source/opt/eliminate_dead_constant_pass.h @@ -31,7 +31,7 @@ class EliminateDeadConstantPass : public Pass { explicit EliminateDeadConstantPass(const MessageConsumer& c) : Pass(c) {} const char* name() const override { return "eliminate-dead-const"; } - bool Process(ir::Module*) override; + Status Process(ir::Module*) override; }; } // namespace opt diff --git a/source/opt/fold_spec_constant_op_and_composite_pass.cpp b/source/opt/fold_spec_constant_op_and_composite_pass.cpp index 6adb497..1922992 100644 --- a/source/opt/fold_spec_constant_op_and_composite_pass.cpp +++ b/source/opt/fold_spec_constant_op_and_composite_pass.cpp @@ -251,7 +251,8 @@ FoldSpecConstantOpAndCompositePass::FoldSpecConstantOpAndCompositePass( type_mgr_(nullptr), id_to_const_val_() {} -bool FoldSpecConstantOpAndCompositePass::ProcessImpl(ir::Module* module) { +Pass::Status FoldSpecConstantOpAndCompositePass::ProcessImpl( + ir::Module* module) { bool modified = false; // Traverse through all the constant defining instructions. For Normal // Constants whose values are determined and do not depend on OpUndef @@ -327,7 +328,7 @@ bool FoldSpecConstantOpAndCompositePass::ProcessImpl(ir::Module* module) { break; } } - return modified; + return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange; } bool FoldSpecConstantOpAndCompositePass::ProcessOpSpecConstantOp( diff --git a/source/opt/fold_spec_constant_op_and_composite_pass.h b/source/opt/fold_spec_constant_op_and_composite_pass.h index 8b913a3..aff7af6 100644 --- a/source/opt/fold_spec_constant_op_and_composite_pass.h +++ b/source/opt/fold_spec_constant_op_and_composite_pass.h @@ -53,7 +53,7 @@ class FoldSpecConstantOpAndCompositePass : public Pass { explicit FoldSpecConstantOpAndCompositePass(const MessageConsumer& c); const char* name() const override { return "fold-spec-const-op-composite"; } - bool Process(ir::Module* module) override { + Status Process(ir::Module* module) override { Initialize(module); return ProcessImpl(module); }; @@ -74,8 +74,8 @@ class FoldSpecConstantOpAndCompositePass : public Pass { // section of the given module, finds the Spec Constants defined with // OpSpecConstantOp and OpSpecConstantComposite instructions. If the result // value of those spec constants can be folded, fold them to their - // corresponding normal constants. Returns true if the module was modified. - bool ProcessImpl(ir::Module*); + // corresponding normal constants. + Status ProcessImpl(ir::Module*); // Processes the OpSpecConstantOp instruction pointed by the given // instruction iterator, folds it to normal constants if possible. Returns diff --git a/source/opt/freeze_spec_constant_value_pass.cpp b/source/opt/freeze_spec_constant_value_pass.cpp index 6d395da..8855c41 100644 --- a/source/opt/freeze_spec_constant_value_pass.cpp +++ b/source/opt/freeze_spec_constant_value_pass.cpp @@ -17,7 +17,7 @@ namespace spvtools { namespace opt { -bool FreezeSpecConstantValuePass::Process(ir::Module* module) { +Pass::Status FreezeSpecConstantValuePass::Process(ir::Module* module) { bool modified = false; module->ForEachInst([&modified](ir::Instruction* inst) { switch (inst->opcode()) { @@ -44,7 +44,7 @@ bool FreezeSpecConstantValuePass::Process(ir::Module* module) { break; } }); - return modified; + return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange; } } // namespace opt diff --git a/source/opt/freeze_spec_constant_value_pass.h b/source/opt/freeze_spec_constant_value_pass.h index 8b21463..7dd31b4 100644 --- a/source/opt/freeze_spec_constant_value_pass.h +++ b/source/opt/freeze_spec_constant_value_pass.h @@ -34,7 +34,7 @@ class FreezeSpecConstantValuePass : public Pass { explicit FreezeSpecConstantValuePass(const MessageConsumer& c) : Pass(c) {} const char* name() const override { return "freeze-spec-const"; } - bool Process(ir::Module*) override; + Status Process(ir::Module*) override; }; } // namespace opt diff --git a/source/opt/null_pass.h b/source/opt/null_pass.h index 7e10791..5b65771 100644 --- a/source/opt/null_pass.h +++ b/source/opt/null_pass.h @@ -27,7 +27,7 @@ class NullPass : public Pass { explicit NullPass(const MessageConsumer& c) : Pass(c) {} const char* name() const override { return "null"; } - bool Process(ir::Module*) override { return false; } + Status Process(ir::Module*) override { return Status::SuccessWithoutChange; } }; } // namespace opt diff --git a/source/opt/pass.h b/source/opt/pass.h index dadb53a..cedb7c8 100644 --- a/source/opt/pass.h +++ b/source/opt/pass.h @@ -27,6 +27,16 @@ namespace opt { // and all analysis and transformation is done via the Process() method. class Pass { public: + // The status of processing a module using a pass. + // + // The numbers for the cases are assigned to make sure that Failure & anything + // is Failure, SuccessWithChange & any success is SuccessWithChange. + enum class Status { + Failure = 0x00, + SuccessWithChange = 0x10, + SuccessWithoutChange = 0x11, + }; + // Constructs a new pass with the given message consumer, which will be // invoked every time there is a message to be communicated to the outside. // @@ -39,9 +49,10 @@ class Pass { // Returns the reference to the message consumer for this pass. const MessageConsumer& consumer() const { return consumer_; } - // Processes the given |module| and returns true if the given |module| is - // modified for optimization. - virtual bool Process(ir::Module* module) = 0; + // Processes the given |module|. Returns Status::Failure if errors occur when + // processing. Returns the corresponding Status::Success if processing is + // succesful to indicate whether changes are made to the module. + virtual Status Process(ir::Module* module) = 0; private: const MessageConsumer& consumer_; // Message consumer. diff --git a/source/opt/pass_manager.cpp b/source/opt/pass_manager.cpp new file mode 100644 index 0000000..18267db --- /dev/null +++ b/source/opt/pass_manager.cpp @@ -0,0 +1,35 @@ +// Copyright (c) 2016 Google Inc. +// +// 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 "pass_manager.h" + +namespace spvtools { +namespace opt { + +Pass::Status PassManager::Run(ir::Module* module) { + auto status = Pass::Status::SuccessWithoutChange; + for (const auto& pass : passes_) { + const auto one_status = pass->Process(module); + if (one_status == Pass::Status::Failure) return one_status; + if (one_status == Pass::Status::SuccessWithChange) status = one_status; + } + // Set the Id bound in the header in case a pass forgot to do so. + if (status == Pass::Status::SuccessWithChange) { + module->SetIdBound(module->ComputeIdBound()); + } + return status; +} + +} // namespace opt +} // namespace spvtools diff --git a/source/opt/pass_manager.h b/source/opt/pass_manager.h index 58c0537..61dc4b5 100644 --- a/source/opt/pass_manager.h +++ b/source/opt/pass_manager.h @@ -21,7 +21,7 @@ #include "log.h" #include "message.h" #include "module.h" -#include "passes.h" +#include "pass.h" namespace spvtools { namespace opt { @@ -35,37 +35,27 @@ class PassManager { explicit PassManager(MessageConsumer c) : consumer_(std::move(c)) {} // Adds an externally constructed pass. - void AddPass(std::unique_ptr pass) { - passes_.push_back(std::move(pass)); - } + void AddPass(std::unique_ptr pass); // Uses the argument |args| to construct a pass instance of type |T|, and adds // the pass instance to this pass manger. The pass added will use this pass // manager's message consumer. template - void AddPass(Args&&... args) { - passes_.emplace_back(new T(consumer_, std::forward(args)...)); - } + void AddPass(Args&&... args); // Returns the number of passes added. - uint32_t NumPasses() const { return static_cast(passes_.size()); } + uint32_t NumPasses() const; // Returns a pointer to the |index|th pass added. - Pass* GetPass(uint32_t index) const { - SPIRV_ASSERT(consumer_, index < passes_.size(), "index out of bound"); - return passes_[index].get(); - } + inline Pass* GetPass(uint32_t index) const; // Returns the message consumer. - const MessageConsumer& consumer() const { return consumer_; } - - // Runs all passes on the given |module|. - void Run(ir::Module* module) { - bool modified = false; - for (const auto& pass : passes_) { - modified |= pass->Process(module); - } - // Set the Id bound in the header in case a pass forgot to do so. - if (modified) module->SetIdBound(module->ComputeIdBound()); - } + inline const MessageConsumer& consumer() const; + + // Runs all passes on the given |module|. Returns Status::Failure if errors + // occur when processing using one of the registered passes. All passes + // registered after the error-reporting pass will be skipped. Returns the + // corresponding Status::Success if processing is succesful to indicate + // whether changes are made to the module. + Pass::Status Run(ir::Module* module); private: // Consumer for messages. @@ -74,6 +64,28 @@ class PassManager { std::vector> passes_; }; +inline void PassManager::AddPass(std::unique_ptr pass) { + passes_.push_back(std::move(pass)); +} + +template +inline void PassManager::AddPass(Args&&... args) { + passes_.emplace_back(new T(consumer_, std::forward(args)...)); +} + +inline uint32_t PassManager::NumPasses() const { + return static_cast(passes_.size()); +} + +inline Pass* PassManager::GetPass(uint32_t index) const { + SPIRV_ASSERT(consumer_, index < passes_.size(), "index out of bound"); + return passes_[index].get(); +} + +inline const MessageConsumer& PassManager::consumer() const { + return consumer_; +} + } // namespace opt } // namespace spvtools diff --git a/source/opt/set_spec_constant_default_value_pass.cpp b/source/opt/set_spec_constant_default_value_pass.cpp index 68db8bf..2e9f36f 100644 --- a/source/opt/set_spec_constant_default_value_pass.cpp +++ b/source/opt/set_spec_constant_default_value_pass.cpp @@ -150,7 +150,7 @@ ir::Instruction* GetSpecIdTargetFromDecorationGroup( } }; -bool SetSpecConstantDefaultValuePass::Process(ir::Module* module) { +Pass::Status SetSpecConstantDefaultValuePass::Process(ir::Module* module) { // The operand index of decoration target in an OpDecorate instruction. const uint32_t kTargetIdOperandIndex = 0; // The operand index of the decoration literal in an OpDecorate instruction. @@ -249,7 +249,7 @@ bool SetSpecConstantDefaultValuePass::Process(ir::Module* module) { // No need to update the DefUse manager, as this pass does not change any // ids. } - return modified; + return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange; } // Returns true if the given char is ':', '\0' or considered as blank space diff --git a/source/opt/set_spec_constant_default_value_pass.h b/source/opt/set_spec_constant_default_value_pass.h index 7762050..e1c6012 100644 --- a/source/opt/set_spec_constant_default_value_pass.h +++ b/source/opt/set_spec_constant_default_value_pass.h @@ -42,7 +42,7 @@ class SetSpecConstantDefaultValuePass : public Pass { : Pass(c), spec_id_to_value_(std::move(default_values)) {} const char* name() const override { return "set-spec-const-default-value"; } - bool Process(ir::Module*) override; + Status Process(ir::Module*) override; // Parses the given null-terminated C string to get a mapping from Spec Id to // default value strings. Returns a unique pointer of the mapping from spec diff --git a/source/opt/strip_debug_info_pass.cpp b/source/opt/strip_debug_info_pass.cpp index 4a15ec1..45dd344 100644 --- a/source/opt/strip_debug_info_pass.cpp +++ b/source/opt/strip_debug_info_pass.cpp @@ -17,7 +17,7 @@ namespace spvtools { namespace opt { -bool StripDebugInfoPass::Process(ir::Module* module) { +Pass::Status StripDebugInfoPass::Process(ir::Module* module) { bool modified = !module->debugs().empty(); module->debug_clear(); @@ -26,7 +26,7 @@ bool StripDebugInfoPass::Process(ir::Module* module) { inst->dbg_line_insts().clear(); }); - return modified; + return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange; } } // namespace opt diff --git a/source/opt/strip_debug_info_pass.h b/source/opt/strip_debug_info_pass.h index 9a8d27c..c2bb1fd 100644 --- a/source/opt/strip_debug_info_pass.h +++ b/source/opt/strip_debug_info_pass.h @@ -28,7 +28,7 @@ class StripDebugInfoPass : public Pass { explicit StripDebugInfoPass(const MessageConsumer& c) : Pass(c) {} const char* name() const override { return "strip-debug"; } - bool Process(ir::Module* module) override; + Status Process(ir::Module* module) override; }; } // namespace opt diff --git a/source/opt/unify_const_pass.cpp b/source/opt/unify_const_pass.cpp index 72fb3cd..1e4da3d 100644 --- a/source/opt/unify_const_pass.cpp +++ b/source/opt/unify_const_pass.cpp @@ -102,7 +102,7 @@ class ResultIdTrie { }; } // anonymous namespace -bool UnifyConstantPass::Process(ir::Module* module) { +Pass::Status UnifyConstantPass::Process(ir::Module* module) { bool modified = false; ResultIdTrie defined_constants; analysis::DefUseManager def_use_mgr(consumer(), module); @@ -164,7 +164,7 @@ bool UnifyConstantPass::Process(ir::Module* module) { break; } } - return modified; + return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange; } } // opt diff --git a/source/opt/unify_const_pass.h b/source/opt/unify_const_pass.h index db17b69..0edb10d 100644 --- a/source/opt/unify_const_pass.h +++ b/source/opt/unify_const_pass.h @@ -39,7 +39,7 @@ class UnifyConstantPass : public Pass { explicit UnifyConstantPass(const MessageConsumer& c) : Pass(c) {} const char* name() const override { return "unify-const"; } - bool Process(ir::Module*) override; + Status Process(ir::Module*) override; }; } // namespace opt diff --git a/test/opt/pass_fixture.h b/test/opt/pass_fixture.h index 9cc831c..9ce5fc9 100644 --- a/test/opt/pass_fixture.h +++ b/test/opt/pass_fixture.h @@ -48,17 +48,17 @@ class PassTest : public TestT { // Runs the given |pass| on the binary assembled from the |assembly|, and // disassebles the optimized binary. Returns a tuple of disassembly string // and the boolean value returned from pass Process() function. - std::tuple OptimizeAndDisassemble( + std::tuple OptimizeAndDisassemble( opt::Pass* pass, const std::string& original, bool skip_nop) { std::unique_ptr module = BuildModule(SPV_ENV_UNIVERSAL_1_1, consumer_, original); EXPECT_NE(nullptr, module) << "Assembling failed for shader:\n" << original << std::endl; if (!module) { - return std::make_tuple(std::string(), false); + return std::make_tuple(std::string(), opt::Pass::Status::Failure); } - const bool modified = pass->Process(module.get()); + const auto status = pass->Process(module.get()); std::vector binary; module->ToBinary(&binary, skip_nop); @@ -66,14 +66,14 @@ class PassTest : public TestT { EXPECT_TRUE(tools_.Disassemble(binary, &optimized)) << "Disassembling failed for shader:\n" << original << std::endl; - return std::make_tuple(optimized, modified); + return std::make_tuple(optimized, status); } // 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 - std::tuple SinglePassRunAndDisassemble( + std::tuple SinglePassRunAndDisassemble( const std::string& assembly, bool skip_nop, Args&&... args) { auto pass = MakeUnique(consumer_, std::forward(args)...); return OptimizeAndDisassemble(pass.get(), assembly, skip_nop); @@ -88,11 +88,13 @@ class PassTest : public TestT { const std::string& expected, bool skip_nop, Args&&... args) { std::string optimized; - bool modified = false; - std::tie(optimized, modified) = SinglePassRunAndDisassemble( + auto status = opt::Pass::Status::SuccessWithoutChange; + std::tie(optimized, status) = SinglePassRunAndDisassemble( original, skip_nop, std::forward(args)...); // Check whether the pass returns the correct modification indication. - EXPECT_EQ(original != expected, modified); + EXPECT_NE(opt::Pass::Status::Failure, status); + EXPECT_EQ(original == expected, + status == opt::Pass::Status::SuccessWithoutChange); EXPECT_EQ(expected, optimized); } diff --git a/test/opt/test_fold_spec_const_op_composite.cpp b/test/opt/test_fold_spec_const_op_composite.cpp index 051c913..b84bf26 100644 --- a/test/opt/test_fold_spec_const_op_composite.cpp +++ b/test/opt/test_fold_spec_const_op_composite.cpp @@ -206,15 +206,16 @@ TEST_P(FoldSpecConstantOpAndCompositePassTest, ParamTestCase) { // Run the optimization and get the result code in disassembly. std::string optimized; - bool modified = false; - std::tie(optimized, modified) = + auto status = opt::Pass::Status::SuccessWithoutChange; + std::tie(optimized, status) = SinglePassRunAndDisassemble( original, /* skip_nop = */ true); // Check the optimized code, but ignore the OpName instructions. + EXPECT_NE(opt::Pass::Status::Failure, status); EXPECT_EQ( - StripOpNameInstructions(expected) != StripOpNameInstructions(original), - modified); + StripOpNameInstructions(expected) == StripOpNameInstructions(original), + status == opt::Pass::Status::SuccessWithoutChange); EXPECT_EQ(StripOpNameInstructions(expected), StripOpNameInstructions(optimized)); } diff --git a/test/opt/test_line_debug_info.cpp b/test/opt/test_line_debug_info.cpp index 3e86ed0..a4af497 100644 --- a/test/opt/test_line_debug_info.cpp +++ b/test/opt/test_line_debug_info.cpp @@ -25,10 +25,15 @@ class NopifyPass : public opt::Pass { explicit NopifyPass(const MessageConsumer& c) : opt::Pass(c) {} const char* name() const override { return "NopifyPass"; } - bool Process(ir::Module* module) override { - module->ForEachInst([](ir::Instruction* inst) { inst->ToNop(); }, - /* run_on_debug_line_insts = */ false); - return true; + Status Process(ir::Module* module) override { + bool modified = false; + module->ForEachInst( + [&modified](ir::Instruction* inst) { + inst->ToNop(); + modified = true; + }, + /* run_on_debug_line_insts = */ false); + return modified ? Status::SuccessWithChange : Status::SuccessWithoutChange; } }; diff --git a/test/opt/test_pass_manager.cpp b/test/opt/test_pass_manager.cpp index b365ad2..71ca3ac 100644 --- a/test/opt/test_pass_manager.cpp +++ b/test/opt/test_pass_manager.cpp @@ -78,10 +78,9 @@ class AppendOpNopPass : public opt::Pass { explicit AppendOpNopPass(const MessageConsumer& c) : opt::Pass(c) {} const char* name() const override { return "AppendOpNop"; } - bool Process(ir::Module* module) override { - auto inst = MakeUnique(); - module->AddDebugInst(std::move(inst)); - return true; + Status Process(ir::Module* module) override { + module->AddDebugInst(MakeUnique()); + return Status::SuccessWithChange; } }; @@ -93,12 +92,11 @@ class AppendMultipleOpNopPass : public opt::Pass { : opt::Pass(c), num_nop_(num_nop) {} const char* name() const override { return "AppendOpNop"; } - bool Process(ir::Module* module) override { + Status Process(ir::Module* module) override { for (uint32_t i = 0; i < num_nop_; i++) { - auto inst = MakeUnique(); - module->AddDebugInst(std::move(inst)); + module->AddDebugInst(MakeUnique()); } - return true; + return Status::SuccessWithChange; } private: @@ -111,10 +109,10 @@ class DuplicateInstPass : public opt::Pass { explicit DuplicateInstPass(const MessageConsumer& c) : opt::Pass(c) {} const char* name() const override { return "DuplicateInst"; } - bool Process(ir::Module* module) override { + Status Process(ir::Module* module) override { auto inst = MakeUnique(*(--module->debug_end())); module->AddDebugInst(std::move(inst)); - return true; + return Status::SuccessWithChange; } }; @@ -149,11 +147,11 @@ class AppendTypeVoidInstPass : public opt::Pass { : opt::Pass(c), result_id_(result_id) {} const char* name() const override { return "AppendTypeVoidInstPass"; } - bool Process(ir::Module* module) override { + Status Process(ir::Module* module) override { auto inst = MakeUnique(SpvOpTypeVoid, 0, result_id_, std::vector{}); module->AddType(std::move(inst)); - return true; + return Status::SuccessWithChange; } private: diff --git a/test/opt/test_unify_const.cpp b/test/opt/test_unify_const.cpp index 3a32372..5813201 100644 --- a/test/opt/test_unify_const.cpp +++ b/test/opt/test_unify_const.cpp @@ -114,8 +114,8 @@ class UnifyConstantTest : public PassTest { // optimized code std::string optimized_before_strip; - bool modified = false; - std::tie(optimized_before_strip, modified) = + auto status = opt::Pass::Status::SuccessWithoutChange; + std::tie(optimized_before_strip, status) = this->template SinglePassRunAndDisassemble( test_builder.GetCode(), /* skip_nop = */ true); @@ -124,8 +124,10 @@ class UnifyConstantTest : public PassTest { std::tie(optimized_without_opnames, optimized_opnames) = StripOpNameInstructionsToSet(optimized_before_strip); - // Flag "modified" should be returned correctly. - EXPECT_EQ(expected_without_opnames != original_without_opnames, modified); + // Flag "status" should be returned correctly. + EXPECT_NE(opt::Pass::Status::Failure, status); + EXPECT_EQ(expected_without_opnames == original_without_opnames, + status == opt::Pass::Status::SuccessWithoutChange); // Code except OpName instructions should be exactly the same. EXPECT_EQ(expected_without_opnames, optimized_without_opnames); // OpName instructions can be in different order, but the content must be @@ -208,11 +210,11 @@ TEST_F(UnifyFrontEndConstantSingleTest, SkipWhenResultIdHasDecorations) { }); expected_builder - .AppendAnnotations({ + .AppendAnnotations({ "OpDecorate %f_1 RelaxedPrecision", "OpDecorate %f_2_dup RelaxedPrecision", - }) - .AppendTypesConstantsGlobals({ + }) + .AppendTypesConstantsGlobals({ // clang-format off "%float = OpTypeFloat 32", "%_pf_float = OpTypePointer Function %float", @@ -223,7 +225,7 @@ TEST_F(UnifyFrontEndConstantSingleTest, SkipWhenResultIdHasDecorations) { "%f_3 = OpConstant %float 3", // clang-format on }) - .AppendInMain({ + .AppendInMain({ // clang-format off "%f_var = OpVariable %_pf_float Function", "OpStore %f_var %f_1_dup", @@ -231,8 +233,8 @@ TEST_F(UnifyFrontEndConstantSingleTest, SkipWhenResultIdHasDecorations) { "OpStore %f_var %f_3", // clang-format on }) - .AppendNames({ - "OpName %f_3 \"f_3_dup\"", + .AppendNames({ + "OpName %f_3 \"f_3_dup\"", }); Check(expected_builder, test_builder); @@ -300,8 +302,7 @@ TEST_F(UnifyFrontEndConstantSingleTest, UnifyWithDecorationOnTypes) { "OpStore %flat_d_var %flat_d_1", }) .AppendNames({ - "OpName %flat_1 \"flat_1_dup\"", - "OpName %flat_d_1 \"flat_d_1_dup\"", + "OpName %flat_1 \"flat_1_dup\"", "OpName %flat_d_1 \"flat_d_1_dup\"", }); Check(expected_builder, test_builder); @@ -348,7 +349,7 @@ TEST_P(UnifyFrontEndConstantParamTest, TestCase) { INSTANTIATE_TEST_CASE_P(Case, UnifyFrontEndConstantParamTest, ::testing::ValuesIn(std::vector({ - // clang-format off + // clang-format off // basic tests for scalar constants { // preserved constants @@ -974,7 +975,7 @@ INSTANTIATE_TEST_CASE_P(Case, UnifyFrontEndConstantParamTest, "OpName %signed_22 \"signed_22_dup\"", }, }, - // clang-format on - }))); + // clang-format on + }))); } // anonymous namespace diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index d15b27c..5f51669 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -20,6 +20,7 @@ #include "source/opt/build_module.h" #include "source/opt/ir_loader.h" #include "source/opt/pass_manager.h" +#include "source/opt/passes.h" #include "tools/io.h" using namespace spvtools; -- 2.7.4