BuildModule: optionally avoid adding new OpLine instructions (#4033)
* BuildModule: optionally avoid adding new OpLine instructions
Fixes #4029 for my use case
* Fix formatting
* Create last_line_inst_ only if doing extra line tracking
diff --git a/source/opt/build_module.cpp b/source/opt/build_module.cpp
index fc76a3c..3b606dc 100644
--- a/source/opt/build_module.cpp
+++ b/source/opt/build_module.cpp
@@ -50,11 +50,20 @@
MessageConsumer consumer,
const uint32_t* binary,
const size_t size) {
+ return BuildModule(env, consumer, binary, size, true);
+}
+
+std::unique_ptr<opt::IRContext> BuildModule(spv_target_env env,
+ MessageConsumer consumer,
+ const uint32_t* binary,
+ const size_t size,
+ bool extra_line_tracking) {
auto context = spvContextCreate(env);
SetContextMessageConsumer(context, consumer);
auto irContext = MakeUnique<opt::IRContext>(env, consumer);
opt::IrLoader loader(consumer, irContext->module());
+ loader.SetExtraLineTracking(extra_line_tracking);
spv_result_t status = spvBinaryParse(context, &loader, binary, size,
SetSpvHeader, SetSpvInst, nullptr);
diff --git a/source/opt/build_module.h b/source/opt/build_module.h
index c9d1cf2..29eaf66 100644
--- a/source/opt/build_module.h
+++ b/source/opt/build_module.h
@@ -27,7 +27,15 @@
// Builds an Module returns the owning IRContext from the given SPIR-V
// |binary|. |size| specifies number of words in |binary|. The |binary| will be
// decoded according to the given target |env|. Returns nullptr if errors occur
-// and sends the errors to |consumer|.
+// and sends the errors to |consumer|. When |extra_line_tracking| is true,
+// extra OpLine instructions are injected to better presere line numbers while
+// later transforms mutate the module.
+std::unique_ptr<opt::IRContext> BuildModule(spv_target_env env,
+ MessageConsumer consumer,
+ const uint32_t* binary, size_t size,
+ bool extra_line_tracking);
+
+// Like above, with extra line tracking turned on.
std::unique_ptr<opt::IRContext> BuildModule(spv_target_env env,
MessageConsumer consumer,
const uint32_t* binary,
diff --git a/source/opt/ir_loader.cpp b/source/opt/ir_loader.cpp
index 4a44309..06099ce 100644
--- a/source/opt/ir_loader.cpp
+++ b/source/opt/ir_loader.cpp
@@ -92,7 +92,8 @@
std::unique_ptr<Instruction> spv_inst(
new Instruction(module()->context(), *inst, std::move(dbg_line_info_)));
if (!spv_inst->dbg_line_insts().empty()) {
- if (spv_inst->dbg_line_insts().back().opcode() != SpvOpNoLine) {
+ if (extra_line_tracking_ &&
+ (spv_inst->dbg_line_insts().back().opcode() != SpvOpNoLine)) {
last_line_inst_ = std::unique_ptr<Instruction>(
spv_inst->dbg_line_insts().back().Clone(module()->context()));
}
diff --git a/source/opt/ir_loader.h b/source/opt/ir_loader.h
index d0610f1..16bc2c7 100644
--- a/source/opt/ir_loader.h
+++ b/source/opt/ir_loader.h
@@ -63,6 +63,10 @@
// or a missing OpFunctionEnd. Resolves internal bookkeeping.
void EndModule();
+ // Sets whether extra OpLine instructions should be injected to better
+ // track line information.
+ void SetExtraLineTracking(bool flag) { extra_line_tracking_ = flag; }
+
private:
// Consumer for communicating messages to outside.
const MessageConsumer& consumer_;
@@ -78,11 +82,17 @@
std::unique_ptr<BasicBlock> block_;
// Line related debug instructions accumulated thus far.
std::vector<Instruction> dbg_line_info_;
- // Line instruction that should be applied to the next instruction.
+ // If doing extra line tracking, this is the line instruction that should be
+ // applied to the next instruction. Otherwise it always contains null.
std::unique_ptr<Instruction> last_line_inst_;
// The last DebugScope information that IrLoader::AddInstruction() handled.
DebugScope last_dbg_scope_;
+
+ // When true, do extra line information tracking: Additional OpLine
+ // instructions will be injected to help track line info more robustly during
+ // transformations.
+ bool extra_line_tracking_ = true;
};
} // namespace opt
diff --git a/test/opt/ir_loader_test.cpp b/test/opt/ir_loader_test.cpp
index 8b77aa3..475dd23 100644
--- a/test/opt/ir_loader_test.cpp
+++ b/test/opt/ir_loader_test.cpp
@@ -19,7 +19,7 @@
#include <utility>
#include <vector>
-#include "gtest/gtest.h"
+#include "gmock/gmock.h"
#include "source/opt/build_module.h"
#include "source/opt/def_use_manager.h"
#include "source/opt/ir_context.h"
@@ -29,6 +29,8 @@
namespace opt {
namespace {
+using ::testing::ContainerEq;
+
constexpr uint32_t kOpLineOperandLineIndex = 1;
void DoRoundTripCheck(const std::string& text) {
@@ -236,6 +238,100 @@
}
}
+TEST(IrBuilder, BuildModule_WithoutExtraLines) {
+ const std::string text = R"(OpCapability Shader
+OpMemoryModel Logical Simple
+OpEntryPoint Vertex %main "main"
+%file = OpString "my file"
+%void = OpTypeVoid
+%voidfn = OpTypeFunction %void
+%float = OpTypeFloat 32
+%float_1 = OpConstant %float 1
+%main = OpFunction %void None %voidfn
+%100 = OpLabel
+%1 = OpFAdd %float %float_1 %float_1
+OpLine %file 1 0
+%2 = OpFMul %float %1 %1
+%3 = OpFSub %float %2 %2
+OpReturn
+OpFunctionEnd
+)";
+
+ std::vector<uint32_t> binary;
+ SpirvTools t(SPV_ENV_UNIVERSAL_1_1);
+ ASSERT_TRUE(t.Assemble(text, &binary,
+ SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS));
+
+ // This is the function we're testing.
+ std::unique_ptr<IRContext> context = BuildModule(
+ SPV_ENV_UNIVERSAL_1_5, nullptr, binary.data(), binary.size(), false);
+ ASSERT_NE(nullptr, context);
+
+ spvtools::opt::analysis::DefUseManager* def_use_mgr =
+ context->get_def_use_mgr();
+
+ std::vector<SpvOp> opcodes;
+ for (auto* inst = def_use_mgr->GetDef(1);
+ inst && (inst->opcode() != SpvOpFunctionEnd); inst = inst->NextNode()) {
+ inst->ForEachInst(
+ [&opcodes](spvtools::opt::Instruction* sub_inst) {
+ opcodes.push_back(sub_inst->opcode());
+ },
+ true);
+ }
+
+ EXPECT_THAT(opcodes,
+ ContainerEq(std::vector<SpvOp>{SpvOpFAdd, SpvOpLine, SpvOpFMul,
+ SpvOpFSub, SpvOpReturn}));
+}
+
+TEST(IrBuilder, BuildModule_WithExtraLines_IsDefault) {
+ const std::string text = R"(OpCapability Shader
+OpMemoryModel Logical Simple
+OpEntryPoint Vertex %main "main"
+%file = OpString "my file"
+%void = OpTypeVoid
+%voidfn = OpTypeFunction %void
+%float = OpTypeFloat 32
+%float_1 = OpConstant %float 1
+%main = OpFunction %void None %voidfn
+%100 = OpLabel
+%1 = OpFAdd %float %float_1 %float_1
+OpLine %file 1 0
+%2 = OpFMul %float %1 %1
+%3 = OpFSub %float %2 %2
+OpReturn
+OpFunctionEnd
+)";
+
+ std::vector<uint32_t> binary;
+
+ SpirvTools t(SPV_ENV_UNIVERSAL_1_1);
+ ASSERT_TRUE(t.Assemble(text, &binary,
+ SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS));
+
+ // This is the function we're testing.
+ std::unique_ptr<IRContext> context =
+ BuildModule(SPV_ENV_UNIVERSAL_1_5, nullptr, binary.data(), binary.size());
+
+ spvtools::opt::analysis::DefUseManager* def_use_mgr =
+ context->get_def_use_mgr();
+
+ std::vector<SpvOp> opcodes;
+ for (auto* inst = def_use_mgr->GetDef(1);
+ inst && (inst->opcode() != SpvOpFunctionEnd); inst = inst->NextNode()) {
+ inst->ForEachInst(
+ [&opcodes](spvtools::opt::Instruction* sub_inst) {
+ opcodes.push_back(sub_inst->opcode());
+ },
+ true);
+ }
+
+ EXPECT_THAT(opcodes, ContainerEq(std::vector<SpvOp>{
+ SpvOpFAdd, SpvOpLine, SpvOpFMul, SpvOpLine,
+ SpvOpFSub, SpvOpLine, SpvOpReturn}));
+}
+
TEST(IrBuilder, ConsumeDebugInfoInst) {
// /* HLSL */
//
