Skip to content

Commit d4b9f57

Browse files
jaebaekjaebaek
authored
[spirv-opt] debug info preservation in ssa-rewrite (KhronosGroup#3356)
Add OpenCL.DebugInfo.100 `DebugValue` instructions for store and phi instructions of local variables to provide the debugger with the updated values of local variables correctly.
1 parent 2a1b8c0 commit d4b9f57

File tree

9 files changed

+1712
-16
lines changed

9 files changed

+1712
-16
lines changed

‎source/opt/debug_info_manager.cpp‎

Lines changed: 287 additions & 5 deletions
Large diffs are not rendered by default.

‎source/opt/debug_info_manager.h‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#define SOURCE_OPT_DEBUG_INFO_MANAGER_H_
1717

1818
#include <unordered_map>
19+
#include <vector>
1920

2021
#include "source/opt/instruction.h"
2122
#include "source/opt/module.h"
@@ -128,6 +129,15 @@ class DebugInfoManager {
128129
uint32_t BuildDebugInlinedAtChain(uint32_t callee_inlined_at,
129130
DebugInlinedAtContext* inlined_at_ctx);
130131

132+
// Generates a DebugValue instruction with value |value_id| for every local
133+
// variable that is in the scope of |scope_and_line| and whose memory is
134+
// |variable_id| and inserts it after the instruction |insert_pos|.
135+
void AddDebugValue(Instruction* scope_and_line, uint32_t variable_id,
136+
uint32_t value_id, Instruction* insert_pos);
137+
138+
// Erases |instr| from data structures of this class.
139+
void ClearDebugInfo(Instruction* instr);
140+
131141
private:
132142
IRContext* context() { return context_; }
133143

@@ -147,6 +157,30 @@ class DebugInfoManager {
147157
// in |inst| must not already be registered.
148158
void RegisterDbgFunction(Instruction* inst);
149159

160+
// Register the DebugDeclare instruction |dbg_declare| into
161+
// |var_id_to_dbg_decl_| using OpVariable id |var_id| as a key.
162+
void RegisterDbgDeclare(uint32_t var_id, Instruction* dbg_declare);
163+
164+
// Returns a DebugExpression instruction without Operation operands.
165+
Instruction* GetEmptyDebugExpression();
166+
167+
// Returns the id of Value operand if |inst| is DebugValue who has Deref
168+
// operation and its Value operand is a result id of OpVariable with
169+
// Function storage class. Otherwise, returns 0.
170+
uint32_t GetVariableIdOfDebugValueUsedForDeclare(Instruction* inst);
171+
172+
// Returns true if a scope |ancestor| is |scope| or an ancestor scope
173+
// of |scope|.
174+
bool IsAncestorOfScope(uint32_t scope, uint32_t ancestor);
175+
176+
// Returns true if the declaration of a local variable |dbg_declare|
177+
// is visible in the scope of an instruction |instr_scope_id|.
178+
bool IsDeclareVisibleToInstr(Instruction* dbg_declare,
179+
uint32_t instr_scope_id);
180+
181+
// Returns the parent scope of the scope |child_scope|.
182+
uint32_t GetParentScope(uint32_t child_scope);
183+
150184
IRContext* context_;
151185

152186
// Mapping from ids of OpenCL.DebugInfo.100 extension instructions
@@ -157,9 +191,18 @@ class DebugInfoManager {
157191
// operand is the function.
158192
std::unordered_map<uint32_t, Instruction*> fn_id_to_dbg_fn_;
159193

194+
// Mapping from local variable ids to DebugDeclare instructions whose
195+
// operand is the local variable.
196+
std::unordered_map<uint32_t, std::vector<Instruction*>> var_id_to_dbg_decl_;
197+
160198
// DebugInfoNone instruction. We need only a single DebugInfoNone.
161199
// To reuse the existing one, we keep it using this member variable.
162200
Instruction* debug_info_none_inst_;
201+
202+
// DebugExpression instruction without Operation operands. We need only
203+
// a single DebugExpression without Operation operands. To reuse the
204+
// existing one, we keep it using this member variable.
205+
Instruction* empty_debug_expr_inst_;
163206
};
164207

165208
} // namespace analysis

‎source/opt/ir_context.cpp‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,9 @@ void IRContext::InvalidateAnalysesExceptFor(
9797
}
9898

9999
void IRContext::InvalidateAnalyses(IRContext::Analysis analyses_to_invalidate) {
100-
// The ConstantManager contains Type pointers. If the TypeManager goes
101-
// away, the ConstantManager has to go away.
100+
// The ConstantManager and DebugInfoManager contain Type pointers. If the
101+
// TypeManager goes away, the ConstantManager and DebugInfoManager have to
102+
// go away.
102103
if (analyses_to_invalidate & kAnalysisTypes) {
103104
analyses_to_invalidate |= kAnalysisConstants;
104105
analyses_to_invalidate |= kAnalysisDebugInfo;
@@ -179,6 +180,9 @@ Instruction* IRContext::KillInst(Instruction* inst) {
179180
decoration_mgr_->RemoveDecoration(inst);
180181
}
181182
}
183+
if (AreAnalysesValid(kAnalysisDebugInfo)) {
184+
get_debug_info_mgr()->ClearDebugInfo(inst);
185+
}
182186
if (type_mgr_ && IsTypeInst(inst->opcode())) {
183187
type_mgr_->RemoveId(inst->result_id());
184188
}
@@ -218,6 +222,13 @@ bool IRContext::KillDef(uint32_t id) {
218222
return false;
219223
}
220224

225+
void IRContext::KillDebugDeclareInsts(Function* fn) {
226+
fn->ForEachInst([this](Instruction* inst) {
227+
if (inst->GetOpenCL100DebugOpcode() == OpenCLDebugInfo100DebugDeclare)
228+
KillInst(inst);
229+
});
230+
}
231+
221232
bool IRContext::ReplaceAllUsesWith(uint32_t before, uint32_t after) {
222233
return ReplaceAllUsesWithPredicate(
223234
before, after, [](Instruction*, uint32_t) { return true; });
@@ -394,6 +405,7 @@ void IRContext::KillOperandFromDebugInstructions(Instruction* inst) {
394405
if (operand.words[0] == id) {
395406
operand.words[0] =
396407
get_debug_info_mgr()->GetDebugInfoNone()->result_id();
408+
get_def_use_mgr()->AnalyzeInstUse(&*it);
397409
}
398410
}
399411
}
@@ -408,6 +420,7 @@ void IRContext::KillOperandFromDebugInstructions(Instruction* inst) {
408420
if (operand.words[0] == id) {
409421
operand.words[0] =
410422
get_debug_info_mgr()->GetDebugInfoNone()->result_id();
423+
get_def_use_mgr()->AnalyzeInstUse(&*it);
411424
}
412425
}
413426
}

‎source/opt/ir_context.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,9 @@ class IRContext {
403403
// instruction exists.
404404
Instruction* KillInst(Instruction* inst);
405405

406+
// Deletes DebugDeclare instructions in the given function |fn|.
407+
void KillDebugDeclareInsts(Function* fn);
408+
406409
// Returns true if all of the given analyses are valid.
407410
bool AreAnalysesValid(Analysis set) { return (set & valid_analyses_) == set; }
408411

‎source/opt/mem_pass.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <set>
2121
#include <vector>
2222

23+
#include "OpenCLDebugInfo100.h"
2324
#include "source/cfa.h"
2425
#include "source/opt/basic_block.h"
2526
#include "source/opt/dominator_analysis.h"
@@ -225,6 +226,11 @@ MemPass::MemPass() {}
225226

226227
bool MemPass::HasOnlySupportedRefs(uint32_t varId) {
227228
return get_def_use_mgr()->WhileEachUser(varId, [this](Instruction* user) {
229+
auto dbg_op = user->GetOpenCL100DebugOpcode();
230+
if (dbg_op == OpenCLDebugInfo100DebugDeclare ||
231+
dbg_op == OpenCLDebugInfo100DebugValue) {
232+
return true;
233+
}
228234
SpvOp op = user->opcode();
229235
if (op != SpvOpStore && op != SpvOpLoad && op != SpvOpName &&
230236
!IsNonTypeDecorate(op)) {

‎source/opt/pass.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ class Pass {
7171
return context()->get_def_use_mgr();
7272
}
7373

74+
analysis::DebugInfoManager* get_debug_info_mgr() const {
75+
return context()->get_debug_info_mgr();
76+
}
77+
7478
analysis::DecorationManager* get_decoration_mgr() const {
7579
return context()->get_decoration_mgr();
7680
}

‎source/opt/ssa_rewrite_pass.cpp‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ void SSARewriter::ProcessStore(Instruction* inst, BasicBlock* bb) {
307307
}
308308
if (pass_->IsTargetVar(var_id)) {
309309
WriteVariable(var_id, bb, val_id);
310+
pass_->get_debug_info_mgr()->AddDebugValue(inst, var_id, val_id, inst);
310311

311312
#if SSA_REWRITE_DEBUGGING_LEVEL > 1
312313
std::cerr << "\tFound store '%" << var_id << " = %" << val_id << "': "
@@ -437,6 +438,8 @@ bool SSARewriter::ApplyReplacements() {
437438

438439
// Add Phi instructions from completed Phi candidates.
439440
std::vector<Instruction*> generated_phis;
441+
// Add DebugValue instructions for Phi instructions.
442+
std::vector<Instruction*> dbg_values_for_phis;
440443
for (const PhiCandidate* phi_candidate : phis_to_generate_) {
441444
#if SSA_REWRITE_DEBUGGING_LEVEL > 2
442445
std::cerr << "Phi candidate: " << phi_candidate->PrettyPrint(pass_->cfg())
@@ -447,9 +450,10 @@ bool SSARewriter::ApplyReplacements() {
447450
"Tried to instantiate a Phi instruction from an incomplete Phi "
448451
"candidate");
449452

453+
auto* local_var = pass_->get_def_use_mgr()->GetDef(phi_candidate->var_id());
454+
450455
// Build the vector of operands for the new OpPhi instruction.
451-
uint32_t type_id = pass_->GetPointeeTypeId(
452-
pass_->get_def_use_mgr()->GetDef(phi_candidate->var_id()));
456+
uint32_t type_id = pass_->GetPointeeTypeId(local_var);
453457
std::vector<Operand> phi_operands;
454458
uint32_t arg_ix = 0;
455459
std::unordered_map<uint32_t, uint32_t> already_seen;
@@ -479,11 +483,17 @@ bool SSARewriter::ApplyReplacements() {
479483
pass_->get_def_use_mgr()->AnalyzeInstDef(&*phi_inst);
480484
pass_->context()->set_instr_block(&*phi_inst, phi_candidate->bb());
481485
auto insert_it = phi_candidate->bb()->begin();
482-
insert_it.InsertBefore(std::move(phi_inst));
486+
insert_it = insert_it.InsertBefore(std::move(phi_inst));
483487
pass_->context()->get_decoration_mgr()->CloneDecorations(
484488
phi_candidate->var_id(), phi_candidate->result_id(),
485489
{SpvDecorationRelaxedPrecision});
486490

491+
// Add DebugValue for the new OpPhi instruction.
492+
insert_it->SetDebugScope(local_var->GetDebugScope());
493+
pass_->get_debug_info_mgr()->AddDebugValue(
494+
&*insert_it, phi_candidate->var_id(), phi_candidate->result_id(),
495+
&*insert_it);
496+
487497
modified = true;
488498
}
489499

@@ -604,6 +614,8 @@ Pass::Status SSARewriter::RewriteFunctionIntoSSA(Function* fp) {
604614
<< fp->PrettyPrint(0) << "\n";
605615
#endif
606616

617+
if (modified) pass_->context()->KillDebugDeclareInsts(fp);
618+
607619
return modified ? Pass::Status::SuccessWithChange
608620
: Pass::Status::SuccessWithoutChange;
609621
}

‎source/opt/ssa_rewrite_pass.h‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ namespace opt {
3939
// (https://link.springer.com/chapter/10.1007/978-3-642-37051-9_6)
4040
class SSARewriter {
4141
public:
42-
SSARewriter(MemPass* pass)
43-
: pass_(pass), first_phi_id_(pass_->get_module()->IdBound()) {}
42+
SSARewriter(MemPass* pass) : pass_(pass) {}
4443

4544
// Rewrites SSA-target variables in function |fp| into SSA. This is the
4645
// entry point for the SSA rewrite algorithm. SSA-target variables are
@@ -287,10 +286,6 @@ class SSARewriter {
287286

288287
// Memory pass requesting the SSA rewriter.
289288
MemPass* pass_;
290-
291-
// ID of the first Phi created by the SSA rewriter. During rewriting, any
292-
// ID bigger than this corresponds to a Phi candidate.
293-
uint32_t first_phi_id_;
294289
};
295290

296291
class SSARewritePass : public MemPass {

0 commit comments

Comments
 (0)