Skip to content

Commit c3bb2ef

Browse files
SLTozerSLTozer
committed
[DLCov][NFC] Propagate annotated DebugLocs through transformations
In order for DebugLoc coverage testing to work, we have to firstly set annotations for intentionally-empty DebugLocs, and secondly we have to ensure that we do not drop these annotations as we propagate DebugLocs through the compiler. As the annotations exist as part of the DebugLoc class, and not DILocation, they will not survive a DebugLoc->DILocation->DebugLoc roundtrip. Therefore this patch modifies a number of places in the compiler to propagate DebugLocs directly rather than via the underlying DILocation. This has no effect on normal builds; it only ensures that during coverage builds, we do not drop annotations and therefore create false positives. The bulk of these changes are in replacing DILocation::getMergedLocation(s) with a DebugLoc equivalent, and in changing the IRBuilder to store a DebugLoc directly rather than storing DILocations in its general Metadata array.
1 parent 92195f6 commit c3bb2ef

20 files changed

+101
-51
lines changed

‎llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ class LegalizationArtifactCombiner {
100100
const LLT DstTy = MRI.getType(DstReg);
101101
if (isInstLegal({TargetOpcode::G_CONSTANT, {DstTy}})) {
102102
auto &CstVal = SrcMI->getOperand(1);
103-
auto *MergedLocation = DILocation::getMergedLocation(
104-
MI.getDebugLoc().get(), SrcMI->getDebugLoc().get());
103+
auto MergedLocation =
104+
DebugLoc::getMergedLocation(MI.getDebugLoc(), SrcMI->getDebugLoc());
105105
// Set the debug location to the merged location of the SrcMI and the MI
106106
// if the aext fold is successful.
107107
Builder.setDebugLoc(MergedLocation);

‎llvm/include/llvm/IR/DebugLoc.h

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,31 @@ namespace llvm {
141141
static inline DebugLoc getDropped() { return DebugLoc(); }
142142
#endif // LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
143143

144+
static DebugLoc getMergedLocations(ArrayRef<DebugLoc> Locs);
145+
static DebugLoc getMergedLocation(DebugLoc LocA, DebugLoc LocB);
146+
147+
/// If this DebugLoc is non-empty, returns this DebugLoc; otherwise, selects
148+
/// \p Other.
149+
/// In coverage-tracking builds, this also accounts for whether this or
150+
/// \p Other have an annotative DebugLocKind applied, such that if both are
151+
/// empty but exactly one has an annotation, we prefer that annotated
152+
/// location.
153+
DebugLoc orElse(DebugLoc Other) const {
154+
if (*this)
155+
return *this;
156+
#if LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
157+
if (Other)
158+
return Other;
159+
if (getKind() != DebugLocKind::Normal)
160+
return *this;
161+
if (Other.getKind() != DebugLocKind::Normal)
162+
return Other;
163+
return *this;
164+
#else
165+
return Other;
166+
#endif // LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
167+
}
168+
144169
/// Get the underlying \a DILocation.
145170
///
146171
/// \pre !*this or \c isa<DILocation>(getAsMDNode()).

‎llvm/include/llvm/IR/IRBuilder.h

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,18 @@ class FMFSource {
112112
/// Common base class shared among various IRBuilders.
113113
class IRBuilderBase {
114114
/// Pairs of (metadata kind, MDNode *) that should be added to all newly
115-
/// created instructions, like !dbg metadata.
115+
/// created instructions, excluding !dbg metadata, which is stored in the
116+
// StoredDL field.
116117
SmallVector<std::pair<unsigned, MDNode *>, 2> MetadataToCopy;
118+
// The DebugLoc that will be applied to instructions inserted by this builder.
119+
DebugLoc StoredDL;
117120

118121
/// Add or update the an entry (Kind, MD) to MetadataToCopy, if \p MD is not
119122
/// null. If \p MD is null, remove the entry with \p Kind.
120123
void AddOrRemoveMetadataToCopy(unsigned Kind, MDNode *MD) {
124+
assert(Kind != LLVMContext::MD_dbg &&
125+
"MD_dbg metadata must be stored in StoredDL");
126+
121127
if (!MD) {
122128
erase_if(MetadataToCopy, [Kind](const std::pair<unsigned, MDNode *> &KV) {
123129
return KV.first == Kind;
@@ -237,7 +243,9 @@ class IRBuilderBase {
237243

238244
/// Set location information used by debugging information.
239245
void SetCurrentDebugLocation(DebugLoc L) {
240-
AddOrRemoveMetadataToCopy(LLVMContext::MD_dbg, L.getAsMDNode());
246+
// For !dbg metadata attachments, we use DebugLoc instead of the raw MDNode
247+
// to include optional introspection data for use in Debugify.
248+
StoredDL = std::move(L);
241249
}
242250

243251
/// Set nosanitize metadata.
@@ -251,8 +259,12 @@ class IRBuilderBase {
251259
/// not on \p Src will be dropped from MetadataToCopy.
252260
void CollectMetadataToCopy(Instruction *Src,
253261
ArrayRef<unsigned> MetadataKinds) {
254-
for (unsigned K : MetadataKinds)
255-
AddOrRemoveMetadataToCopy(K, Src->getMetadata(K));
262+
for (unsigned K : MetadataKinds) {
263+
if (K == LLVMContext::MD_dbg)
264+
SetCurrentDebugLocation(Src->getDebugLoc());
265+
else
266+
AddOrRemoveMetadataToCopy(K, Src->getMetadata(K));
267+
}
256268
}
257269

258270
/// Get location information used by debugging information.
@@ -266,6 +278,7 @@ class IRBuilderBase {
266278
void AddMetadataToInst(Instruction *I) const {
267279
for (const auto &KV : MetadataToCopy)
268280
I->setMetadata(KV.first, KV.second);
281+
SetInstDebugLocation(I);
269282
}
270283

271284
/// Get the return type of the current function that we're emitting

‎llvm/include/llvm/IR/Instruction.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,7 @@ class Instruction : public User,
695695
/// applications, thus the N-way merging should be in code path.
696696
/// The DebugLoc attached to this instruction will be overwritten by the
697697
/// merged DebugLoc.
698-
void applyMergedLocation(DILocation *LocA, DILocation *LocB);
698+
void applyMergedLocation(DebugLoc LocA, DebugLoc LocB);
699699

700700
/// Updates the debug location given that the instruction has been hoisted
701701
/// from a block to a predecessor of that block.

‎llvm/lib/CodeGen/BranchFolding.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -863,7 +863,7 @@ void BranchFolder::mergeCommonTails(unsigned commonTailIndex) {
863863
"Reached BB end within common tail");
864864
}
865865
assert(MI.isIdenticalTo(*Pos) && "Expected matching MIIs!");
866-
DL = DILocation::getMergedLocation(DL, Pos->getDebugLoc());
866+
DL = DebugLoc::getMergedLocation(DL, Pos->getDebugLoc());
867867
NextCommonInsts[i] = ++Pos;
868868
}
869869
MI.setDebugLoc(DL);

‎llvm/lib/CodeGen/GlobalISel/CSEMIRBuilder.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,7 @@ CSEMIRBuilder::getDominatingInstrForID(FoldingSetNodeID &ID,
5353
} else if (!dominates(MI, CurrPos)) {
5454
// Update the spliced machineinstr's debug location by merging it with the
5555
// debug location of the instruction at the insertion point.
56-
auto *Loc = DILocation::getMergedLocation(getDebugLoc().get(),
57-
MI->getDebugLoc().get());
56+
auto Loc = DebugLoc::getMergedLocation(getDebugLoc(), MI->getDebugLoc());
5857
MI->setDebugLoc(Loc);
5958
CurMBB->splice(CurrPos, CurMBB, MI);
6059
}
@@ -170,7 +169,7 @@ CSEMIRBuilder::generateCopiesIfRequired(ArrayRef<DstOp> DstOps,
170169
if (Observer)
171170
Observer->changingInstr(*MIB);
172171
MIB->setDebugLoc(
173-
DILocation::getMergedLocation(MIB->getDebugLoc(), getDebugLoc()));
172+
DebugLoc::getMergedLocation(MIB->getDebugLoc(), getDebugLoc()));
174173
if (Observer)
175174
Observer->changedInstr(*MIB);
176175
}

‎llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ bool LoadStoreOpt::doSingleStoreMerge(SmallVectorImpl<GStore *> &Stores) {
371371
// For each store, compute pairwise merged debug locs.
372372
DebugLoc MergedLoc = Stores.front()->getDebugLoc();
373373
for (auto *Store : drop_begin(Stores))
374-
MergedLoc = DILocation::getMergedLocation(MergedLoc, Store->getDebugLoc());
374+
MergedLoc = DebugLoc::getMergedLocation(MergedLoc, Store->getDebugLoc());
375375

376376
Builder.setInstr(*Stores.back());
377377
Builder.setDebugLoc(MergedLoc);

‎llvm/lib/CodeGen/MachineBasicBlock.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1575,7 +1575,7 @@ MachineBasicBlock::findBranchDebugLoc() {
15751575
DL = TI->getDebugLoc();
15761576
for (++TI ; TI != end() ; ++TI)
15771577
if (TI->isBranch())
1578-
DL = DILocation::getMergedLocation(DL, TI->getDebugLoc());
1578+
DL = DebugLoc::getMergedLocation(DL, TI->getDebugLoc());
15791579
}
15801580
return DL;
15811581
}

‎llvm/lib/CodeGen/MachineSink.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,8 +1611,8 @@ static void performSink(MachineInstr &MI, MachineBasicBlock &SuccToSinkTo,
16111611
// location to prevent debug-info driven tools from potentially reporting
16121612
// wrong location information.
16131613
if (!SuccToSinkTo.empty() && InsertPos != SuccToSinkTo.end())
1614-
MI.setDebugLoc(DILocation::getMergedLocation(MI.getDebugLoc(),
1615-
InsertPos->getDebugLoc()));
1614+
MI.setDebugLoc(DebugLoc::getMergedLocation(MI.getDebugLoc(),
1615+
InsertPos->getDebugLoc()));
16161616
else
16171617
MI.setDebugLoc(DebugLoc());
16181618

‎llvm/lib/IR/DebugInfo.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -951,8 +951,8 @@ unsigned llvm::getDebugMetadataVersionFromModule(const Module &M) {
951951
return 0;
952952
}
953953

954-
void Instruction::applyMergedLocation(DILocation *LocA, DILocation *LocB) {
955-
setDebugLoc(DILocation::getMergedLocation(LocA, LocB));
954+
void Instruction::applyMergedLocation(DebugLoc LocA, DebugLoc LocB) {
955+
setDebugLoc(DebugLoc::getMergedLocation(LocA, LocB));
956956
}
957957

958958
void Instruction::mergeDIAssignID(

‎llvm/lib/IR/DebugLoc.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,27 @@ DebugLoc DebugLoc::appendInlinedAt(const DebugLoc &DL, DILocation *InlinedAt,
142142
return Last;
143143
}
144144

145+
DebugLoc DebugLoc::getMergedLocations(ArrayRef<DebugLoc> Locs) {
146+
if (Locs.empty())
147+
return DebugLoc();
148+
if (Locs.size() == 1)
149+
return Locs[0];
150+
DebugLoc Merged = Locs[0];
151+
for (const DebugLoc &DL : llvm::drop_begin(Locs)) {
152+
Merged = getMergedLocation(Merged, DL);
153+
if (!Merged)
154+
break;
155+
}
156+
return Merged;
157+
}
158+
DebugLoc DebugLoc::getMergedLocation(DebugLoc LocA, DebugLoc LocB) {
159+
if (!LocA)
160+
return LocA;
161+
if (!LocB)
162+
return LocB;
163+
return DILocation::getMergedLocation(LocA, LocB);
164+
}
165+
145166
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
146167
LLVM_DUMP_METHOD void DebugLoc::dump() const { print(dbgs()); }
147168
#endif

‎llvm/lib/IR/IRBuilder.cpp

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,19 +61,12 @@ Type *IRBuilderBase::getCurrentFunctionReturnType() const {
6161
return BB->getParent()->getReturnType();
6262
}
6363

64-
DebugLoc IRBuilderBase::getCurrentDebugLocation() const {
65-
for (auto &KV : MetadataToCopy)
66-
if (KV.first == LLVMContext::MD_dbg)
67-
return {cast<DILocation>(KV.second)};
68-
69-
return {};
70-
}
64+
DebugLoc IRBuilderBase::getCurrentDebugLocation() const { return StoredDL; }
7165
void IRBuilderBase::SetInstDebugLocation(Instruction *I) const {
72-
for (const auto &KV : MetadataToCopy)
73-
if (KV.first == LLVMContext::MD_dbg) {
74-
I->setDebugLoc(DebugLoc(KV.second));
75-
return;
76-
}
66+
// We prefer to set our current debug location if any has been set, but if
67+
// our debug location is empty and I has a valid location, we shouldn't
68+
// overwrite it.
69+
I->setDebugLoc(StoredDL.orElse(I->getDebugLoc()));
7770
}
7871

7972
Value *IRBuilderBase::CreateAggregateCast(Value *V, Type *DestTy) {

‎llvm/lib/IR/Instruction.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,6 +1355,9 @@ void Instruction::swapProfMetadata() {
13551355

13561356
void Instruction::copyMetadata(const Instruction &SrcInst,
13571357
ArrayRef<unsigned> WL) {
1358+
if (WL.empty() || is_contained(WL, LLVMContext::MD_dbg))
1359+
setDebugLoc(getDebugLoc().orElse(SrcInst.getDebugLoc()));
1360+
13581361
if (!SrcInst.hasMetadata())
13591362
return;
13601363

@@ -1368,8 +1371,6 @@ void Instruction::copyMetadata(const Instruction &SrcInst,
13681371
if (WL.empty() || WLS.count(MD.first))
13691372
setMetadata(MD.first, MD.second);
13701373
}
1371-
if (WL.empty() || WLS.count(LLVMContext::MD_dbg))
1372-
setDebugLoc(SrcInst.getDebugLoc());
13731374
}
13741375

13751376
Instruction *Instruction::clone() const {

‎llvm/lib/Target/BPF/BPFPreserveStaticOffset.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,10 @@ static CallInst *isGEPAndStore(Value *I) {
150150
}
151151

152152
template <class T = Instruction>
153-
static DILocation *mergeDILocations(SmallVector<T *> &Insns) {
154-
DILocation *Merged = (*Insns.begin())->getDebugLoc();
153+
static DebugLoc mergeDebugLocs(SmallVector<T *> &Insns) {
154+
DebugLoc Merged = (*Insns.begin())->getDebugLoc();
155155
for (T *I : Insns)
156-
Merged = DILocation::getMergedLocation(Merged, I->getDebugLoc());
156+
Merged = DebugLoc::getMergedLocation(Merged, I->getDebugLoc());
157157
return Merged;
158158
}
159159

@@ -227,7 +227,7 @@ static Instruction *makeGEPAndLoad(Module *M, GEPChainInfo &GEP,
227227
CallInst *Call = makeIntrinsicCall(M, Intrinsic::bpf_getelementptr_and_load,
228228
{Load->getType()}, Args);
229229
setParamElementType(Call, 0, GEP.SourceElementType);
230-
Call->applyMergedLocation(mergeDILocations(GEP.Members), Load->getDebugLoc());
230+
Call->applyMergedLocation(mergeDebugLocs(GEP.Members), Load->getDebugLoc());
231231
Call->setName((*GEP.Members.rbegin())->getName());
232232
if (Load->isUnordered()) {
233233
Call->setOnlyReadsMemory();
@@ -251,8 +251,7 @@ static Instruction *makeGEPAndStore(Module *M, GEPChainInfo &GEP,
251251
setParamElementType(Call, 1, GEP.SourceElementType);
252252
if (Store->getValueOperand()->getType()->isPointerTy())
253253
setParamReadNone(Call, 0);
254-
Call->applyMergedLocation(mergeDILocations(GEP.Members),
255-
Store->getDebugLoc());
254+
Call->applyMergedLocation(mergeDebugLocs(GEP.Members), Store->getDebugLoc());
256255
if (Store->isUnordered()) {
257256
Call->setOnlyWritesMemory();
258257
Call->setOnlyAccessesArgMemory();

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,8 +1581,8 @@ bool InstCombinerImpl::mergeStoreIntoSuccessor(StoreInst &SI) {
15811581
// Insert a PHI node now if we need it.
15821582
Value *MergedVal = OtherStore->getValueOperand();
15831583
// The debug locations of the original instructions might differ. Merge them.
1584-
DebugLoc MergedLoc = DILocation::getMergedLocation(SI.getDebugLoc(),
1585-
OtherStore->getDebugLoc());
1584+
DebugLoc MergedLoc =
1585+
DebugLoc::getMergedLocation(SI.getDebugLoc(), OtherStore->getDebugLoc());
15861586
if (MergedVal != SI.getValueOperand()) {
15871587
PHINode *PN =
15881588
PHINode::Create(SI.getValueOperand()->getType(), 2, "storemerge");

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5334,8 +5334,7 @@ bool InstCombinerImpl::run() {
53345334
// We copy the old instruction's DebugLoc to the new instruction, unless
53355335
// InstCombine already assigned a DebugLoc to it, in which case we
53365336
// should trust the more specifically selected DebugLoc.
5337-
if (!Result->getDebugLoc())
5338-
Result->setDebugLoc(I->getDebugLoc());
5337+
Result->setDebugLoc(Result->getDebugLoc().orElse(I->getDebugLoc()));
53395338
// We also copy annotation metadata to the new instruction.
53405339
Result->copyMetadata(*I, LLVMContext::MD_annotation);
53415340
// Everything uses the new instruction now.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,7 @@ bool ConstantHoistingPass::emitBaseConstants(GlobalVariable *BaseGV) {
883883
emitBaseConstants(Base, &R);
884884
ReBasesNum++;
885885
// Use the same debug location as the last user of the constant.
886-
Base->setDebugLoc(DILocation::getMergedLocation(
886+
Base->setDebugLoc(DebugLoc::getMergedLocation(
887887
Base->getDebugLoc(), R.User.Inst->getDebugLoc()));
888888
}
889889
assert(!Base->use_empty() && "The use list is empty!?");

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2225,10 +2225,10 @@ bool llvm::promoteLoopAccessesToScalars(
22252225
});
22262226

22272227
// Look at all the loop uses, and try to merge their locations.
2228-
std::vector<DILocation *> LoopUsesLocs;
2229-
for (auto *U : LoopUses)
2230-
LoopUsesLocs.push_back(U->getDebugLoc().get());
2231-
auto DL = DebugLoc(DILocation::getMergedLocations(LoopUsesLocs));
2228+
std::vector<DebugLoc> LoopUsesLocs;
2229+
for (auto U : LoopUses)
2230+
LoopUsesLocs.push_back(U->getDebugLoc());
2231+
auto DL = DebugLoc::getMergedLocations(LoopUsesLocs);
22322232

22332233
// We use the SSAUpdater interface to insert phi nodes as required.
22342234
SmallVector<PHINode *, 16> NewPHIs;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ performBlockTailMerging(Function &F, ArrayRef<BasicBlock *> BBs,
128128

129129
// Now, go through each block (with the current terminator type)
130130
// we've recorded, and rewrite it to branch to the new common block.
131-
DILocation *CommonDebugLoc = nullptr;
131+
DebugLoc CommonDebugLoc;
132132
for (BasicBlock *BB : BBs) {
133133
auto *Term = BB->getTerminator();
134134
assert(Term->getOpcode() == CanonicalTerm->getOpcode() &&
@@ -145,7 +145,7 @@ performBlockTailMerging(Function &F, ArrayRef<BasicBlock *> BBs,
145145
CommonDebugLoc = Term->getDebugLoc();
146146
else
147147
CommonDebugLoc =
148-
DILocation::getMergedLocation(CommonDebugLoc, Term->getDebugLoc());
148+
DebugLoc::getMergedLocation(CommonDebugLoc, Term->getDebugLoc());
149149

150150
// And turn BB into a block that just unconditionally branches
151151
// to the canonical block.

‎llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2077,11 +2077,11 @@ bool SimplifyCFGOpt::hoistSuccIdenticalTerminatorToSwitchOrIf(
20772077

20782078
// Ensure terminator gets a debug location, even an unknown one, in case
20792079
// it involves inlinable calls.
2080-
SmallVector<DILocation *, 4> Locs;
2080+
SmallVector<DebugLoc, 4> Locs;
20812081
Locs.push_back(I1->getDebugLoc());
20822082
for (auto *OtherSuccTI : OtherSuccTIs)
20832083
Locs.push_back(OtherSuccTI->getDebugLoc());
2084-
NT->setDebugLoc(DILocation::getMergedLocations(Locs));
2084+
NT->setDebugLoc(DebugLoc::getMergedLocations(Locs));
20852085

20862086
// PHIs created below will adopt NT's merged DebugLoc.
20872087
IRBuilder<NoFolder> Builder(NT);
@@ -2885,7 +2885,7 @@ static void mergeCompatibleInvokesImpl(ArrayRef<InvokeInst *> Invokes,
28852885
MergedDebugLoc = II->getDebugLoc();
28862886
else
28872887
MergedDebugLoc =
2888-
DILocation::getMergedLocation(MergedDebugLoc, II->getDebugLoc());
2888+
DebugLoc::getMergedLocation(MergedDebugLoc, II->getDebugLoc());
28892889

28902890
// And replace the old `invoke` with an unconditionally branch
28912891
// to the block with the merged `invoke`.

0 commit comments

Comments
 (0)