Skip to content

Commit 7bdbdc7

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 9c52b3c commit 7bdbdc7

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
@@ -142,6 +142,31 @@ namespace llvm {
142142
static inline DebugLoc getDropped() { return DebugLoc(); }
143143
#endif // LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
144144

145+
static DebugLoc getMergedLocations(ArrayRef<DebugLoc> Locs);
146+
static DebugLoc getMergedLocation(DebugLoc LocA, DebugLoc LocB);
147+
148+
/// If this DebugLoc is non-empty, returns this DebugLoc; otherwise, selects
149+
/// \p Other.
150+
/// In coverage-tracking builds, this also accounts for whether this or
151+
/// \p Other have an annotative DebugLocKind applied, such that if both are
152+
/// empty but exactly one has an annotation, we prefer that annotated
153+
/// location.
154+
DebugLoc orElse(DebugLoc Other) const {
155+
if (*this)
156+
return *this;
157+
#if LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
158+
if (Other)
159+
return Other;
160+
if (getKind() != DebugLocKind::Normal)
161+
return *this;
162+
if (Other.getKind() != DebugLocKind::Normal)
163+
return Other;
164+
return *this;
165+
#else
166+
return Other;
167+
#endif // LLVM_ENABLE_DEBUGLOC_COVERAGE_TRACKING
168+
}
169+
145170
/// Get the underlying \a DILocation.
146171
///
147172
/// \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
@@ -113,12 +113,18 @@ class FMFSource {
113113
/// Common base class shared among various IRBuilders.
114114
class IRBuilderBase {
115115
/// Pairs of (metadata kind, MDNode *) that should be added to all newly
116-
/// created instructions, like !dbg metadata.
116+
/// created instructions, excluding !dbg metadata, which is stored in the
117+
// StoredDL field.
117118
SmallVector<std::pair<unsigned, MDNode *>, 2> MetadataToCopy;
119+
// The DebugLoc that will be applied to instructions inserted by this builder.
120+
DebugLoc StoredDL;
118121

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

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

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

259271
/// Get location information used by debugging information.
@@ -267,6 +279,7 @@ class IRBuilderBase {
267279
void AddMetadataToInst(Instruction *I) const {
268280
for (const auto &KV : MetadataToCopy)
269281
I->setMetadata(KV.first, KV.second);
282+
SetInstDebugLocation(I);
270283
}
271284

272285
/// 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
@@ -698,7 +698,7 @@ class Instruction : public User,
698698
/// applications, thus the N-way merging should be in code path.
699699
/// The DebugLoc attached to this instruction will be overwritten by the
700700
/// merged DebugLoc.
701-
LLVM_ABI void applyMergedLocation(DILocation *LocA, DILocation *LocB);
701+
LLVM_ABI void applyMergedLocation(DebugLoc LocA, DebugLoc LocB);
702702

703703
/// Updates the debug location given that the instruction has been hoisted
704704
/// 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
@@ -862,7 +862,7 @@ void BranchFolder::mergeCommonTails(unsigned commonTailIndex) {
862862
"Reached BB end within common tail");
863863
}
864864
assert(MI.isIdenticalTo(*Pos) && "Expected matching MIIs!");
865-
DL = DILocation::getMergedLocation(DL, Pos->getDebugLoc());
865+
DL = DebugLoc::getMergedLocation(DL, Pos->getDebugLoc());
866866
NextCommonInsts[i] = ++Pos;
867867
}
868868
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
@@ -370,7 +370,7 @@ bool LoadStoreOpt::doSingleStoreMerge(SmallVectorImpl<GStore *> &Stores) {
370370
// For each store, compute pairwise merged debug locs.
371371
DebugLoc MergedLoc = Stores.front()->getDebugLoc();
372372
for (auto *Store : drop_begin(Stores))
373-
MergedLoc = DILocation::getMergedLocation(MergedLoc, Store->getDebugLoc());
373+
MergedLoc = DebugLoc::getMergedLocation(MergedLoc, Store->getDebugLoc());
374374

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

‎llvm/lib/CodeGen/MachineBasicBlock.cpp

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

‎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
@@ -960,8 +960,8 @@ unsigned llvm::getDebugMetadataVersionFromModule(const Module &M) {
960960
return 0;
961961
}
962962

963-
void Instruction::applyMergedLocation(DILocation *LocA, DILocation *LocB) {
964-
setDebugLoc(DILocation::getMergedLocation(LocA, LocB));
963+
void Instruction::applyMergedLocation(DebugLoc LocA, DebugLoc LocB) {
964+
setDebugLoc(DebugLoc::getMergedLocation(LocA, LocB));
965965
}
966966

967967
void Instruction::mergeDIAssignID(

‎llvm/lib/IR/DebugLoc.cpp

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

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

13641364
void Instruction::copyMetadata(const Instruction &SrcInst,
13651365
ArrayRef<unsigned> WL) {
1366+
if (WL.empty() || is_contained(WL, LLVMContext::MD_dbg))
1367+
setDebugLoc(getDebugLoc().orElse(SrcInst.getDebugLoc()));
1368+
13661369
if (!SrcInst.hasMetadata())
13671370
return;
13681371

@@ -1376,8 +1379,6 @@ void Instruction::copyMetadata(const Instruction &SrcInst,
13761379
if (WL.empty() || WLS.count(MD.first))
13771380
setMetadata(MD.first, MD.second);
13781381
}
1379-
if (WL.empty() || WLS.count(LLVMContext::MD_dbg))
1380-
setDebugLoc(SrcInst.getDebugLoc());
13811382
}
13821383

13831384
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
@@ -5340,8 +5340,7 @@ bool InstCombinerImpl::run() {
53405340
// We copy the old instruction's DebugLoc to the new instruction, unless
53415341
// InstCombine already assigned a DebugLoc to it, in which case we
53425342
// should trust the more specifically selected DebugLoc.
5343-
if (!Result->getDebugLoc())
5344-
Result->setDebugLoc(I->getDebugLoc());
5343+
Result->setDebugLoc(Result->getDebugLoc().orElse(I->getDebugLoc()));
53455344
// We also copy annotation metadata to the new instruction.
53465345
Result->copyMetadata(*I, LLVMContext::MD_annotation);
53475346
// 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
@@ -2224,10 +2224,10 @@ bool llvm::promoteLoopAccessesToScalars(
22242224
});
22252225

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

22322232
// We use the SSAUpdater interface to insert phi nodes as required.
22332233
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
@@ -2095,11 +2095,11 @@ bool SimplifyCFGOpt::hoistSuccIdenticalTerminatorToSwitchOrIf(
20952095

20962096
// Ensure terminator gets a debug location, even an unknown one, in case
20972097
// it involves inlinable calls.
2098-
SmallVector<DILocation *, 4> Locs;
2098+
SmallVector<DebugLoc, 4> Locs;
20992099
Locs.push_back(I1->getDebugLoc());
21002100
for (auto *OtherSuccTI : OtherSuccTIs)
21012101
Locs.push_back(OtherSuccTI->getDebugLoc());
2102-
NT->setDebugLoc(DILocation::getMergedLocations(Locs));
2102+
NT->setDebugLoc(DebugLoc::getMergedLocations(Locs));
21032103

21042104
// PHIs created below will adopt NT's merged DebugLoc.
21052105
IRBuilder<NoFolder> Builder(NT);
@@ -2895,7 +2895,7 @@ static void mergeCompatibleInvokesImpl(ArrayRef<InvokeInst *> Invokes,
28952895
MergedDebugLoc = II->getDebugLoc();
28962896
else
28972897
MergedDebugLoc =
2898-
DILocation::getMergedLocation(MergedDebugLoc, II->getDebugLoc());
2898+
DebugLoc::getMergedLocation(MergedDebugLoc, II->getDebugLoc());
28992899

29002900
// And replace the old `invoke` with an unconditionally branch
29012901
// to the block with the merged `invoke`.

0 commit comments

Comments
 (0)