[llvm] Refactor/dbg salvage record components (PR #215971)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 12 23:37:40 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Eric Christopher (echristo)
<details>
<summary>Changes</summary>
(#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.
---
Full diff: https://github.com/llvm/llvm-project/pull/215971.diff
3 Files Affected:
- (modified) llvm/include/llvm/Transforms/Utils/Local.h (+14-7)
- (modified) llvm/lib/Transforms/Utils/Local.cpp (+89-80)
- (modified) llvm/unittests/Transforms/Utils/LocalTest.cpp (+68)
``````````diff
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index 493a256c2ef58..f07b2f0ba3bdd 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -317,17 +317,24 @@ 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 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
+/// 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);
+ 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 7a73cabb4c762..2d28453b7f5f6 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2030,35 +2030,36 @@ 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);
- 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);
- // Check if the salvage failed.
- if (!NewV)
+ // Keep an address we cannot salvage. If I is deleted, its remaining metadata
+ // use is replaced with poison.
+ 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");
@@ -2066,91 +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);
- }
- // salvageDebugInfoImpl should fail on examining the first element of
- // DbgUsers, or none of them.
- 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;
``````````
</details>
https://github.com/llvm/llvm-project/pull/215971
More information about the llvm-commits
mailing list