spirv-opt: Add WhileEachInst to Module, InstructionList, and Graph Adds WhileEachInst (const and non-const overloads) to: - InstructionList - Graph - Module Allows iterating through all instructions and terminating early when the callback returns false. Adds unit tests to verify early termination. Also: - Re-implements ForEachInst in terms of WhileEachInst across Module, InstructionList, and Graph to reduce code duplication. - Unifies the section traversal order in Module::WhileEachInstImpl and adds a static_assert to verify matching constness between ModuleT and InstT. - Includes trailing_dbg_line_info_ in both const and non-const iteration.
diff --git a/source/opt/graph.cpp b/source/opt/graph.cpp index bcb5b74..ec00cfe 100644 --- a/source/opt/graph.cpp +++ b/source/opt/graph.cpp
@@ -49,57 +49,107 @@ void Graph::ForEachInst(const std::function<void(Instruction*)>& f, bool run_on_debug_line_insts, bool run_on_non_semantic_insts) { - (void)run_on_debug_line_insts; - - f(def_inst_.get()); - - for (auto& inst : inputs_) { - f(inst.get()); - } - - for (auto& inst : insts_) { - f(inst.get()); - } - - for (auto& inst : outputs_) { - f(inst.get()); - } - - f(end_inst_.get()); - - if (run_on_non_semantic_insts) { - for (auto& inst : non_semantic_) { - f(inst.get()); - } - } + WhileEachInst( + [&f](Instruction* inst) { + f(inst); + return true; + }, + run_on_debug_line_insts, run_on_non_semantic_insts); } void Graph::ForEachInst(const std::function<void(const Instruction*)>& f, bool run_on_debug_line_insts, bool run_on_non_semantic_insts) const { + WhileEachInst( + [&f](const Instruction* inst) { + f(inst); + return true; + }, + run_on_debug_line_insts, run_on_non_semantic_insts); +} + +bool Graph::WhileEachInst(const std::function<bool(Instruction*)>& f, + bool run_on_debug_line_insts, + bool run_on_non_semantic_insts) { (void)run_on_debug_line_insts; - f(def_inst_.get()); + if (!f(def_inst_.get())) { + return false; + } for (auto& inst : inputs_) { - f(inst.get()); + if (!f(inst.get())) { + return false; + } } for (auto& inst : insts_) { - f(inst.get()); + if (!f(inst.get())) { + return false; + } } for (auto& inst : outputs_) { - f(inst.get()); + if (!f(inst.get())) { + return false; + } } - f(end_inst_.get()); + if (!f(end_inst_.get())) { + return false; + } if (run_on_non_semantic_insts) { for (auto& inst : non_semantic_) { - f(inst.get()); + if (!f(inst.get())) { + return false; + } } } + + return true; +} + +bool Graph::WhileEachInst(const std::function<bool(const Instruction*)>& f, + bool run_on_debug_line_insts, + bool run_on_non_semantic_insts) const { + (void)run_on_debug_line_insts; + + if (!f(def_inst_.get())) { + return false; + } + + for (auto& inst : inputs_) { + if (!f(inst.get())) { + return false; + } + } + + for (auto& inst : insts_) { + if (!f(inst.get())) { + return false; + } + } + + for (auto& inst : outputs_) { + if (!f(inst.get())) { + return false; + } + } + + if (!f(end_inst_.get())) { + return false; + } + + if (run_on_non_semantic_insts) { + for (auto& inst : non_semantic_) { + if (!f(inst.get())) { + return false; + } + } + } + + return true; } } // namespace opt
diff --git a/source/opt/graph.h b/source/opt/graph.h index 679db16..3a71e1a 100644 --- a/source/opt/graph.h +++ b/source/opt/graph.h
@@ -90,6 +90,18 @@ bool run_on_debug_line_insts = false, bool run_on_non_semantic_insts = false) const; + // Runs the given function |f| on instructions in this graph, in order, + // and optionally on debug line instructions that might precede them and + // non-semantic instructions that succeed the graph. Terminates early if |f| + // returns false. Returns true if all invocations of |f| returned true; + // otherwise returns false. + bool WhileEachInst(const std::function<bool(Instruction*)>& f, + bool run_on_debug_line_insts = false, + bool run_on_non_semantic_insts = false); + bool WhileEachInst(const std::function<bool(const Instruction*)>& f, + bool run_on_debug_line_insts = false, + bool run_on_non_semantic_insts = false) const; + private: // The OpGraph instruction that begins the definition of this graph. std::unique_ptr<Instruction> def_inst_;
diff --git a/source/opt/instruction_list.h b/source/opt/instruction_list.h index b3e4274..4d9768a 100644 --- a/source/opt/instruction_list.h +++ b/source/opt/instruction_list.h
@@ -116,11 +116,48 @@ // on the preceding debug line instructions. inline void ForEachInst(const std::function<void(Instruction*)>& f, bool run_on_debug_line_insts) { + WhileEachInst( + [&f](Instruction* inst) { + f(inst); + return true; + }, + run_on_debug_line_insts); + } + + inline void ForEachInst(const std::function<void(const Instruction*)>& f, + bool run_on_debug_line_insts) const { + WhileEachInst( + [&f](const Instruction* inst) { + f(inst); + return true; + }, + run_on_debug_line_insts); + } + + // Runs the given function |f| on the instructions in the list and optionally + // on the preceding debug line instructions. Terminates early if |f| returns + // false. Returns true if all invocations of |f| returned true; otherwise + // returns false. + inline bool WhileEachInst(const std::function<bool(Instruction*)>& f, + bool run_on_debug_line_insts) { auto next = begin(); for (auto i = next; i != end(); i = next) { ++next; - i->ForEachInst(f, run_on_debug_line_insts); + if (!i->WhileEachInst(f, run_on_debug_line_insts)) { + return false; + } } + return true; + } + + inline bool WhileEachInst(const std::function<bool(const Instruction*)>& f, + bool run_on_debug_line_insts) const { + for (const auto& i : *this) { + if (!i.WhileEachInst(f, run_on_debug_line_insts)) { + return false; + } + } + return true; } };
diff --git a/source/opt/module.cpp b/source/opt/module.cpp index 242ebd2..b63634c 100644 --- a/source/opt/module.cpp +++ b/source/opt/module.cpp
@@ -87,68 +87,22 @@ void Module::ForEachInst(const std::function<void(Instruction*)>& f, bool run_on_debug_line_insts) { -#define DELEGATE(list) list.ForEachInst(f, run_on_debug_line_insts) - DELEGATE(capabilities_); - DELEGATE(extensions_); - DELEGATE(ext_inst_imports_); - if (memory_model_) memory_model_->ForEachInst(f, run_on_debug_line_insts); - if (sampled_image_address_mode_) - sampled_image_address_mode_->ForEachInst(f, run_on_debug_line_insts); - DELEGATE(entry_points_); - DELEGATE(graph_entry_points_); - DELEGATE(execution_modes_); - DELEGATE(debugs1_); - DELEGATE(debugs2_); - DELEGATE(debugs3_); - DELEGATE(ext_inst_debuginfo_); - DELEGATE(annotations_); - DELEGATE(types_values_); - for (auto& i : functions_) { - i->ForEachInst(f, run_on_debug_line_insts, - /* run_on_non_semantic_insts = */ true); - } - for (auto& g : graphs_) { - g->ForEachInst(f, run_on_debug_line_insts, - /* run_on_non_semantic_insts = */ true); - } -#undef DELEGATE + WhileEachInst( + [&f](Instruction* inst) { + f(inst); + return true; + }, + run_on_debug_line_insts); } void Module::ForEachInst(const std::function<void(const Instruction*)>& f, bool run_on_debug_line_insts) const { -#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_) - static_cast<const Instruction*>(memory_model_.get()) - ->ForEachInst(f, run_on_debug_line_insts); - if (sampled_image_address_mode_) - static_cast<const Instruction*>(sampled_image_address_mode_.get()) - ->ForEachInst(f, run_on_debug_line_insts); - for (auto& i : entry_points_) DELEGATE(i); - for (auto& i : execution_modes_) DELEGATE(i); - for (auto& i : debugs1_) DELEGATE(i); - for (auto& i : debugs2_) DELEGATE(i); - for (auto& i : debugs3_) DELEGATE(i); - for (auto& i : annotations_) DELEGATE(i); - for (auto& i : types_values_) DELEGATE(i); - for (auto& i : ext_inst_debuginfo_) DELEGATE(i); - for (auto& i : functions_) { - static_cast<const Function*>(i.get())->ForEachInst( - f, run_on_debug_line_insts, - /* run_on_non_semantic_insts = */ true); - } - for (auto& i : graph_entry_points_) DELEGATE(i); - for (auto& i : graphs_) { - static_cast<const Graph*>(i.get())->ForEachInst( - f, run_on_debug_line_insts, - /* run_on_non_semantic_insts = */ true); - } - if (run_on_debug_line_insts) { - for (auto& i : trailing_dbg_line_info_) DELEGATE(i); - } -#undef DELEGATE + WhileEachInst( + [&f](const Instruction* inst) { + f(inst); + return true; + }, + run_on_debug_line_insts); } void Module::ToBinary(std::vector<uint32_t>* binary, bool skip_nop, @@ -307,5 +261,79 @@ return str; } +template <typename ModuleT, typename InstT> +bool Module::WhileEachInstImpl(ModuleT* module, + const std::function<bool(InstT*)>& f, + bool run_on_debug_line_insts) { + static_assert(std::is_const<ModuleT>::value == std::is_const<InstT>::value, + "ModuleT and InstT must both be const or both be non-const"); + using FuncT = typename std::conditional<std::is_const<InstT>::value, + const Function, Function>::type; + using GraphT = typename std::conditional<std::is_const<InstT>::value, + const Graph, Graph>::type; + +#define DELEGATE(list) \ + if (!list.WhileEachInst(f, run_on_debug_line_insts)) { \ + return false; \ + } + DELEGATE(module->capabilities_); + DELEGATE(module->extensions_); + DELEGATE(module->ext_inst_imports_); + if (module->memory_model_) { + if (!static_cast<InstT*>(module->memory_model_.get()) + ->WhileEachInst(f, run_on_debug_line_insts)) { + return false; + } + } + if (module->sampled_image_address_mode_) { + if (!static_cast<InstT*>(module->sampled_image_address_mode_.get()) + ->WhileEachInst(f, run_on_debug_line_insts)) { + return false; + } + } + DELEGATE(module->entry_points_); + DELEGATE(module->execution_modes_); + DELEGATE(module->debugs1_); + DELEGATE(module->debugs2_); + DELEGATE(module->debugs3_); + DELEGATE(module->annotations_); + DELEGATE(module->types_values_); + DELEGATE(module->ext_inst_debuginfo_); + for (auto& i : module->functions_) { + if (!static_cast<FuncT*>(i.get())->WhileEachInst( + f, run_on_debug_line_insts, + /* run_on_non_semantic_insts = */ true)) { + return false; + } + } + DELEGATE(module->graph_entry_points_); + for (auto& g : module->graphs_) { + if (!static_cast<GraphT*>(g.get())->WhileEachInst( + f, run_on_debug_line_insts, + /* run_on_non_semantic_insts = */ true)) { + return false; + } + } + if (run_on_debug_line_insts) { + for (auto& i : module->trailing_dbg_line_info_) { + if (!static_cast<InstT*>(&i)->WhileEachInst(f, run_on_debug_line_insts)) { + return false; + } + } + } +#undef DELEGATE + return true; +} + +bool Module::WhileEachInst(const std::function<bool(Instruction*)>& f, + bool run_on_debug_line_insts) { + return WhileEachInstImpl(this, f, run_on_debug_line_insts); +} + +bool Module::WhileEachInst(const std::function<bool(const Instruction*)>& f, + bool run_on_debug_line_insts) const { + return WhileEachInstImpl(this, f, run_on_debug_line_insts); +} + } // namespace opt } // namespace spvtools
diff --git a/source/opt/module.h b/source/opt/module.h index f8733ff..9cc46a9 100644 --- a/source/opt/module.h +++ b/source/opt/module.h
@@ -275,6 +275,15 @@ void ForEachInst(const std::function<void(const Instruction*)>& f, bool run_on_debug_line_insts = false) const; + // Invokes function |f| on all instructions in this module, and optionally on + // the debug line instructions that precede them. Terminates early if |f| + // returns false. Returns true if all invocations of |f| returned true; + // otherwise returns false. + bool WhileEachInst(const std::function<bool(Instruction*)>& f, + bool run_on_debug_line_insts = false); + bool WhileEachInst(const std::function<bool(const Instruction*)>& f, + bool run_on_debug_line_insts = false) const; + // Pushes the binary segments for this module into the back of *`binary`. // If `skip_nop` is true, OpNop instructions will not be added to binary. // If `filter_duplicate_decorations` is true, duplicate decorations will not @@ -312,6 +321,13 @@ } private: + // Helper template for WhileEachInst to share traversal logic between const + // and non-const versions. + template <typename ModuleT, typename InstT> + static bool WhileEachInstImpl(ModuleT* module, + const std::function<bool(InstT*)>& f, + bool run_on_debug_line_insts); + ModuleHeader header_; // Module header // The following fields respect the "Logical Layout of a Module" in
diff --git a/test/opt/module_test.cpp b/test/opt/module_test.cpp index b1fe645..cc78428 100644 --- a/test/opt/module_test.cpp +++ b/test/opt/module_test.cpp
@@ -336,6 +336,73 @@ EXPECT_EQ(1, non_semantic_ids.count(12)); } +TEST(ModuleTest, WhileEachInstEarlyTermination) { + const std::string text = R"( +OpCapability Shader +OpMemoryModel Logical Simple +OpEntryPoint Vertex %main "main" +%void = OpTypeVoid +%func = OpTypeFunction %void +%main = OpFunction %void None %func +%10 = OpLabel +OpReturn +OpFunctionEnd +)"; + + std::unique_ptr<IRContext> context = BuildModule(text); + int count = 0; + bool completed = context->module()->WhileEachInst([&count](Instruction*) { + ++count; + return count < 3; + }); + + EXPECT_FALSE(completed); + EXPECT_EQ(3, count); + + count = 0; + completed = context->module()->WhileEachInst([&count](Instruction*) { + ++count; + return true; + }); + + EXPECT_TRUE(completed); + EXPECT_GT(count, 3); +} + +TEST(ModuleTest, ConstWhileEachInstEarlyTermination) { + const std::string text = R"( +OpCapability Shader +OpMemoryModel Logical Simple +OpEntryPoint Vertex %main "main" +%void = OpTypeVoid +%func = OpTypeFunction %void +%main = OpFunction %void None %func +%10 = OpLabel +OpReturn +OpFunctionEnd +)"; + + std::unique_ptr<IRContext> context = BuildModule(text); + const Module* module = context->module(); + int count = 0; + bool completed = module->WhileEachInst([&count](const Instruction*) { + ++count; + return count < 3; + }); + + EXPECT_FALSE(completed); + EXPECT_EQ(3, count); + + count = 0; + completed = module->WhileEachInst([&count](const Instruction*) { + ++count; + return true; + }); + + EXPECT_TRUE(completed); + EXPECT_GT(count, 3); +} + // Assembles `text`, serializes it to binary (with duplicate decorations // filtered if `filter_duplicate_decorations` is true), disassembles it, and // checks the output against the Effcee checks in `text` using the given