Skip to content

[DebugInfo][RemoveDIs] Handle non-instr debug-info in GlobalISel #75228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

[DebugInfo][RemoveDIs] Handle non-instr debug-info in GlobalISel #75228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d2bf6b4
[DebugInfo][RemoveDIs] Handle non-instr debug-info in GlobalISel
jmorse Dec 12, 2023
012a9af
Remove excess parens
jmorse Jan 22, 2024
0d6c960
Capitalization correction
jmorse Jan 22, 2024
a2475cf
Merge remote-tracking branch 'github/main' into ddd-globalisel
jmorse Jan 22, 2024
dfbad2c
Refactor production of dbg.declares to be abstracted from storage
jmorse Jan 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,27 @@ class IRTranslator : public MachineFunctionPass {
/// \return true if the materialization succeeded.
bool translate(const Constant &C, Register Reg);

/// Examine any debug-info attached to the instruction (in the form of
/// DPValues) and translate it.
void translateDbgInfo(const Instruction &Inst,
MachineIRBuilder &MIRBuilder);

/// Translate a debug-info record of a dbg.value into a DBG_* instruction.
/// Pass in all the contents of the record, rather than relying on how it's
/// stored.
void translateDbgValueRecord(Value *V, bool HasArgList,
const DILocalVariable *Variable,
const DIExpression *Expression, const DebugLoc &DL,
MachineIRBuilder &MIRBuilder);

/// Translate a debug-info record of a dbg.declare into an indirect DBG_*
/// instruction. Pass in all the contents of the record, rather than relying
/// on how it's stored.
void translateDbgDeclareRecord(Value *Address, bool HasArgList,
const DILocalVariable *Variable,
const DIExpression *Expression, const DebugLoc &DL,
MachineIRBuilder &MIRBuilder);

// Translate U as a copy of V.
bool translateCopy(const User &U, const Value &V,
MachineIRBuilder &MIRBuilder);
Expand Down Expand Up @@ -250,14 +271,14 @@ class IRTranslator : public MachineFunctionPass {
/// possible.
std::optional<MCRegister> getArgPhysReg(Argument &Arg);

/// If DebugInst targets an Argument and its expression is an EntryValue,
/// lower it as an entry in the MF debug table.
bool translateIfEntryValueArgument(const DbgDeclareInst &DebugInst);

/// If DebugInst targets an Argument and its expression is an EntryValue,
/// lower as a DBG_VALUE targeting the corresponding livein register for that
/// Argument.
bool translateIfEntryValueArgument(const DbgValueInst &DebugInst,
/// If debug-info targets an Argument and its expression is an EntryValue,
/// lower it as either an entry in the MF debug table (dbg.declare), or a
/// DBG_VALUE targeting the corresponding livein register for that Argument
/// (dbg.value).
bool translateIfEntryValueArgument(bool isDeclare, Value *Arg,
const DILocalVariable *Var,
const DIExpression *Expr,
const DebugLoc &DL,
MachineIRBuilder &MIRBuilder);

bool translateInlineAsm(const CallBase &CB, MachineIRBuilder &MIRBuilder);
Expand Down
212 changes: 122 additions & 90 deletions llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ class DILocationVerifier : public GISelChangeObserver {
// they could have originated from constants, and we don't want a jumpy
// debug experience.
assert((CurrInst->getDebugLoc() == MI.getDebugLoc() ||
(MI.getParent()->isEntryBlock() && !MI.getDebugLoc())) &&
(MI.getParent()->isEntryBlock() && !MI.getDebugLoc()) ||
(MI.isDebugInstr())) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor point, but is this a good guarantee to drop? I assume that this condition isn't true when we translate DPValues, because MI is a debug instruction corresponding to the DPV, and CurrInst is unrelated, but this also drops all checks that DebugLocs for debug instructions are translated correctly. Maybe that translation is trivial enough that we don't care about verifier coverage though - it looks it!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point; we should only open a gap in this check for the things that are absolutely necessary, I'll refine it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came back to this (after a while) and prototyped something, however I'm feeling that this is too tricky. Specifically, as long as we have debug intrinsics (like dbg.label, for at least the next two weeks...) they can have DPValues attached to the front of them, which will translate into multiple debug instructions, only one of which we would want to check. IMO, while this might have been an easy and useful assertion in the past, now that we're making debug-info "something else" it's too difficult to keep checking that.

"Line info was not transferred to all instructions");
}
};
Expand Down Expand Up @@ -2006,47 +2007,35 @@ std::optional<MCRegister> IRTranslator::getArgPhysReg(Argument &Arg) {
return VRegDef->getOperand(1).getReg().asMCReg();
}

bool IRTranslator::translateIfEntryValueArgument(const DbgValueInst &DebugInst,
bool IRTranslator::translateIfEntryValueArgument(bool isDeclare, Value *Val,
const DILocalVariable *Var,
const DIExpression *Expr,
const DebugLoc &DL,
MachineIRBuilder &MIRBuilder) {
auto *Arg = dyn_cast<Argument>(DebugInst.getValue());
auto *Arg = dyn_cast<Argument>(Val);
if (!Arg)
return false;

const DIExpression *Expr = DebugInst.getExpression();
if (!Expr->isEntryValue())
return false;

std::optional<MCRegister> PhysReg = getArgPhysReg(*Arg);
if (!PhysReg) {
LLVM_DEBUG(dbgs() << "Dropping dbg.value: expression is entry_value but "
"couldn't find a physical register\n"
<< DebugInst << "\n");
LLVM_DEBUG(dbgs() << "Dropping dbg." << (isDeclare ? "declare" : "value")
<< ": expression is entry_value but "
<< "couldn't find a physical register\n");
LLVM_DEBUG(dbgs() << *Var << "\n");
return true;
}

MIRBuilder.buildDirectDbgValue(*PhysReg, DebugInst.getVariable(),
DebugInst.getExpression());
return true;
}

bool IRTranslator::translateIfEntryValueArgument(
const DbgDeclareInst &DebugInst) {
auto *Arg = dyn_cast<Argument>(DebugInst.getAddress());
if (!Arg)
return false;

const DIExpression *Expr = DebugInst.getExpression();
if (!Expr->isEntryValue())
return false;

std::optional<MCRegister> PhysReg = getArgPhysReg(*Arg);
if (!PhysReg)
return false;
if (isDeclare) {
// Append an op deref to account for the fact that this is a dbg_declare.
Expr = DIExpression::append(Expr, dwarf::DW_OP_deref);
MF->setVariableDbgInfo(Var, Expr, *PhysReg, DL);
} else {
MIRBuilder.buildDirectDbgValue(*PhysReg, Var, Expr);
}

// Append an op deref to account for the fact that this is a dbg_declare.
Expr = DIExpression::append(Expr, dwarf::DW_OP_deref);
MF->setVariableDbgInfo(DebugInst.getVariable(), Expr, *PhysReg,
DebugInst.getDebugLoc());
return true;
}

Expand Down Expand Up @@ -2100,32 +2089,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
case Intrinsic::dbg_declare: {
const DbgDeclareInst &DI = cast<DbgDeclareInst>(CI);
assert(DI.getVariable() && "Missing variable");

const Value *Address = DI.getAddress();
if (!Address || isa<UndefValue>(Address)) {
LLVM_DEBUG(dbgs() << "Dropping debug info for " << DI << "\n");
return true;
}

assert(DI.getVariable()->isValidLocationForIntrinsic(
MIRBuilder.getDebugLoc()) &&
"Expected inlined-at fields to agree");
auto AI = dyn_cast<AllocaInst>(Address);
if (AI && AI->isStaticAlloca()) {
// Static allocas are tracked at the MF level, no need for DBG_VALUE
// instructions (in fact, they get ignored if they *do* exist).
MF->setVariableDbgInfo(DI.getVariable(), DI.getExpression(),
getOrCreateFrameIndex(*AI), DI.getDebugLoc());
return true;
}

if (translateIfEntryValueArgument(DI))
return true;

// A dbg.declare describes the address of a source variable, so lower it
// into an indirect DBG_VALUE.
MIRBuilder.buildIndirectDbgValue(getOrCreateVReg(*Address),
DI.getVariable(), DI.getExpression());
translateDbgDeclareRecord(DI.getAddress(), DI.hasArgList(), DI.getVariable(),
DI.getExpression(), DI.getDebugLoc(), MIRBuilder);
return true;
}
case Intrinsic::dbg_label: {
Expand Down Expand Up @@ -2158,41 +2123,8 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
case Intrinsic::dbg_value: {
// This form of DBG_VALUE is target-independent.
const DbgValueInst &DI = cast<DbgValueInst>(CI);
const Value *V = DI.getValue();
assert(DI.getVariable()->isValidLocationForIntrinsic(
MIRBuilder.getDebugLoc()) &&
"Expected inlined-at fields to agree");
if (!V || DI.hasArgList()) {
// DI cannot produce a valid DBG_VALUE, so produce an undef DBG_VALUE to
// terminate any prior location.
MIRBuilder.buildIndirectDbgValue(0, DI.getVariable(), DI.getExpression());
return true;
}
if (const auto *CI = dyn_cast<Constant>(V)) {
MIRBuilder.buildConstDbgValue(*CI, DI.getVariable(), DI.getExpression());
return true;
}
if (auto *AI = dyn_cast<AllocaInst>(V);
AI && AI->isStaticAlloca() && DI.getExpression()->startsWithDeref()) {
// If the value is an alloca and the expression starts with a
// dereference, track a stack slot instead of a register, as registers
// may be clobbered.
auto ExprOperands = DI.getExpression()->getElements();
auto *ExprDerefRemoved =
DIExpression::get(AI->getContext(), ExprOperands.drop_front());
MIRBuilder.buildFIDbgValue(getOrCreateFrameIndex(*AI), DI.getVariable(),
ExprDerefRemoved);
return true;
}
if (translateIfEntryValueArgument(DI, MIRBuilder))
return true;
for (Register Reg : getOrCreateVRegs(*V)) {
// FIXME: This does not handle register-indirect values at offset 0. The
// direct/indirect thing shouldn't really be handled by something as
// implicit as reg+noreg vs reg+imm in the first place, but it seems
// pretty baked in right now.
MIRBuilder.buildDirectDbgValue(Reg, DI.getVariable(), DI.getExpression());
}
translateDbgValueRecord(DI.getValue(), DI.hasArgList(), DI.getVariable(),
DI.getExpression(), DI.getDebugLoc(), MIRBuilder);
return true;
}
case Intrinsic::uadd_with_overflow:
Expand Down Expand Up @@ -3250,6 +3182,102 @@ void IRTranslator::finishPendingPhis() {
}
}

void IRTranslator::translateDbgValueRecord(Value *V, bool HasArgList,
const DILocalVariable *Variable,
const DIExpression *Expression,
const DebugLoc &DL,
MachineIRBuilder &MIRBuilder) {
assert(Variable->isValidLocationForIntrinsic(DL) &&
"Expected inlined-at fields to agree");
// Act as if we're handling a debug intrinsic.
MIRBuilder.setDebugLoc(DL);

if (!V || HasArgList) {
// DI cannot produce a valid DBG_VALUE, so produce an undef DBG_VALUE to
// terminate any prior location.
MIRBuilder.buildIndirectDbgValue(0, Variable, Expression);
return;
}

if (const auto *CI = dyn_cast<Constant>(V)) {
MIRBuilder.buildConstDbgValue(*CI, Variable, Expression);
return;
}

if (auto *AI = dyn_cast<AllocaInst>(V);
AI && AI->isStaticAlloca() && Expression->startsWithDeref()) {
// If the value is an alloca and the expression starts with a
// dereference, track a stack slot instead of a register, as registers
// may be clobbered.
auto ExprOperands = Expression->getElements();
auto *ExprDerefRemoved =
DIExpression::get(AI->getContext(), ExprOperands.drop_front());
MIRBuilder.buildFIDbgValue(getOrCreateFrameIndex(*AI), Variable,
ExprDerefRemoved);
return;
}
if (translateIfEntryValueArgument(false, V, Variable, Expression, DL,
MIRBuilder))
return;
for (Register Reg : getOrCreateVRegs(*V)) {
// FIXME: This does not handle register-indirect values at offset 0. The
// direct/indirect thing shouldn't really be handled by something as
// implicit as reg+noreg vs reg+imm in the first place, but it seems
// pretty baked in right now.
MIRBuilder.buildDirectDbgValue(Reg, Variable, Expression);
}
return;
}

void IRTranslator::translateDbgDeclareRecord(Value *Address, bool HasArgList,
const DILocalVariable *Variable,
const DIExpression *Expression,
const DebugLoc &DL,
MachineIRBuilder &MIRBuilder) {
if (!Address || isa<UndefValue>(Address)) {
LLVM_DEBUG(dbgs() << "Dropping debug info for " << *Variable << "\n");
return;
}

assert(Variable->isValidLocationForIntrinsic(DL) &&
"Expected inlined-at fields to agree");
auto AI = dyn_cast<AllocaInst>(Address);
if (AI && AI->isStaticAlloca()) {
// Static allocas are tracked at the MF level, no need for DBG_VALUE
// instructions (in fact, they get ignored if they *do* exist).
MF->setVariableDbgInfo(Variable, Expression,
getOrCreateFrameIndex(*AI), DL);
return;
}

if (translateIfEntryValueArgument(true, Address, Variable,
Expression, DL,
MIRBuilder))
return;

// A dbg.declare describes the address of a source variable, so lower it
// into an indirect DBG_VALUE.
MIRBuilder.setDebugLoc(DL);
MIRBuilder.buildIndirectDbgValue(getOrCreateVReg(*Address),
Variable, Expression);
return;
}

void IRTranslator::translateDbgInfo(const Instruction &Inst,
MachineIRBuilder &MIRBuilder) {
for (DPValue &DPV : Inst.getDbgValueRange()) {
const DILocalVariable *Variable = DPV.getVariable();
const DIExpression *Expression = DPV.getExpression();
Value *V = DPV.getVariableLocationOp(0);
if (DPV.isDbgDeclare())
translateDbgDeclareRecord(V, DPV.hasArgList(), Variable,
Expression, DPV.getDebugLoc(), MIRBuilder);
else
translateDbgValueRecord(V, DPV.hasArgList(), Variable,
Expression, DPV.getDebugLoc(), MIRBuilder);
}
}

bool IRTranslator::translate(const Instruction &Inst) {
CurBuilder->setDebugLoc(Inst.getDebugLoc());
CurBuilder->setPCSections(Inst.getMetadata(LLVMContext::MD_pcsections));
Expand Down Expand Up @@ -3760,6 +3788,10 @@ bool IRTranslator::runOnMachineFunction(MachineFunction &CurMF) {
#ifndef NDEBUG
Verifier.setCurrentInst(&Inst);
#endif // ifndef NDEBUG

// Translate any debug-info attached to the instruction.
translateDbgInfo(Inst, *CurBuilder.get());

if (translate(Inst))
continue;

Expand Down
1 change: 1 addition & 0 deletions llvm/test/CodeGen/AArch64/GlobalISel/combine-shift-of-shifted-dbg-value-fallback.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc < %s -global-isel -mtriple=arm64-linux-gnu -global-isel-abort=1 | FileCheck %s
; RUN: llc < %s -global-isel -mtriple=arm64-linux-gnu -global-isel-abort=1 --try-experimental-debuginfo-iterators | FileCheck %s
target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
target triple = "arm64-apple-ios9.0.0"

Expand Down
2 changes: 2 additions & 0 deletions llvm/test/CodeGen/AArch64/GlobalISel/debug-cpp.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
; RUN: llc -global-isel -mtriple=aarch64 %s -stop-after=irtranslator -o - | FileCheck %s
; RUN: llc -mtriple=aarch64 -global-isel --global-isel-abort=0 -o /dev/null
; RUN: llc -global-isel -mtriple=aarch64 %s -stop-after=irtranslator -o - --try-experimental-debuginfo-iterators | FileCheck %s
; RUN: llc -mtriple=aarch64 -global-isel --global-isel-abort=0 -o /dev/null --try-experimental-debuginfo-iterators

; struct NTCopy {
; NTCopy();
Expand Down
4 changes: 4 additions & 0 deletions llvm/test/CodeGen/AArch64/GlobalISel/debug-insts.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
; RUN: llc -global-isel -mtriple=aarch64 %s -stop-after=irtranslator -o - | FileCheck %s
; RUN: llc -mtriple=aarch64 -global-isel --global-isel-abort=0 %s -o /dev/null
; RUN: llc -mtriple=aarch64 -global-isel --global-isel-abort=0 %s -o /dev/null -debug
;
; RUN: llc -global-isel -mtriple=aarch64 %s -stop-after=irtranslator -o - --try-experimental-debuginfo-iterators | FileCheck %s
; RUN: llc -mtriple=aarch64 -global-isel --global-isel-abort=0 %s -o /dev/null --try-experimental-debuginfo-iterators
; RUN: llc -mtriple=aarch64 -global-isel --global-isel-abort=0 %s -o /dev/null -debug --try-experimental-debuginfo-iterators

; CHECK-LABEL: name: debug_declare
; CHECK: stack:
Expand Down
18 changes: 10 additions & 8 deletions llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-dilocation.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
; RUN: llc -O0 -mtriple=aarch64-apple-ios -global-isel -debug-only=irtranslator \
; RUN: -stop-after=irtranslator %s -o - 2>&1 | FileCheck %s
; RUN: llc -O0 -mtriple=aarch64-apple-ios -global-isel -debug-only=irtranslator \
; RUN: -stop-after=irtranslator %s -o - 2>&1 --try-experimental-debuginfo-iterators | FileCheck %s

; REQUIRES: asserts

; CHECK: Checking DILocation from %retval = alloca i32, align 4 was copied to G_FRAME_INDEX
; CHECK: Checking DILocation from %rv = alloca i32, align 4 was copied to G_FRAME_INDEX
; CHECK: Checking DILocation from store i32 0, ptr %retval, align 4 was copied to G_CONSTANT
; CHECK: Checking DILocation from store i32 0, ptr %retval, align 4 was copied to G_STORE
; CHECK: Checking DILocation from store i32 0, ptr %rv, align 4, !dbg !12 was copied to G_STORE debug-location !12; t.cpp:2:5
; CHECK: Checking DILocation from %0 = load i32, ptr %rv, align 4, !dbg !13 was copied to G_LOAD debug-location !13; t.cpp:3:8
; CHECK: Checking DILocation from ret i32 %0, !dbg !14 was copied to COPY debug-location !14; t.cpp:3:1
; CHECK: Checking DILocation from ret i32 %0, !dbg !14 was copied to RET_ReallyLR implicit $w0, debug-location !14; t.cpp:3:1
; CHECK: Checking DILocation from %retval = alloca i32, align 4{{.*}} was copied to G_FRAME_INDEX
; CHECK: Checking DILocation from %rv = alloca i32, align 4{{.*}} was copied to G_FRAME_INDEX
; CHECK: Checking DILocation from store i32 0, ptr %retval, align 4{{.*}} was copied to G_CONSTANT
; CHECK: Checking DILocation from store i32 0, ptr %retval, align 4{{.*}} was copied to G_STORE
; CHECK: Checking DILocation from store i32 0, ptr %rv, align 4, !dbg !12{{.*}} was copied to G_STORE debug-location !12; t.cpp:2:5
; CHECK: Checking DILocation from %0 = load i32, ptr %rv, align 4, !dbg !13{{.*}} was copied to G_LOAD debug-location !13; t.cpp:3:8
; CHECK: Checking DILocation from ret i32 %0, !dbg !14{{.*}} was copied to COPY debug-location !14; t.cpp:3:1
; CHECK: Checking DILocation from ret i32 %0, !dbg !14{{.*}} was copied to RET_ReallyLR implicit $w0, debug-location !14; t.cpp:3:1

source_filename = "t.cpp"
target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
Expand Down
1 change: 1 addition & 0 deletions llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-extract-used-by-dbg.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
; RUN: llc -O0 -stop-after=irtranslator -global-isel -verify-machineinstrs %s -o - 2>&1 | FileCheck %s
; RUN: llc -O0 -stop-after=irtranslator -global-isel -verify-machineinstrs %s -o - 2>&1 --try-experimental-debuginfo-iterators | FileCheck %s

target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
target triple = "aarch64-unknown-fuchsia"
Expand Down
1 change: 1 addition & 0 deletions llvm/test/CodeGen/X86/GlobalISel/x86-calllowering-dbg-trunc.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
; RUN: llc -mtriple=i386-linux-gnu -global-isel -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=ALL
; RUN: llc -mtriple=i386-linux-gnu -global-isel -verify-machineinstrs < %s -o - --try-experimental-debuginfo-iterators | FileCheck %s --check-prefix=ALL

; This file is the output of clang -g -O2
; int test_dbg_trunc(unsigned long long a) { return a; }
Expand Down