[DebugInfo][RemoveDIs] Handle non-instr debug-info in GlobalISel #75228
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The RemoveDIs project is aiming to eliminate debug intrinsics like dbg.value and dbg.declare from LLVM, and replace them with DPValue objects attached to instructions. ISel is one of the "terminals" where that information needs to be converted into MIR format: this patch implements support for that in GlobalISel. We aim for the output of LLVM to be identical with/without RemoveDIs debug-info. This patch should be NFC, as we're handling the same data about varaibles stored in a different format -- it now appears in a DPValue object rather than as an intrinsic. To that end, I've refactored the handling of dbg.values into a dedicated function, and call it whenever a dbg.value or a DPValue is encountered. Right now only dbg.value intrinsics are converted to DPValues, but shortly so will dbg.declares. Hence I've left some room for folding common code for dbg.values and dbg.declares into one function, with a "isDeclare" argument to disambiguate the two. Testing: adding the --try-experimental-debuginfo-iterators switch to llc causes it to try and convert to the "new" debug-info format if it's built in (LLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS=On), and it'll be covered by our buildbot. One test has a few extra wildcard-regexes added: this is because there's some extra data printed about attached debug-info, which is safe to ignore.
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-debuginfo Author: Jeremy Morse (jmorse) ChangesThe RemoveDIs project is aiming to eliminate debug intrinsics like dbg.value and dbg.declare from LLVM, and replace them with DPValue objects attached to instructions. ISel is one of the "terminals" where that information needs to be converted into MIR format: this patch implements support for that in GlobalISel. We aim for the output of LLVM to be identical with/without RemoveDIs debug-info. This patch should be NFC, as we're handling the same data about variables stored in a different format -- it now appears in a DPValue object rather than as an intrinsic. To that end, I've refactored the handling of dbg.values into a dedicated function, and call it whenever a dbg.value or a DPValue is encountered. Right now only dbg.value intrinsics are converted to DPValues, but shortly so will dbg.declares. Hence I've left some room for folding common code for dbg.values and dbg.declares into one function, with a "isDeclare" argument to disambiguate the two. Testing: adding the --try-experimental-debuginfo-iterators switch to llc causes it to try and convert to the "new" debug-info format if it's built in (LLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS=On), and it'll be covered by our buildbot. One test has a few extra wildcard-regexes added: this is because there's some extra data printed about attached debug-info, which is safe to ignore. Full diff: https://github.com/llvm/llvm-project/pull/75228.diff 8 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h b/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
index bffc03ed0187e6..51c7c262822c35 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
@@ -204,6 +204,19 @@ 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: either a dbg.declare or dbg.value
+ /// equivalent. Pass in all the contents of the record, rather than relying
+ /// on how it's stored.
+ void translateDbgRecord(bool isDeclare, Value *V, 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);
@@ -250,14 +263,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);
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 27a53e55f32fa3..40c925653258a1 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -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())) &&
"Line info was not transferred to all instructions");
}
};
@@ -1927,47 +1928,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;
}
@@ -2040,7 +2029,9 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
return true;
}
- if (translateIfEntryValueArgument(DI))
+ if (translateIfEntryValueArgument(true, DI.getAddress(), DI.getVariable(),
+ DI.getExpression(), DI.getDebugLoc(),
+ MIRBuilder))
return true;
// A dbg.declare describes the address of a source variable, so lower it
@@ -2079,41 +2070,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());
- }
+ translateDbgRecord(false, DI.getValue(), DI.hasArgList(), DI.getVariable(),
+ DI.getExpression(), DI.getDebugLoc(), MIRBuilder);
return true;
}
case Intrinsic::uadd_with_overflow:
@@ -3159,6 +3117,65 @@ void IRTranslator::finishPendingPhis() {
}
}
+void IRTranslator::translateDbgRecord(bool isDeclare, 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(isDeclare, 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::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);
+ // NB: support dbg.declares.
+ translateDbgRecord(false, 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));
@@ -3669,6 +3686,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;
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/combine-shift-of-shifted-dbg-value-fallback.ll b/llvm/test/CodeGen/AArch64/GlobalISel/combine-shift-of-shifted-dbg-value-fallback.ll
index adaf54cbc96208..930d39dfa298be 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/combine-shift-of-shifted-dbg-value-fallback.ll
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/combine-shift-of-shifted-dbg-value-fallback.ll
@@ -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"
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/debug-cpp.ll b/llvm/test/CodeGen/AArch64/GlobalISel/debug-cpp.ll
index cb4a01cbcf092a..485ab18ec7db9c 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/debug-cpp.ll
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/debug-cpp.ll
@@ -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();
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/debug-insts.ll b/llvm/test/CodeGen/AArch64/GlobalISel/debug-insts.ll
index 47ddca8fd05779..960ea4e5b9f42e 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/debug-insts.ll
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/debug-insts.ll
@@ -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:
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-dilocation.ll b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-dilocation.ll
index a8fc761a3a533b..fc17a62a4d7041 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-dilocation.ll
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-dilocation.ll
@@ -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"
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-extract-used-by-dbg.ll b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-extract-used-by-dbg.ll
index 9f398b4a9d3b1e..1304747789f2a5 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-extract-used-by-dbg.ll
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-extract-used-by-dbg.ll
@@ -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"
diff --git a/llvm/test/CodeGen/X86/GlobalISel/x86-calllowering-dbg-trunc.ll b/llvm/test/CodeGen/X86/GlobalISel/x86-calllowering-dbg-trunc.ll
index 90c9290a83bbd4..f9c34d3b0bb277 100644
--- a/llvm/test/CodeGen/X86/GlobalISel/x86-calllowering-dbg-trunc.ll
+++ b/llvm/test/CodeGen/X86/GlobalISel/x86-calllowering-dbg-trunc.ll
@@ -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; }
|
You can test this locally with the following command:git-clang-format --diff f188f4589cc8f690779c62996520663718df106d dfbad2c75a847b7dcb9abc4b72ec15a6a62460bf -- llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h llvm/lib/CodeGen/GlobalISel/IRTranslator.cppView the diff from clang-format here.diff --git a/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h b/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
index 5454df0291..8c65be7c9b 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
@@ -206,24 +206,25 @@ private:
/// Examine any debug-info attached to the instruction (in the form of
/// DPValues) and translate it.
- void translateDbgInfo(const Instruction &Inst,
- MachineIRBuilder &MIRBuilder);
+ 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);
+ 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);
+ 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,
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 1a71c1232c..e8fc7d707f 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2089,8 +2089,9 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
case Intrinsic::dbg_declare: {
const DbgDeclareInst &DI = cast<DbgDeclareInst>(CI);
assert(DI.getVariable() && "Missing variable");
- translateDbgDeclareRecord(DI.getAddress(), DI.hasArgList(), DI.getVariable(),
- DI.getExpression(), DI.getDebugLoc(), MIRBuilder);
+ translateDbgDeclareRecord(DI.getAddress(), DI.hasArgList(),
+ DI.getVariable(), DI.getExpression(),
+ DI.getDebugLoc(), MIRBuilder);
return true;
}
case Intrinsic::dbg_label: {
@@ -2124,7 +2125,7 @@ bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
// This form of DBG_VALUE is target-independent.
const DbgValueInst &DI = cast<DbgValueInst>(CI);
translateDbgValueRecord(DI.getValue(), DI.hasArgList(), DI.getVariable(),
- DI.getExpression(), DI.getDebugLoc(), MIRBuilder);
+ DI.getExpression(), DI.getDebugLoc(), MIRBuilder);
return true;
}
case Intrinsic::uadd_with_overflow:
@@ -3183,10 +3184,10 @@ void IRTranslator::finishPendingPhis() {
}
void IRTranslator::translateDbgValueRecord(Value *V, bool HasArgList,
- const DILocalVariable *Variable,
- const DIExpression *Expression,
- const DebugLoc &DL,
- MachineIRBuilder &MIRBuilder) {
+ 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.
@@ -3230,10 +3231,10 @@ void IRTranslator::translateDbgValueRecord(Value *V, bool HasArgList,
}
void IRTranslator::translateDbgDeclareRecord(Value *Address, bool HasArgList,
- const DILocalVariable *Variable,
- const DIExpression *Expression,
- const DebugLoc &DL,
- MachineIRBuilder &MIRBuilder) {
+ 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;
@@ -3245,36 +3246,35 @@ void IRTranslator::translateDbgDeclareRecord(Value *Address, bool HasArgList,
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);
+ MF->setVariableDbgInfo(Variable, Expression, getOrCreateFrameIndex(*AI),
+ DL);
return;
}
- if (translateIfEntryValueArgument(true, Address, Variable,
- Expression, DL,
+ 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);
+ MIRBuilder.buildIndirectDbgValue(getOrCreateVReg(*Address), Variable,
+ Expression);
return;
}
void IRTranslator::translateDbgInfo(const Instruction &Inst,
- MachineIRBuilder &MIRBuilder) {
+ 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);
+ translateDbgDeclareRecord(V, DPV.hasArgList(), Variable, Expression,
+ DPV.getDebugLoc(), MIRBuilder);
else
- translateDbgValueRecord(V, DPV.hasArgList(), Variable,
- Expression, DPV.getDebugLoc(), MIRBuilder);
+ translateDbgValueRecord(V, DPV.hasArgList(), Variable, Expression,
+ DPV.getDebugLoc(), MIRBuilder);
}
}
|
SLTozer
approved these changes
Dec 13, 2023
| /// Translate a debug-info record: either a dbg.declare or dbg.value | ||
| /// equivalent. Pass in all the contents of the record, rather than relying | ||
| /// on how it's stored. | ||
| void translateDbgRecord(bool isDeclare, Value *V, bool hasArgList, |
There was a problem hiding this comment.
Very minor nit, should these args be capitalized?
| @@ -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())) && | |||
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
Good point; we should only open a gap in this check for the things that are absolutely necessary, I'll refine it.
There was a problem hiding this comment.
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.
arsenm
reviewed
Jan 17, 2024
| 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") |
There was a problem hiding this comment.
Excess parens around isDeclare
|
I ran into difficulties when testing this before landing; as dbg.declares now get converted to DPValues there are some additional code paths that need conversion. I've merged master (no idea how github will be presenting that...) and put the refactor for dbg.declares in dfbad2c. I was originally intending on having translateDbgRecord a common facility for dbg.declare/dbg.value equivalents, however it looks like their implementation will be too different, hence separating them. CC @OCHyams if you could take a look. |
|
Thanks -- I'm going to land this manually seeing how I don't trust githubs handling of squashing merge commits |
jmorse
added a commit
that referenced
this pull request
Jan 23, 2024
) The RemoveDIs project is aiming to eliminate debug intrinsics like dbg.value and dbg.declare from LLVM, and replace them with DPValue objects attached to instructions. ISel is one of the "terminals" where that information needs to be converted into MIR format: this patch implements support for that in GlobalISel. We aim for the output of LLVM to be identical with/without RemoveDIs debug-info. This patch should be NFC, as we're handling the same data about variables stored in a different format -- it now appears in a DPValue object rather than as an intrinsic. To that end, I've refactored the handling of dbg.values into a dedicated function, and call it whenever a dbg.value or a DPValue is encountered. dbg.declare is handled in a similar way. Testing: adding the --try-experimental-debuginfo-iterators switch to llc causes it to try and convert to the "new" debug-info format if it's built in (LLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS=On), and it'll be covered by our buildbot. One test has a few extra wildcard-regexes added: this is because there's some extra data printed about attached debug-info, which is safe to ignore.
|
Committed in 0871722 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The RemoveDIs project is aiming to eliminate debug intrinsics like dbg.value and dbg.declare from LLVM, and replace them with DPValue objects attached to instructions. ISel is one of the "terminals" where that information needs to be converted into MIR format: this patch implements support for that in GlobalISel. We aim for the output of LLVM to be identical with/without RemoveDIs debug-info.
This patch should be NFC, as we're handling the same data about variables stored in a different format -- it now appears in a DPValue object rather than as an intrinsic. To that end, I've refactored the handling of dbg.values into a dedicated function, and call it whenever a dbg.value or a DPValue is encountered.
Right now only dbg.value intrinsics are converted to DPValues, but shortly so will dbg.declares. Hence I've left some room for folding common code for dbg.values and dbg.declares into one function, with a "isDeclare" argument to disambiguate the two.
Testing: adding the --try-experimental-debuginfo-iterators switch to llc causes it to try and convert to the "new" debug-info format if it's built in (LLVM_EXPERIMENTAL_DEBUGINFO_ITERATORS=On), and it'll be covered by our buildbot. One test has a few extra wildcard-regexes added: this is because there's some extra data printed about attached debug-info, which is safe to ignore.