Skip to content

Commit 7be603e

Browse files
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 b2379bd commit 7be603e

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(

0 commit comments

Comments
 (0)