Skip to content

Commit df69c69

Browse files
SLTozerSLTozer
committed
[DebugInfo] Handle multiple variable location operands in IR
This patch updates the various IR passes to correctly handle dbg.values with a DIArgList location. This patch does not actually allow DIArgLists to be produced by salvageDebugInfo, and it does not affect any pass after codegen-prepare. Other than that, it should cover every IR pass. Most of the changes simply extend code that operated on a single debug value to operate on the list of debug values in the style of any_of, all_of, for_each, etc. Instances of setOperand(0, ...) have been replaced with with replaceVariableLocationOp, which takes the value that is being replaced as an additional argument. In places where this value isn't readily available, we have to track the old value through to the point where it gets replaced. Differential Revision: https://reviews.llvm.org/D88232
1 parent 2986a9c commit df69c69

File tree

19 files changed

+284
-147
lines changed

19 files changed

+284
-147
lines changed

‎llvm/include/llvm/IR/IntrinsicInst.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ class DbgVariableIntrinsic : public DbgInfoIntrinsic {
203203
Value *getVariableLocationOp(unsigned OpIdx) const;
204204

205205
void replaceVariableLocationOp(Value *OldValue, Value *NewValue);
206+
void replaceVariableLocationOp(unsigned OpIdx, Value *NewValue);
206207

207208
void setVariable(DILocalVariable *NewVar) {
208209
setArgOperand(1, MetadataAsValue::get(NewVar->getContext(), NewVar));

‎llvm/include/llvm/Transforms/Utils/Local.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,10 @@ void salvageDebugInfoForDbgValues(Instruction &I,
311311
/// Given an instruction \p I and DIExpression \p DIExpr operating on it, write
312312
/// the effects of \p I into the returned DIExpression, or return nullptr if
313313
/// it cannot be salvaged. \p StackVal: whether DW_OP_stack_value should be
314-
/// appended to the expression.
314+
/// appended to the expression. \p LocNo: the index of the location operand to
315+
/// which \p I applies, should be 0 for debug info without a DIArgList.
315316
DIExpression *salvageDebugInfoImpl(Instruction &I, DIExpression *DIExpr,
316-
bool StackVal);
317+
bool StackVal, unsigned LocNo);
317318

318319
/// Point debug users of \p From to \p To or salvage them. Use this function
319320
/// only when replacing all uses of \p From with \p To, with a guarantee that

‎llvm/lib/CodeGen/CodeGenPrepare.cpp

Lines changed: 63 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2855,11 +2855,16 @@ class TypePromotionTransaction {
28552855
/// Keep track of the debug users.
28562856
SmallVector<DbgValueInst *, 1> DbgValues;
28572857

2858+
/// Keep track of the new value so that we can undo it by replacing
2859+
/// instances of the new value with the original value.
2860+
Value *New;
2861+
28582862
using use_iterator = SmallVectorImpl<InstructionAndIdx>::iterator;
28592863

28602864
public:
28612865
/// Replace all the use of \p Inst by \p New.
2862-
UsesReplacer(Instruction *Inst, Value *New) : TypePromotionAction(Inst) {
2866+
UsesReplacer(Instruction *Inst, Value *New)
2867+
: TypePromotionAction(Inst), New(New) {
28632868
LLVM_DEBUG(dbgs() << "Do: UsersReplacer: " << *Inst << " with " << *New
28642869
<< "\n");
28652870
// Record the original uses.
@@ -2885,7 +2890,7 @@ class TypePromotionTransaction {
28852890
// the original debug uses must also be reinstated to maintain the
28862891
// correctness and utility of debug value instructions.
28872892
for (auto *DVI : DbgValues)
2888-
DVI->replaceVariableLocationOp(DVI->getVariableLocationOp(0), Inst);
2893+
DVI->replaceVariableLocationOp(New, Inst);
28892894
}
28902895
};
28912896

@@ -7876,18 +7881,21 @@ bool CodeGenPrepare::fixupDbgValue(Instruction *I) {
78767881
DbgValueInst &DVI = *cast<DbgValueInst>(I);
78777882

78787883
// Does this dbg.value refer to a sunk address calculation?
7879-
Value *Location = DVI.getVariableLocationOp(0);
7880-
WeakTrackingVH SunkAddrVH = SunkAddrs[Location];
7881-
Value *SunkAddr = SunkAddrVH.pointsToAliveValue() ? SunkAddrVH : nullptr;
7882-
if (SunkAddr) {
7883-
// Point dbg.value at locally computed address, which should give the best
7884-
// opportunity to be accurately lowered. This update may change the type of
7885-
// pointer being referred to; however this makes no difference to debugging
7886-
// information, and we can't generate bitcasts that may affect codegen.
7887-
DVI.replaceVariableLocationOp(Location, SunkAddr);
7888-
return true;
7889-
}
7890-
return false;
7884+
bool AnyChange = false;
7885+
for (Value *Location : DVI.getValues()) {
7886+
WeakTrackingVH SunkAddrVH = SunkAddrs[Location];
7887+
Value *SunkAddr = SunkAddrVH.pointsToAliveValue() ? SunkAddrVH : nullptr;
7888+
if (SunkAddr) {
7889+
// Point dbg.value at locally computed address, which should give the best
7890+
// opportunity to be accurately lowered. This update may change the type
7891+
// of pointer being referred to; however this makes no difference to
7892+
// debugging information, and we can't generate bitcasts that may affect
7893+
// codegen.
7894+
DVI.replaceVariableLocationOp(Location, SunkAddr);
7895+
AnyChange = true;
7896+
}
7897+
}
7898+
return AnyChange;
78917899
}
78927900

78937901
// A llvm.dbg.value may be using a value before its definition, due to
@@ -7906,30 +7914,51 @@ bool CodeGenPrepare::placeDbgValues(Function &F) {
79067914
if (!DVI)
79077915
continue;
79087916

7909-
Instruction *VI = dyn_cast_or_null<Instruction>(DVI->getValue());
7917+
SmallVector<Instruction *, 4> VIs;
7918+
for (Value *V : DVI->getValues())
7919+
if (Instruction *VI = dyn_cast_or_null<Instruction>(V))
7920+
VIs.push_back(VI);
7921+
7922+
// This DVI may depend on multiple instructions, complicating any
7923+
// potential sink. This block takes the defensive approach, opting to
7924+
// "undef" the DVI if it has more than one instruction and any of them do
7925+
// not dominate DVI.
7926+
for (Instruction *VI : VIs) {
7927+
if (VI->isTerminator())
7928+
continue;
79107929

7911-
if (!VI || VI->isTerminator())
7912-
continue;
7930+
// If VI is a phi in a block with an EHPad terminator, we can't insert
7931+
// after it.
7932+
if (isa<PHINode>(VI) && VI->getParent()->getTerminator()->isEHPad())
7933+
continue;
79137934

7914-
// If VI is a phi in a block with an EHPad terminator, we can't insert
7915-
// after it.
7916-
if (isa<PHINode>(VI) && VI->getParent()->getTerminator()->isEHPad())
7917-
continue;
7935+
// If the defining instruction dominates the dbg.value, we do not need
7936+
// to move the dbg.value.
7937+
if (DT.dominates(VI, DVI))
7938+
continue;
79187939

7919-
// If the defining instruction dominates the dbg.value, we do not need
7920-
// to move the dbg.value.
7921-
if (DT.dominates(VI, DVI))
7922-
continue;
7940+
// If we depend on multiple instructions and any of them doesn't
7941+
// dominate this DVI, we probably can't salvage it: moving it to
7942+
// after any of the instructions could cause us to lose the others.
7943+
if (VIs.size() > 1) {
7944+
LLVM_DEBUG(
7945+
dbgs()
7946+
<< "Unable to find valid location for Debug Value, undefing:\n"
7947+
<< *DVI);
7948+
DVI->setUndef();
7949+
break;
7950+
}
79237951

7924-
LLVM_DEBUG(dbgs() << "Moving Debug Value before :\n"
7925-
<< *DVI << ' ' << *VI);
7926-
DVI->removeFromParent();
7927-
if (isa<PHINode>(VI))
7928-
DVI->insertBefore(&*VI->getParent()->getFirstInsertionPt());
7929-
else
7930-
DVI->insertAfter(VI);
7931-
MadeChange = true;
7932-
++NumDbgValueMoved;
7952+
LLVM_DEBUG(dbgs() << "Moving Debug Value before :\n"
7953+
<< *DVI << ' ' << *VI);
7954+
DVI->removeFromParent();
7955+
if (isa<PHINode>(VI))
7956+
DVI->insertBefore(&*VI->getParent()->getFirstInsertionPt());
7957+
else
7958+
DVI->insertAfter(VI);
7959+
MadeChange = true;
7960+
++NumDbgValueMoved;
7961+
}
79337962
}
79347963
}
79357964
return MadeChange;

‎llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,7 +1251,8 @@ void SelectionDAGBuilder::salvageUnresolvedDbgValue(DanglingDebugInfo &DDI) {
12511251
// variable. FIXME: Further work could recover those too.
12521252
while (isa<Instruction>(V)) {
12531253
Instruction &VAsInst = *cast<Instruction>(V);
1254-
DIExpression *NewExpr = salvageDebugInfoImpl(VAsInst, Expr, StackValue);
1254+
// Temporary "0", awaiting real implementation.
1255+
DIExpression *NewExpr = salvageDebugInfoImpl(VAsInst, Expr, StackValue, 0);
12551256

12561257
// If we cannot salvage any further, and haven't yet found a suitable debug
12571258
// expression, bail out.
@@ -6044,7 +6045,7 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
60446045
DILocalVariable *Variable = DI.getVariable();
60456046
DIExpression *Expression = DI.getExpression();
60466047
dropDanglingDebugInfo(Variable, Expression);
6047-
SmallVector<Value *> Values(DI.getValues());
6048+
SmallVector<Value *, 4> Values(DI.getValues());
60486049
if (Values.empty())
60496050
return;
60506051

‎llvm/lib/IR/IntrinsicInst.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,24 @@ void DbgVariableIntrinsic::replaceVariableLocationOp(Value *OldValue,
9898
setArgOperand(
9999
0, MetadataAsValue::get(getContext(), DIArgList::get(getContext(), MDs)));
100100
}
101+
void DbgVariableIntrinsic::replaceVariableLocationOp(unsigned OpIdx,
102+
Value *NewValue) {
103+
assert(OpIdx < getNumVariableLocationOps() && "Invalid Operand Index");
104+
if (!hasArgList()) {
105+
Value *NewOperand = isa<MetadataAsValue>(NewValue)
106+
? NewValue
107+
: MetadataAsValue::get(
108+
getContext(), ValueAsMetadata::get(NewValue));
109+
return setArgOperand(0, NewOperand);
110+
}
111+
SmallVector<ValueAsMetadata *, 4> MDs;
112+
ValueAsMetadata *NewOperand = getAsMetadata(NewValue);
113+
for (unsigned Idx = 0; Idx < getNumVariableLocationOps(); ++Idx)
114+
MDs.push_back(Idx == OpIdx ? NewOperand
115+
: getAsMetadata(getVariableLocationOp(Idx)));
116+
setArgOperand(
117+
0, MetadataAsValue::get(getContext(), DIArgList::get(getContext(), MDs)));
118+
}
101119

102120
Optional<uint64_t> DbgVariableIntrinsic::getFragmentSizeInBits() const {
103121
if (auto Fragment = getExpression()->getFragmentInfo())

‎llvm/lib/IR/User.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ void User::replaceUsesOfWith(Value *From, Value *To) {
3131
// most importantly, removing "this" from the use list of "From".
3232
setOperand(i, To);
3333
}
34+
if (auto DVI = dyn_cast_or_null<DbgVariableIntrinsic>(this)) {
35+
if (is_contained(DVI->location_ops(), From))
36+
DVI->replaceVariableLocationOp(From, To);
37+
}
3438
}
3539

3640
//===----------------------------------------------------------------------===//

‎llvm/lib/Target/AArch64/AArch64StackTagging.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -563,10 +563,9 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) {
563563
}
564564

565565
if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(I)) {
566-
if (auto *AI =
567-
dyn_cast_or_null<AllocaInst>(DVI->getVariableLocationOp(0))) {
568-
Allocas[AI].DbgVariableIntrinsics.push_back(DVI);
569-
}
566+
for (Value *V : DVI->location_ops())
567+
if (auto *AI = dyn_cast_or_null<AllocaInst>(V))
568+
Allocas[AI].DbgVariableIntrinsics.push_back(DVI);
570569
continue;
571570
}
572571

‎llvm/lib/Transforms/Coroutines/CoroFrame.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2173,7 +2173,7 @@ void coro::salvageDebugInfo(
21732173
Storage = StInst->getOperand(0);
21742174
} else if (auto *GEPInst = dyn_cast<GetElementPtrInst>(Storage)) {
21752175
Expr = llvm::salvageDebugInfoImpl(*GEPInst, Expr,
2176-
/*WithStackValue=*/false);
2176+
/*WithStackValue=*/false, 0);
21772177
if (!Expr)
21782178
return;
21792179
Storage = GEPInst->getOperand(0);

‎llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3576,23 +3576,14 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
35763576
llvm::sort(DbgUsersToSink,
35773577
[](auto *A, auto *B) { return B->comesBefore(A); });
35783578

3579-
// Update the arguments of a dbg.declare instruction, so that it
3580-
// does not point into a sunk instruction.
3581-
auto updateDbgDeclare = [](DbgVariableIntrinsic *DII) {
3582-
if (!isa<DbgDeclareInst>(DII))
3583-
return false;
3584-
3585-
return true;
3586-
};
3587-
35883579
SmallVector<DbgVariableIntrinsic *, 2> DIIClones;
35893580
SmallSet<DebugVariable, 4> SunkVariables;
35903581
for (auto User : DbgUsersToSink) {
35913582
// A dbg.declare instruction should not be cloned, since there can only be
35923583
// one per variable fragment. It should be left in the original place
35933584
// because the sunk instruction is not an alloca (otherwise we could not be
35943585
// here).
3595-
if (updateDbgDeclare(User))
3586+
if (isa<DbgDeclareInst>(User))
35963587
continue;
35973588

35983589
DebugVariable DbgUserVariable =
@@ -3603,6 +3594,8 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
36033594
continue;
36043595

36053596
DIIClones.emplace_back(cast<DbgVariableIntrinsic>(User->clone()));
3597+
if (isa<DbgDeclareInst>(User) && isa<CastInst>(I))
3598+
DIIClones.back()->replaceVariableLocationOp(I, I->getOperand(0));
36063599
LLVM_DEBUG(dbgs() << "CLONE: " << *DIIClones.back() << '\n');
36073600
}
36083601

‎llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,10 +1218,10 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) {
12181218
isa<CleanupReturnInst>(Inst))
12191219
RetVec.push_back(&Inst);
12201220

1221-
if (auto *DDI = dyn_cast<DbgVariableIntrinsic>(&Inst))
1222-
if (auto *Alloca =
1223-
dyn_cast_or_null<AllocaInst>(DDI->getVariableLocationOp(0)))
1224-
AllocaDbgMap[Alloca].push_back(DDI);
1221+
if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst))
1222+
for (Value *V : DVI->location_ops())
1223+
if (auto *Alloca = dyn_cast_or_null<AllocaInst>(V))
1224+
AllocaDbgMap[Alloca].push_back(DVI);
12251225

