From: David Neto Date: Sat, 20 Aug 2016 13:47:00 +0000 (-0400) Subject: ForEachInst optionally runs on attached debug line insts X-Git-Tag: upstream/2018.6~1101 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=97fc6aa3b88e63d9bde76f92cdf06e723f337d92;p=platform%2Fupstream%2FSPIRV-Tools.git ForEachInst optionally runs on attached debug line insts Also: - Add const forms of ForEachInst - Rewrite Module::ToBinary in terms of ForEachInst - Add Instruction::ToBinaryWithoutAttachedDebugInsts - Delete the ToBinary method on Function, BasicBlock, and Instruction since it can now be implemented with ForEachInst in a less confusing way, e.g. without recursion. - Preserve debug line instructions on OpFunctionEnd (and store that instruction as a unique-pointer, for regularity). --- diff --git a/source/opt/basic_block.h b/source/opt/basic_block.h index 62b50ce..52f9707 100644 --- a/source/opt/basic_block.h +++ b/source/opt/basic_block.h @@ -62,19 +62,19 @@ class BasicBlock { const_iterator cbegin() { return const_iterator(&insts_, insts_.cbegin()); } const_iterator cend() { return const_iterator(&insts_, insts_.cend()); } - // Runs the given function |f| on each instruction in this basic block. - inline void ForEachInst(const std::function& f); - - // Pushes the binary segments for this instruction into the back of *|binary|. - // If |skip_nop| is true and this is a OpNop, do nothing. - inline void ToBinary(std::vector* binary, bool skip_nop) const; + // Runs the given function |f| on each instruction in this basic block, and + // optionally on the debug line instructions that might precede them. + inline void ForEachInst(const std::function& f, + bool run_on_debug_line_insts = false); + inline void ForEachInst(const std::function& f, + bool run_on_debug_line_insts = false) const; private: // The enclosing function. Function* function_; // The label starting this basic block. std::unique_ptr label_; - // Instructions inside this basic block. + // Instructions inside this basic block, but not the OpLabel. std::vector> insts_; }; @@ -85,16 +85,20 @@ inline void BasicBlock::AddInstruction(std::unique_ptr i) { insts_.emplace_back(std::move(i)); } -inline void BasicBlock::ForEachInst( - const std::function& f) { - label_->ForEachInst(f); - for (auto& inst : insts_) f(inst.get()); +inline void BasicBlock::ForEachInst(const std::function& f, + bool run_on_debug_line_insts) { + label_->ForEachInst(f, run_on_debug_line_insts); + for (auto& inst : insts_) inst->ForEachInst(f, run_on_debug_line_insts); } -inline void BasicBlock::ToBinary(std::vector* binary, - bool skip_nop) const { - label_->ToBinary(binary, skip_nop); - for (const auto& inst : insts_) inst->ToBinary(binary, skip_nop); +inline void BasicBlock::ForEachInst( + const std::function& f, + bool run_on_debug_line_insts) const { + static_cast(label_.get()) + ->ForEachInst(f, run_on_debug_line_insts); + for (const auto& inst : insts_) + static_cast(inst.get()) + ->ForEachInst(f, run_on_debug_line_insts); } } // namespace ir diff --git a/source/opt/function.cpp b/source/opt/function.cpp index 335ccbc..4327258 100644 --- a/source/opt/function.cpp +++ b/source/opt/function.cpp @@ -29,18 +29,29 @@ namespace spvtools { namespace ir { -void Function::ForEachInst(const std::function& f) { - def_inst_->ForEachInst(f); - for (auto& param : params_) param->ForEachInst(f); - for (auto& bb : blocks_) bb->ForEachInst(f); - end_inst_.ForEachInst(f); +void Function::ForEachInst(const std::function& f, + bool run_on_debug_line_insts) { + def_inst_->ForEachInst(f, run_on_debug_line_insts); + for (auto& param : params_) param->ForEachInst(f, run_on_debug_line_insts); + for (auto& bb : blocks_) bb->ForEachInst(f, run_on_debug_line_insts); + end_inst_->ForEachInst(f, run_on_debug_line_insts); } -void Function::ToBinary(std::vector* binary, bool skip_nop) const { - def_inst_->ToBinary(binary, skip_nop); - for (const auto& param : params_) param->ToBinary(binary, skip_nop); - for (const auto& bb : blocks_) bb->ToBinary(binary, skip_nop); - end_inst_.ToBinary(binary, skip_nop); +void Function::ForEachInst(const std::function& f, + bool run_on_debug_line_insts) const { + static_cast(def_inst_.get()) + ->ForEachInst(f, run_on_debug_line_insts); + + for (const auto& param : params_) + static_cast(param.get()) + ->ForEachInst(f, run_on_debug_line_insts); + + for (const auto& bb : blocks_) + static_cast(bb.get())->ForEachInst( + f, run_on_debug_line_insts); + + static_cast(end_inst_.get()) + ->ForEachInst(f, run_on_debug_line_insts); } } // namespace ir diff --git a/source/opt/function.h b/source/opt/function.h index 8615a36..f87b838 100644 --- a/source/opt/function.h +++ b/source/opt/function.h @@ -58,17 +58,20 @@ class Function { // Appends a basic block to this function. inline void AddBasicBlock(std::unique_ptr b); + // Saves the given function end instruction. + inline void SetFunctionEnd(std::unique_ptr end_inst); + iterator begin() { return iterator(&blocks_, blocks_.begin()); } iterator end() { return iterator(&blocks_, blocks_.end()); } const_iterator cbegin() { return const_iterator(&blocks_, blocks_.cbegin()); } const_iterator cend() { return const_iterator(&blocks_, blocks_.cend()); } - // Runs the given function |f| on each instruction in this basic block. - void ForEachInst(const std::function& f); - - // Pushes the binary segments for this instruction into the back of *|binary|. - // If |skip_nop| is true and this is a OpNop, do nothing. - void ToBinary(std::vector* binary, bool skip_nop) const; + // Runs the given function |f| on each instruction in this function, and + // optionally on debug line instructions that might precede them. + void ForEachInst(const std::function& f, + bool run_on_debug_line_insts = false); + void ForEachInst(const std::function& f, + bool run_on_debug_line_insts = false) const; private: // The enclosing module. @@ -80,13 +83,11 @@ class Function { // All basic blocks inside this function. std::vector> blocks_; // The OpFunctionEnd instruction. - Instruction end_inst_; + std::unique_ptr end_inst_; }; inline Function::Function(std::unique_ptr def_inst) - : module_(nullptr), - def_inst_(std::move(def_inst)), - end_inst_(SpvOpFunctionEnd) {} + : module_(nullptr), def_inst_(std::move(def_inst)), end_inst_() {} inline void Function::AddParameter(std::unique_ptr p) { params_.emplace_back(std::move(p)); @@ -96,6 +97,10 @@ inline void Function::AddBasicBlock(std::unique_ptr b) { blocks_.emplace_back(std::move(b)); } +inline void Function::SetFunctionEnd(std::unique_ptr end_inst) { + end_inst_ = std::move(end_inst); +} + } // namespace ir } // namespace spvtools diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 03031b4..e71c242 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -94,13 +94,8 @@ uint32_t Instruction::NumInOperandWords() const { return size; } -void Instruction::ToBinary(std::vector* binary, bool skip_nop) const { - for (const auto& dbg_line : dbg_line_insts_) { - dbg_line.ToBinary(binary, skip_nop); - } - - if (skip_nop && IsNop()) return; - +void Instruction::ToBinaryWithoutAttachedDebugInsts( + std::vector* binary) const { const uint32_t num_words = 1 + NumOperandWords(); binary->push_back((num_words << 16) | static_cast(opcode_)); for (const auto& operand : operands_) diff --git a/source/opt/instruction.h b/source/opt/instruction.h index bf7d13b..44df0fa 100644 --- a/source/opt/instruction.h +++ b/source/opt/instruction.h @@ -154,13 +154,16 @@ class Instruction { // line-related debug instructions. inline void ToNop(); - // Runs the given function |f| on this instruction and preceding debug - // instructions. - inline void ForEachInst(const std::function& f); + // Runs the given function |f| on this instruction and optionally on the + // preceding debug line instructions. The function will always be run + // if this is itself a debug line instruction. + inline void ForEachInst(const std::function& f, + bool run_on_debug_line_insts = false); + inline void ForEachInst(const std::function& f, + bool run_on_debug_line_insts = false) const; // Pushes the binary segments for this instruction into the back of *|binary|. - // If |skip_nop| is true and this is a OpNop, do nothing. - void ToBinary(std::vector* binary, bool skip_nop) const; + void ToBinaryWithoutAttachedDebugInsts(std::vector* binary) const; private: // Returns the toal count of result type id and result id. @@ -210,9 +213,18 @@ inline void Instruction::ToNop() { operands_.clear(); } +inline void Instruction::ForEachInst(const std::function& f, + bool run_on_debug_line_insts) { + if (run_on_debug_line_insts) + for (auto& dbg_line : dbg_line_insts_) f(&dbg_line); + f(this); +} + inline void Instruction::ForEachInst( - const std::function& f) { - for (auto& dbg_line : dbg_line_insts_) f(&dbg_line); + const std::function& f, + bool run_on_debug_line_insts) const { + if (run_on_debug_line_insts) + for (auto& dbg_line : dbg_line_insts_) f(&dbg_line); f(this); } diff --git a/source/opt/ir_loader.cpp b/source/opt/ir_loader.cpp index 01ffce4..47aa82d 100644 --- a/source/opt/ir_loader.cpp +++ b/source/opt/ir_loader.cpp @@ -52,6 +52,7 @@ void IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) { } else if (opcode == SpvOpFunctionEnd) { assert(function_ != nullptr); assert(block_ == nullptr); + function_->SetFunctionEnd(std::move(spv_inst)); module_->AddFunction(std::move(function_)); function_ = nullptr; } else if (opcode == SpvOpLabel) { diff --git a/source/opt/module.cpp b/source/opt/module.cpp index c08b2fd..16cb16e 100644 --- a/source/opt/module.cpp +++ b/source/opt/module.cpp @@ -66,17 +66,41 @@ std::vector Module::GetConstants() const { return insts; }; -void Module::ForEachInst(const std::function& f) { - for (auto& i : capabilities_) f(i.get()); - for (auto& i : extensions_) f(i.get()); - for (auto& i : ext_inst_imports_) f(i.get()); - if (memory_model_) f(memory_model_.get()); - for (auto& i : entry_points_) f(i.get()); - for (auto& i : execution_modes_) f(i.get()); - for (auto& i : debugs_) f(i.get()); - for (auto& i : annotations_) f(i.get()); - for (auto& i : types_values_) f(i.get()); - for (auto& i : functions_) i->ForEachInst(f); +void Module::ForEachInst(const std::function& f, + bool run_on_debug_line_insts) { +#define DELEGATE(i) i->ForEachInst(f, run_on_debug_line_insts) + for (auto& i : capabilities_) DELEGATE(i); + for (auto& i : extensions_) DELEGATE(i); + for (auto& i : ext_inst_imports_) DELEGATE(i); + if (memory_model_) DELEGATE(memory_model_); + for (auto& i : entry_points_) DELEGATE(i); + for (auto& i : execution_modes_) DELEGATE(i); + for (auto& i : debugs_) DELEGATE(i); + for (auto& i : annotations_) DELEGATE(i); + for (auto& i : types_values_) DELEGATE(i); + for (auto& i : functions_) DELEGATE(i); +#undef DELEGATE +} + +void Module::ForEachInst(const std::function& f, + bool run_on_debug_line_insts) const { +#define DELEGATE(i) \ + static_cast(i.get())->ForEachInst( \ + f, run_on_debug_line_insts) + for (auto& i : capabilities_) DELEGATE(i); + for (auto& i : extensions_) DELEGATE(i); + for (auto& i : ext_inst_imports_) DELEGATE(i); + if (memory_model_) DELEGATE(memory_model_); + for (auto& i : entry_points_) DELEGATE(i); + for (auto& i : execution_modes_) DELEGATE(i); + for (auto& i : debugs_) DELEGATE(i); + for (auto& i : annotations_) DELEGATE(i); + for (auto& i : types_values_) DELEGATE(i); + for (auto& i : functions_) { + static_cast(i.get())->ForEachInst(f, + run_on_debug_line_insts); + } +#undef DELEGATE } void Module::ToBinary(std::vector* binary, bool skip_nop) const { @@ -87,17 +111,10 @@ void Module::ToBinary(std::vector* binary, bool skip_nop) const { binary->push_back(header_.bound); binary->push_back(header_.reserved); - // TODO(antiagainst): wow, looks like a duplication of the above. - for (const auto& c : capabilities_) c->ToBinary(binary, skip_nop); - for (const auto& e : extensions_) e->ToBinary(binary, skip_nop); - for (const auto& e : ext_inst_imports_) e->ToBinary(binary, skip_nop); - if (memory_model_) memory_model_->ToBinary(binary, skip_nop); - for (const auto& e : entry_points_) e->ToBinary(binary, skip_nop); - for (const auto& e : execution_modes_) e->ToBinary(binary, skip_nop); - for (const auto& d : debugs_) d->ToBinary(binary, skip_nop); - for (const auto& a : annotations_) a->ToBinary(binary, skip_nop); - for (const auto& t : types_values_) t->ToBinary(binary, skip_nop); - for (const auto& f : functions_) f->ToBinary(binary, skip_nop); + auto write_inst = [this, binary, skip_nop](const Instruction* i) { + if (!skip_nop || !i->IsNop()) i->ToBinaryWithoutAttachedDebugInsts(binary); + }; + ForEachInst(write_inst, true); } } // namespace ir diff --git a/source/opt/module.h b/source/opt/module.h index 732a6ce..7c359cf 100644 --- a/source/opt/module.h +++ b/source/opt/module.h @@ -122,8 +122,12 @@ class Module { inline const_iterator cbegin() const; inline const_iterator cend() const; - // Invokes function |f| on all instructions in this module. - void ForEachInst(const std::function& f); + // Invokes function |f| on all instructions in this module, and optionally on + // the debug line instructions that precede them. + void ForEachInst(const std::function& f, + bool run_on_debug_line_insts = false); + void ForEachInst(const std::function& f, + bool run_on_debug_line_insts = false) const; // Pushes the binary segments for this instruction into the back of *|binary|. // If |skip_nop| is true and this is a OpNop, do nothing. diff --git a/test/opt/test_def_use.cpp b/test/opt/test_def_use.cpp index 608a7ca..833a6ed 100644 --- a/test/opt/test_def_use.cpp +++ b/test/opt/test_def_use.cpp @@ -48,7 +48,7 @@ std::string DisassembleInst(ir::Instruction* inst) { std::vector binary; // We need this to generate the necessary header in the binary. tools.Assemble("", &binary); - inst->ToBinary(&binary, /* skip_nop = */ false); + inst->ToBinaryWithoutAttachedDebugInsts(&binary); std::string text; // We'll need to check the underlying id numbers. diff --git a/test/opt/test_ir_loader.cpp b/test/opt/test_ir_loader.cpp index 479cb4b..6066997 100644 --- a/test/opt/test_ir_loader.cpp +++ b/test/opt/test_ir_loader.cpp @@ -36,7 +36,7 @@ using namespace spvtools; void DoRoundTripCheck(const std::string& text) { SpvTools t(SPV_ENV_UNIVERSAL_1_1); std::unique_ptr module = t.BuildModule(text); - ASSERT_NE(nullptr, module); + ASSERT_NE(nullptr, module) << "Failed to assemble\n" << text; std::vector binary; module->ToBinary(&binary, /* skip_nop = */ false); @@ -244,4 +244,55 @@ TEST(IrBuilder, OpUndefInBasicBlock) { // clang-format on } +TEST(IrBuilder, KeepLineDebugInfoBeforeType) { + DoRoundTripCheck( + // clang-format off + "OpCapability Shader\n" + "OpMemoryModel Logical GLSL450\n" + "%1 = OpString \"minimal.vert\"\n" + "OpLine %1 1 1\n" + "OpNoLine\n" + "%void = OpTypeVoid\n" + "OpLine %1 2 2\n" + "%3 = OpTypeFunction %void\n"); + // clang-format on +} + +TEST(IrBuilder, KeepLineDebugInfoBeforeLabel) { + DoRoundTripCheck( + // clang-format off + "OpCapability Shader\n" + "OpMemoryModel Logical GLSL450\n" + "%1 = OpString \"minimal.vert\"\n" + "%void = OpTypeVoid\n" + "%3 = OpTypeFunction %void\n" + "%4 = OpFunction %void None %3\n" + "%5 = OpLabel\n" + "OpBranch %6\n" + "OpLine %1 1 1\n" + "OpLine %1 2 2\n" + "%6 = OpLabel\n" + "OpBranch %7\n" + "OpLine %1 100 100\n" + "%7 = OpLabel\n" + "OpReturn\n" + "OpFunctionEnd\n"); + // clang-format on +} + +TEST(IrBuilder, KeepLineDebugInfoBeforeFunctionEnd) { + DoRoundTripCheck( + // clang-format off + "OpCapability Shader\n" + "OpMemoryModel Logical GLSL450\n" + "%1 = OpString \"minimal.vert\"\n" + "%void = OpTypeVoid\n" + "%3 = OpTypeFunction %void\n" + "%4 = OpFunction %void None %3\n" + "OpLine %1 1 1\n" + "OpLine %1 2 2\n" + "OpFunctionEnd\n"); + // clang-format on +} + } // anonymous namespace