[llvm] Refactor/dbg salvage record components (PR #215971)
Eric Christopher via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 12 23:37:00 PDT 2026
https://github.com/echristo created https://github.com/llvm/llvm-project/pull/215971
(#Before Commit Remove this: Depends on #215907)
Split address and variable-location salvage into helpers so the ordering and
the kill fallback stay visible in salvageDebugInfoForDbgValues, with the
variable-location helper returning whether it processed the record. Rename the
locals and parameters the move touches to say what they hold, and use
isAddressOfVariable() for the two #dbg_declare tests, which is the same
comparison.
Replace the address helper's single-use template with DbgVariableRecord and
pass it the instruction the caller already checked, rather than recovering it
with a dyn_cast the caller's check already covers. An assert records that
precondition.
Add a unit test for the dbg.assign address path.
No regressions on check-llvm or check-lldb; ran about 80 auto-generated C
tests with 225ish locations and no differences in DW_AT_location on
aarch64 at O2.
Depends on #215907 which is stacked in this commit.
>From 4daa95f6fa469c503b725290ec9c3f637ca7bc6b Mon Sep 17 00:00:00 2001
From: Eric Christopher <echristopher at nvidia.com>
Date: Tue, 11 Aug 2026 23:19:02 -0700
Subject: [PATCH 1/2] [DebugInfo][NFC] Document debug record salvage
Document the order salvageDebugInfoForDbgValues works in: a dbg.assign address
before its variable location, stop once a variable location can't be salvaged,
and kill every supplied record when none of them were processed.
salvageDebugInfo is documented on both its declaration and its definition, and
both say uses become undef where they now become poison. Keep the header copy
and update it.
No regressions on check-llvm.
---
llvm/include/llvm/Transforms/Utils/Local.h | 19 +++++++++++++------
llvm/lib/Transforms/Utils/Local.cpp | 11 ++++++-----
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index 493a256c2ef58..e81cfd3b94ad9 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -317,14 +317,21 @@ LLVM_ABI bool replaceDbgDeclare(Value *Address, Value *NewAddress,
LLVM_ABI void replaceDbgValueForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
DIBuilder &Builder, int Offset = 0);
-/// Assuming the instruction \p I is going to be deleted, attempt to salvage
-/// debug users of \p I by writing the effect of \p I in a DIExpression. If it
-/// cannot be salvaged changes its debug uses to undef.
+/// Salvage debug records that use \p I before the instruction is deleted.
+/// Rewrite those uses in terms of its operands where we can, and encode the
+/// instruction's effect in the record's DIExpression. Deleting the instruction
+/// replaces any remaining debug-record uses with poison.
LLVM_ABI void salvageDebugInfo(Instruction &I);
-/// Implementation of salvageDebugInfo, applying only to instructions in
-/// \p Insns, rather than all debug users from findDbgUsers( \p I).
-/// Mark undef if salvaging cannot be completed.
+/// Salvage only the records in \p DPInsns instead of finding every debug
+/// user of \p I. Every record must be a debug user of the instruction.
+///
+/// Process records in order. For a dbg.assign, salvage a matching address
+/// before its variable location since replacing a variable-location operand
+/// can also replace the address. Stop when a checked variable location cannot
+/// be salvaged. A matching address counts as processed even if salvage leaves
+/// it unchanged. If nothing was processed, call setKillLocation() on every
+/// supplied record.
LLVM_ABI void
salvageDebugInfoForDbgValues(Instruction &I,
ArrayRef<DbgVariableRecord *> DPInsns);
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 7a73cabb4c762..0f96140a4b161 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2030,8 +2030,6 @@ void llvm::replaceDbgValueForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
Builder, Offset);
}
-/// Where possible to salvage debug information for \p I do so.
-/// If not possible mark undef.
void llvm::salvageDebugInfo(Instruction &I) {
SmallVector<DbgVariableRecord *, 1> DPUsers;
findDbgUsers(&I, DPUsers);
@@ -2053,7 +2051,8 @@ template <typename T> static void salvageDbgAssignAddress(T *Assign) {
SmallVector<uint64_t, 16> Ops;
Value *NewV = salvageDebugInfoImpl(*I, CurrentLocOps, Ops, AdditionalValues);
- // Check if the salvage failed.
+ // Keep an address we cannot salvage. If I is deleted, its remaining metadata
+ // use is replaced with poison.
if (!NewV)
return;
@@ -2083,6 +2082,8 @@ void llvm::salvageDebugInfoForDbgValues(Instruction &I,
bool Salvaged = false;
for (auto *DVR : DPUsers) {
+ // replaceVariableLocationOp also updates a matching dbg.assign address, so
+ // salvage the address before changing the variable location.
if (DVR->isDbgAssign()) {
if (DVR->getAddress() == &I) {
salvageDbgAssignAddress(DVR);
@@ -2120,8 +2121,8 @@ void llvm::salvageDebugInfoForDbgValues(Instruction &I,
DIExpression::appendOpsToArg(SalvagedExpr, Ops, LocNo, StackValue);
LocItr = std::find(++LocItr, DVRLocation.end(), &I);
}
- // salvageDebugInfoImpl should fail on examining the first element of
- // DbgUsers, or none of them.
+ // The failure conditions in salvageDebugInfoImpl do not depend on
+ // CurrentLocOps, so failure can only occur on the first occurrence.
if (!Op0)
break;
>From 5afbdde2a483bd48e8a241781b5cb14d5dc934ad Mon Sep 17 00:00:00 2001
From: Eric Christopher <echristopher at nvidia.com>
Date: Tue, 11 Aug 2026 23:20:46 -0700
Subject: [PATCH 2/2] [DebugInfo][NFC] Refactor debug record salvage
Split address and variable-location salvage into helpers so the ordering and
the kill fallback stay visible in salvageDebugInfoForDbgValues, with the
variable-location helper returning whether it processed the record. Rename the
locals and parameters the move touches to say what they hold, and use
isAddressOfVariable() for the two #dbg_declare tests, which is the same
comparison.
Replace the address helper's single-use template with DbgVariableRecord and
pass it the instruction the caller already checked, rather than recovering it
with a dyn_cast the caller's check already covers. An assert records that
precondition.
Add a unit test for the dbg.assign address path.
No regressions on check-llvm or check-lldb.
---
llvm/include/llvm/Transforms/Utils/Local.h | 4 +-
llvm/lib/Transforms/Utils/Local.cpp | 162 +++++++++---------
llvm/unittests/Transforms/Utils/LocalTest.cpp | 68 ++++++++
3 files changed, 155 insertions(+), 79 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index e81cfd3b94ad9..f07b2f0ba3bdd 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -323,7 +323,7 @@ LLVM_ABI void replaceDbgValueForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
/// replaces any remaining debug-record uses with poison.
LLVM_ABI void salvageDebugInfo(Instruction &I);
-/// Salvage only the records in \p DPInsns instead of finding every debug
+/// Salvage only the records in \p DbgRecords instead of finding every debug
/// user of \p I. Every record must be a debug user of the instruction.
///
/// Process records in order. For a dbg.assign, salvage a matching address
@@ -334,7 +334,7 @@ LLVM_ABI void salvageDebugInfo(Instruction &I);
/// supplied record.
LLVM_ABI void
salvageDebugInfoForDbgValues(Instruction &I,
- ArrayRef<DbgVariableRecord *> DPInsns);
+ ArrayRef<DbgVariableRecord *> DbgRecords);
/// Given an instruction \p I and DIExpression \p DIExpr operating on
/// it, append the effects of \p I to the DIExpression operand list
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 0f96140a4b161..2d28453b7f5f6 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2031,33 +2031,35 @@ void llvm::replaceDbgValueForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
}
void llvm::salvageDebugInfo(Instruction &I) {
- SmallVector<DbgVariableRecord *, 1> DPUsers;
- findDbgUsers(&I, DPUsers);
- salvageDebugInfoForDbgValues(I, DPUsers);
+ SmallVector<DbgVariableRecord *, 1> DbgRecords;
+ findDbgUsers(&I, DbgRecords);
+ salvageDebugInfoForDbgValues(I, DbgRecords);
}
-template <typename T> static void salvageDbgAssignAddress(T *Assign) {
- Instruction *I = dyn_cast<Instruction>(Assign->getAddress());
- // Only instructions can be salvaged at the moment.
- if (!I)
- return;
-
- assert(!Assign->getAddressExpression()->getFragmentInfo().has_value() &&
+/// Salvage the address of \p Assign, which the caller has checked is \p I. An
+/// address we cannot salvage stays as it is rather than stopping the caller,
+/// which counts the record as processed either way and goes on to salvage its
+/// variable location.
+static void salvageDbgAssignAddress(Instruction &I, DbgVariableRecord &Assign) {
+ assert(Assign.isDbgAssign() && Assign.getAddress() == &I &&
+ "dbg.assign must use salvaged instruction as its address");
+ assert(!Assign.getAddressExpression()->getFragmentInfo().has_value() &&
"address-expression shouldn't have fragment info");
// The address component of a dbg.assign cannot be variadic.
uint64_t CurrentLocOps = 0;
SmallVector<Value *, 4> AdditionalValues;
SmallVector<uint64_t, 16> Ops;
- Value *NewV = salvageDebugInfoImpl(*I, CurrentLocOps, Ops, AdditionalValues);
+ Value *NewAddress =
+ salvageDebugInfoImpl(I, CurrentLocOps, Ops, AdditionalValues);
// Keep an address we cannot salvage. If I is deleted, its remaining metadata
// use is replaced with poison.
- if (!NewV)
+ if (!NewAddress)
return;
DIExpression *SalvagedExpr = DIExpression::appendOpsToArg(
- Assign->getAddressExpression(), Ops, 0, /*StackValue=*/false);
+ Assign.getAddressExpression(), Ops, 0, /*StackValue=*/false);
assert(!SalvagedExpr->getFragmentInfo().has_value() &&
"address-expression shouldn't have fragment info");
@@ -2065,93 +2067,99 @@ template <typename T> static void salvageDbgAssignAddress(T *Assign) {
// Salvage succeeds if no additional values are required.
if (AdditionalValues.empty()) {
- Assign->setAddress(NewV);
- Assign->setAddressExpression(SalvagedExpr);
+ Assign.setAddress(NewAddress);
+ Assign.setAddressExpression(SalvagedExpr);
} else {
- Assign->setKillAddress();
+ Assign.setKillAddress();
}
}
-void llvm::salvageDebugInfoForDbgValues(Instruction &I,
- ArrayRef<DbgVariableRecord *> DPUsers) {
+/// Rewrite \p DVR's variable location in terms of \p I's operands. Return false
+/// and leave the record alone when the instruction cannot be salvaged. Return
+/// true once it can, including when the location ends up killed.
+static bool salvageDbgVariableLocation(Instruction &I, DbgVariableRecord &DVR) {
// These are arbitrary chosen limits on the maximum number of values and the
// maximum size of a debug expression we can salvage up to, used for
// performance reasons.
const unsigned MaxDebugArgs = 16;
const unsigned MaxExpressionSize = 128;
- bool Salvaged = false;
- for (auto *DVR : DPUsers) {
+ // Do not add DW_OP_stack_value for DbgDeclare and DbgAddr, because they
+ // are implicitly pointing out the value as a DWARF memory location
+ // description.
+ const bool StackValue = !DVR.isAddressOfVariable();
+ auto LocationOps = DVR.location_ops();
+ assert(is_contained(LocationOps, &I) &&
+ "DbgVariableRecord must use salvaged instruction as its location");
+ SmallVector<Value *, 4> AdditionalValues;
+ // 'I' may appear more than once in DVR's location ops, and each use of 'I'
+ // must be updated in the DIExpression and potentially have additional
+ // values added; thus we call salvageDebugInfoImpl for each 'I' instance in
+ // LocationOps.
+ Value *Replacement = nullptr;
+ DIExpression *SalvagedExpr = DVR.getExpression();
+ auto LocIt = find(LocationOps, &I);
+ while (SalvagedExpr && LocIt != LocationOps.end()) {
+ SmallVector<uint64_t, 16> Ops;
+ unsigned LocationIndex = std::distance(LocationOps.begin(), LocIt);
+ uint64_t CurrentLocOps = SalvagedExpr->getNumLocationOperands();
+ Replacement = salvageDebugInfoImpl(I, CurrentLocOps, Ops, AdditionalValues);
+ if (!Replacement)
+ break;
+ SalvagedExpr = DIExpression::appendOpsToArg(SalvagedExpr, Ops,
+ LocationIndex, StackValue);
+ LocIt = std::find(++LocIt, LocationOps.end(), &I);
+ }
+ // The failure conditions in salvageDebugInfoImpl do not depend on
+ // CurrentLocOps, so failure can only occur on the first occurrence.
+ if (!Replacement)
+ return false;
+
+ SalvagedExpr = SalvagedExpr->foldConstantMath();
+ DVR.replaceVariableLocationOp(&I, Replacement);
+ const bool FitsExpressionLimit =
+ SalvagedExpr->getNumElements() <= MaxExpressionSize;
+ if (AdditionalValues.empty() && FitsExpressionLimit) {
+ DVR.setExpression(SalvagedExpr);
+ } else if (!DVR.isAddressOfVariable() && FitsExpressionLimit &&
+ DVR.getNumVariableLocationOps() + AdditionalValues.size() <=
+ MaxDebugArgs) {
+ DVR.addVariableLocationOps(AdditionalValues, SalvagedExpr);
+ } else {
+ // Do not salvage using DIArgList for dbg.addr/dbg.declare, as it is
+ // currently only valid for stack value expressions.
+ // Also do not salvage if the resulting DIArgList would contain an
+ // unreasonably large number of values.
+ DVR.setKillLocation();
+ }
+ LLVM_DEBUG(dbgs() << "SALVAGE: " << DVR << '\n');
+ return true;
+}
+
+void llvm::salvageDebugInfoForDbgValues(
+ Instruction &I, ArrayRef<DbgVariableRecord *> DbgRecords) {
+ bool ProcessedAnyUse = false;
+
+ for (auto *DVR : DbgRecords) {
// replaceVariableLocationOp also updates a matching dbg.assign address, so
// salvage the address before changing the variable location.
if (DVR->isDbgAssign()) {
if (DVR->getAddress() == &I) {
- salvageDbgAssignAddress(DVR);
- Salvaged = true;
+ salvageDbgAssignAddress(I, *DVR);
+ ProcessedAnyUse = true;
}
if (DVR->getValue() != &I)
continue;
}
-
- // Do not add DW_OP_stack_value for DbgDeclare and DbgAddr, because they
- // are implicitly pointing out the value as a DWARF memory location
- // description.
- bool StackValue =
- DVR->getType() != DbgVariableRecord::LocationType::Declare;
- auto DVRLocation = DVR->location_ops();
- assert(
- is_contained(DVRLocation, &I) &&
- "DbgVariableIntrinsic must use salvaged instruction as its location");
- SmallVector<Value *, 4> AdditionalValues;
- // 'I' may appear more than once in DVR's location ops, and each use of 'I'
- // must be updated in the DIExpression and potentially have additional
- // values added; thus we call salvageDebugInfoImpl for each 'I' instance in
- // DVRLocation.
- Value *Op0 = nullptr;
- DIExpression *SalvagedExpr = DVR->getExpression();
- auto LocItr = find(DVRLocation, &I);
- while (SalvagedExpr && LocItr != DVRLocation.end()) {
- SmallVector<uint64_t, 16> Ops;
- unsigned LocNo = std::distance(DVRLocation.begin(), LocItr);
- uint64_t CurrentLocOps = SalvagedExpr->getNumLocationOperands();
- Op0 = salvageDebugInfoImpl(I, CurrentLocOps, Ops, AdditionalValues);
- if (!Op0)
- break;
- SalvagedExpr =
- DIExpression::appendOpsToArg(SalvagedExpr, Ops, LocNo, StackValue);
- LocItr = std::find(++LocItr, DVRLocation.end(), &I);
- }
- // The failure conditions in salvageDebugInfoImpl do not depend on
- // CurrentLocOps, so failure can only occur on the first occurrence.
- if (!Op0)
+ if (!salvageDbgVariableLocation(I, *DVR))
break;
-
- SalvagedExpr = SalvagedExpr->foldConstantMath();
- DVR->replaceVariableLocationOp(&I, Op0);
- bool IsValidSalvageExpr =
- SalvagedExpr->getNumElements() <= MaxExpressionSize;
- if (AdditionalValues.empty() && IsValidSalvageExpr) {
- DVR->setExpression(SalvagedExpr);
- } else if (DVR->getType() != DbgVariableRecord::LocationType::Declare &&
- IsValidSalvageExpr &&
- DVR->getNumVariableLocationOps() + AdditionalValues.size() <=
- MaxDebugArgs) {
- DVR->addVariableLocationOps(AdditionalValues, SalvagedExpr);
- } else {
- // Do not salvage using DIArgList for dbg.addr/dbg.declare, as it is
- // currently only valid for stack value expressions.
- // Also do not salvage if the resulting DIArgList would contain an
- // unreasonably large number of values.
- DVR->setKillLocation();
- }
- LLVM_DEBUG(dbgs() << "SALVAGE: " << *DVR << '\n');
- Salvaged = true;
+ ProcessedAnyUse = true;
}
- if (Salvaged)
+ if (ProcessedAnyUse)
return;
- for (auto *DVR : DPUsers)
+ for (auto *DVR : DbgRecords)
DVR->setKillLocation();
}
diff --git a/llvm/unittests/Transforms/Utils/LocalTest.cpp b/llvm/unittests/Transforms/Utils/LocalTest.cpp
index 3c17687c11c8f..9656cf26f73f5 100644
--- a/llvm/unittests/Transforms/Utils/LocalTest.cpp
+++ b/llvm/unittests/Transforms/Utils/LocalTest.cpp
@@ -685,6 +685,74 @@ TEST(Local, FindDbgRecords) {
EXPECT_EQ(Records.size(), 1u);
}
+TEST(Local, SalvageDbgAssignAddress) {
+ // Salvage rewrites a dbg_assign's address through the address expression,
+ // which is separate from the expression on the variable location. Giving the
+ // record a constant value keeps salvage off the variable location, so the
+ // address is the only thing that moves.
+ //
+ // The GEP index is a constant since a constant offset needs no extra location
+ // operands, and that's what makes salvage keep the salvaged address rather
+ // than kill it.
+ //
+ // assignment-tracking/salvage-value.ll also covers this path end to end.
+ LLVMContext Ctx;
+ std::unique_ptr<Module> M = parseIR(Ctx,
+ R"(
+ define dso_local void @fun(ptr %a) !dbg !11 {
+ entry:
+ %arrayidx = getelementptr inbounds i32, ptr %a, i64 1
+ #dbg_assign(i32 0, !16, !DIExpression(), !15, ptr %arrayidx, !DIExpression(), !19)
+ ret void
+ }
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!2, !3, !9}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+ !1 = !DIFile(filename: "test.cpp", directory: "/")
+ !2 = !{i32 7, !"Dwarf Version", i32 5}
+ !3 = !{i32 2, !"Debug Info Version", i32 3}
+ !9 = !{i32 7, !"debug-info-assignment-tracking", i1 true}
+ !11 = distinct !DISubprogram(name: "fun", linkageName: "fun", scope: !1, file: !1, line: 1, type: !12, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !14)
+ !12 = !DISubroutineType(types: !13)
+ !13 = !{null}
+ !14 = !{}
+ !15 = distinct !DIAssignID()
+ !16 = !DILocalVariable(name: "x", scope: !11, file: !1, line: 2, type: !18)
+ !18 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+ !19 = !DILocation(line: 0, scope: !11)
+ )");
+
+ bool BrokenDebugInfo = true;
+ verifyModule(*M, &errs(), &BrokenDebugInfo);
+ ASSERT_FALSE(BrokenDebugInfo);
+
+ Function &Fun = *cast<Function>(M->getNamedValue("fun"));
+ Value *Arg = Fun.getArg(0);
+ Instruction &GEP = *Fun.getEntryBlock().getFirstNonPHIOrDbg();
+
+ SmallVector<DbgVariableRecord *> Records;
+ findDbgUsers(&GEP, Records);
+ ASSERT_EQ(Records.size(), 1u);
+ DbgVariableRecord *Assign = Records[0];
+ ASSERT_TRUE(Assign->isDbgAssign());
+ ASSERT_EQ(Assign->getAddress(), &GEP);
+
+ salvageDebugInfo(GEP);
+
+ // The address points at the GEP's pointer operand and the constant offset
+ // moved into the address expression. i32 at index 1 is 4 bytes in.
+ EXPECT_EQ(Assign->getAddress(), Arg);
+ EXPECT_EQ(Assign->getAddressExpression()->getNumElements(), 2u);
+ EXPECT_EQ(Assign->getAddressExpression()->getElement(0),
+ dwarf::DW_OP_plus_uconst);
+ EXPECT_EQ(Assign->getAddressExpression()->getElement(1), 4u);
+ // The variable location is a constant, so salvage left it alone.
+ EXPECT_EQ(Assign->getNumVariableLocationOps(), 1u);
+ EXPECT_TRUE(isa<ConstantInt>(Assign->getVariableLocationOp(0)));
+}
+
TEST(Local, ReplaceAllDbgUsesWith) {
using namespace llvm::dwarf;
LLVMContext Ctx;
More information about the llvm-commits
mailing list