12261226
if (InstrumentLandingPads && isa<LandingPadInst>(Inst))
12271227
LandingPadVec.push_back(&Inst);
@@ -1297,13 +1297,18 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) {
12971297
}
12981298

12991299
if (!AllocaToPaddedAllocaMap.empty()) {
1300-
for (auto &BB : F)
1301-
for (auto &Inst : BB)
1302-
if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst))
1303-
if (auto *AI =
1304-
dyn_cast_or_null<AllocaInst>(DVI->getVariableLocationOp(0)))
1305-
if (auto *NewAI = AllocaToPaddedAllocaMap.lookup(AI))
1306-
DVI->replaceVariableLocationOp(AI, NewAI);
1300+
for (auto &BB : F) {
1301+
for (auto &Inst : BB) {
1302+
if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst)) {
1303+
for (Value *V : DVI->location_ops()) {
1304+
if (auto *AI = dyn_cast_or_null<AllocaInst>(V)) {
1305+
if (auto *NewAI = AllocaToPaddedAllocaMap.lookup(AI))
1306+
DVI->replaceVariableLocationOp(V, NewAI);
1307+
}
1308+
}
1309+
}
1310+
}
1311+
}
13071312
for (auto &P : AllocaToPaddedAllocaMap)
13081313
P.first->eraseFromParent();
13091314
}

‎llvm/lib/Transforms/Scalar/ADCE.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -521,10 +521,14 @@ bool AggressiveDeadCodeElimination::removeDeadInstructions() {
521521
// If intrinsic is pointing at a live SSA value, there may be an
522522
// earlier optimization bug: if we know the location of the variable,
523523
// why isn't the scope of the location alive?
524-
if (Value *V = DII->getVariableLocationOp(0))
525-
if (Instruction *II = dyn_cast<Instruction>(V))
526-
if (isLive(II))
524+
for (Value *V : DII->location_ops()) {
525+
if (Instruction *II = dyn_cast<Instruction>(V)) {
526+
if (isLive(II)) {
527527
dbgs() << "Dropping debug info for " << *DII << "\n";
528+
break;
529+
}
530+
}
531+
}
528532
}
529533
}
530534
});

0 commit comments

Comments
 (0)