Change interface of Pass::Process() to return possible failures.

This commit is contained in:
Lei Zhang 2016-09-12 12:39:44 -04:00
parent 12b5777912
commit 2cbb2cce3e
23 changed files with 158 additions and 89 deletions

View File

@ -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

View File

@ -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<ir::Instruction*> 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

View File

@ -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

View File

@ -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(

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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.

View File

@ -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

View File

@ -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> pass) {
passes_.push_back(std::move(pass));
}
void AddPass(std::unique_ptr<Pass> 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 <typename T, typename... Args>
void AddPass(Args&&... args) {
passes_.emplace_back(new T(consumer_, std::forward<Args>(args)...));
}
void AddPass(Args&&... args);
// Returns the number of passes added.
uint32_t NumPasses() const { return static_cast<uint32_t>(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_; }
inline const MessageConsumer& consumer() const;
// 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());
}
// 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<std::unique_ptr<Pass>> passes_;
};
inline void PassManager::AddPass(std::unique_ptr<Pass> pass) {
passes_.push_back(std::move(pass));
}
template <typename T, typename... Args>
inline void PassManager::AddPass(Args&&... args) {
passes_.emplace_back(new T(consumer_, std::forward<Args>(args)...));
}
inline uint32_t PassManager::NumPasses() const {
return static_cast<uint32_t>(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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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<std::string, bool> OptimizeAndDisassemble(
std::tuple<std::string, opt::Pass::Status> OptimizeAndDisassemble(
opt::Pass* pass, const std::string& original, bool skip_nop) {
std::unique_ptr<ir::Module> 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<uint32_t> 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 <typename PassT, typename... Args>
std::tuple<std::string, bool> SinglePassRunAndDisassemble(
std::tuple<std::string, opt::Pass::Status> SinglePassRunAndDisassemble(
const std::string& assembly, bool skip_nop, Args&&... args) {
auto pass = MakeUnique<PassT>(consumer_, std::forward<Args>(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<PassT>(
auto status = opt::Pass::Status::SuccessWithoutChange;
std::tie(optimized, status) = SinglePassRunAndDisassemble<PassT>(
original, skip_nop, std::forward<Args>(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);
}

View File

@ -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<opt::FoldSpecConstantOpAndCompositePass>(
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));
}

View File

@ -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;
}
};

View File

@ -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<ir::Instruction>();
module->AddDebugInst(std::move(inst));
return true;
Status Process(ir::Module* module) override {
module->AddDebugInst(MakeUnique<ir::Instruction>());
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<ir::Instruction>();
module->AddDebugInst(std::move(inst));
module->AddDebugInst(MakeUnique<ir::Instruction>());
}
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<ir::Instruction>(*(--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<ir::Instruction>(SpvOpTypeVoid, 0, result_id_,
std::vector<ir::Operand>{});
module->AddType(std::move(inst));
return true;
return Status::SuccessWithChange;
}
private:

View File

@ -114,8 +114,8 @@ class UnifyConstantTest : public PassTest<T> {
// 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<opt::UnifyConstantPass>(
test_builder.GetCode(),
/* skip_nop = */ true);
@ -124,8 +124,10 @@ class UnifyConstantTest : public PassTest<T> {
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<UnifyConstantTestCase>({
// 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

View File

@ -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;