Skip to content

Commit aa8a1fa

Browse files
authored
[DLCov][NFC] Annotate intentionally-blank DebugLocs in existing code (#136192)
Following the work in PR #107279, this patch applies the annotative DebugLocs, which indicate that a particular instruction is intentionally missing a location for a given reason, to existing sites in the compiler where their conditions apply. This is NFC in ordinary LLVM builds (each function `DebugLoc::getFoo()` is inlined as `DebugLoc()`), but marks the instruction in coverage-tracking builds so that it will be ignored by Debugify, allowing only real errors to be reported. From a developer standpoint, it also communicates the intentionality and reason for a missing DebugLoc. Some notes for reviewers: - The difference between `I->dropLocation()` and `I->setDebugLoc(DebugLoc::getDropped())` is that the former _may_ decide to keep some debug info alive, while the latter will always be empty; in this patch, I always used the latter (even if the former could technically be correct), because the former could result in some (barely) different output, and I'd prefer to keep this patch purely NFC. - I've generally documented the uses of `DebugLoc::getUnknown()`, with the exception of the vectorizers - in summary, they are a huge cause of dropped source locations, and I don't have the time or the domain knowledge currently to solve that, so I've plastered it all over them as a form of "fixme".
1 parent f1575de commit aa8a1fa

19 files changed

+101
-37
lines changed

llvm/lib/Transforms/IPO/GlobalOpt.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,8 +1494,14 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
14941494
// FIXME: Pass Global's alignment when globals have alignment
14951495
AllocaInst *Alloca = new AllocaInst(ElemTy, DL.getAllocaAddrSpace(),
14961496
nullptr, GV->getName(), FirstI);
1497-
if (!isa<UndefValue>(GV->getInitializer()))
1498-
new StoreInst(GV->getInitializer(), Alloca, FirstI);
1497+
Alloca->setDebugLoc(DebugLoc::getCompilerGenerated());
1498+
if (!isa<UndefValue>(GV->getInitializer())) {
1499+
auto *SI = new StoreInst(GV->getInitializer(), Alloca, FirstI);
1500+
// FIXME: We're localizing a global and creating a store instruction for
1501+
// the initial value of that global. Could we logically use the global
1502+
// variable's (if one exists) line for this?
1503+
SI->setDebugLoc(DebugLoc::getCompilerGenerated());
1504+
}
14991505

15001506
GV->replaceAllUsesWith(Alloca);
15011507
GV->eraseFromParent();

llvm/lib/Transforms/IPO/IROutliner.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,7 @@ static void moveFunctionData(Function &Old, Function &New,
730730
// other outlined instructions.
731731
if (!isa<CallInst>(&Val)) {
732732
// Remove the debug information for outlined functions.
733-
Val.setDebugLoc(DebugLoc());
733+
Val.setDebugLoc(DebugLoc::getDropped());
734734

735735
// Loop info metadata may contain line locations. Update them to have no
736736
// value in the new subprogram since the outlined code could be from
@@ -1864,7 +1864,7 @@ replaceArgumentUses(OutlinableRegion &Region,
18641864
Value *ValueOperand = SI->getValueOperand();
18651865

18661866
StoreInst *NewI = cast<StoreInst>(I->clone());
1867-
NewI->setDebugLoc(DebugLoc());
1867+
NewI->setDebugLoc(DebugLoc::getDropped());
18681868
BasicBlock *OutputBB = VBBIt->second;
18691869
NewI->insertInto(OutputBB, OutputBB->end());
18701870
LLVM_DEBUG(dbgs() << "Move store for instruction " << *I << " to "

llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,14 @@ Instruction *InstCombinerImpl::foldPHIArgZextsIntoPHI(PHINode &Phi) {
870870
NewPhi->addIncoming(NewIncoming[I], Phi.getIncomingBlock(I));
871871

872872
InsertNewInstBefore(NewPhi, Phi.getIterator());
873-
return CastInst::CreateZExtOrBitCast(NewPhi, Phi.getType());
873+
auto *CI = CastInst::CreateZExtOrBitCast(NewPhi, Phi.getType());
874+
875+
// We use a dropped location here because the new ZExt is necessarily a merge
876+
// of ZExtInsts and at least one constant from incoming branches; the presence
877+
// of the constant means we have no viable DebugLoc from that branch, and
878+
// therefore we must use a dropped location.
879+
CI->setDebugLoc(DebugLoc::getDropped());
880+
return CI;
874881
}
875882

876883
/// If all operands to a PHI node are the same "unary" operator and they all are

llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,8 @@ static bool processSwitch(SwitchInst *I, LazyValueInfo *LVI,
432432
BasicBlock *NewUnreachableBB =
433433
BasicBlock::Create(BB->getContext(), "default.unreachable",
434434
BB->getParent(), DefaultDest);
435-
new UnreachableInst(BB->getContext(), NewUnreachableBB);
435+
auto *UI = new UnreachableInst(BB->getContext(), NewUnreachableBB);
436+
UI->setDebugLoc(DebugLoc::getTemporary());
436437

437438
DefaultDest->removePredecessor(BB);
438439
SI->setDefaultDest(NewUnreachableBB);

llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,6 +1506,9 @@ bool IndVarSimplify::canonicalizeExitCondition(Loop *L) {
15061506
auto *NewRHS = CastInst::Create(
15071507
Instruction::Trunc, RHS, LHSOp->getType(), "",
15081508
L->getLoopPreheader()->getTerminator()->getIterator());
1509+
// NewRHS is an operation that has been hoisted out of the loop, and
1510+
// therefore should have a dropped location.
1511+
NewRHS->setDebugLoc(DebugLoc::getDropped());
15091512
ICmp->setOperand(Swapped ? 1 : 0, LHSOp);
15101513
ICmp->setOperand(Swapped ? 0 : 1, NewRHS);
15111514
// Samesign flag cannot be preserved after narrowing the compare.

llvm/lib/Transforms/Scalar/JumpThreading.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3001,8 +3001,10 @@ bool JumpThreadingPass::tryToUnfoldSelectInCurrBB(BasicBlock *BB) {
30013001
continue;
30023002
// Expand the select.
30033003
Value *Cond = SI->getCondition();
3004-
if (!isGuaranteedNotToBeUndefOrPoison(Cond, nullptr, SI))
3004+
if (!isGuaranteedNotToBeUndefOrPoison(Cond, nullptr, SI)) {
30053005
Cond = new FreezeInst(Cond, "cond.fr", SI->getIterator());
3006+
cast<FreezeInst>(Cond)->setDebugLoc(DebugLoc::getTemporary());
3007+
}
30063008
MDNode *BranchWeights = getBranchWeightMDNode(*SI);
30073009
Instruction *Term =
30083010
SplitBlockAndInsertIfThen(Cond, SI, false, BranchWeights);

llvm/lib/Transforms/Scalar/LICM.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2248,7 +2248,7 @@ bool llvm::promoteLoopAccessesToScalars(
22482248
if (SawUnorderedAtomic)
22492249
PreheaderLoad->setOrdering(AtomicOrdering::Unordered);
22502250
PreheaderLoad->setAlignment(Alignment);
2251-
PreheaderLoad->setDebugLoc(DebugLoc());
2251+
PreheaderLoad->setDebugLoc(DebugLoc::getDropped());
22522252
if (AATags && LoadIsGuaranteedToExecute)
22532253
PreheaderLoad->setAAMetadata(AATags);
22542254

@@ -2808,6 +2808,7 @@ static bool hoistMulAddAssociation(Instruction &I, Loop &L,
28082808
auto *NewBO =
28092809
BinaryOperator::Create(Ins->getOpcode(), LHS, RHS,
28102810
Ins->getName() + ".reass", Ins->getIterator());
2811+
NewBO->setDebugLoc(DebugLoc::getDropped());
28112812
NewBO->copyIRFlags(Ins);
28122813
if (VariantOp == Ins)
28132814
VariantOp = NewBO;
@@ -2864,6 +2865,7 @@ static bool hoistBOAssociation(Instruction &I, Loop &L,
28642865

28652866
auto *NewBO = BinaryOperator::Create(
28662867
Opcode, LV, Inv, BO->getName() + ".reass", BO->getIterator());
2868+
NewBO->setDebugLoc(DebugLoc::getDropped());
28672869

28682870
if (Opcode == Instruction::FAdd || Opcode == Instruction::FMul) {
28692871
// Intersect FMF flags for FADD and FMUL.

llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,14 +442,15 @@ class LoadEliminationForLoop {
442442
assert(PH && "Preheader should exist!");
443443
Value *InitialPtr = SEE.expandCodeFor(PtrSCEV->getStart(), Ptr->getType(),
444444
PH->getTerminator());
445-
Value *Initial =
445+
Instruction *Initial =
446446
new LoadInst(Cand.Load->getType(), InitialPtr, "load_initial",
447447
/* isVolatile */ false, Cand.Load->getAlign(),
448448
PH->getTerminator()->getIterator());
449449
// We don't give any debug location to Initial, because it is inserted
450450
// into the loop's preheader. A debug location inside the loop will cause
451451
// a misleading stepping when debugging. The test update-debugloc-store
452452
// -forwarded.ll checks this.
453+
Initial->setDebugLoc(DebugLoc::getDropped());
453454

454455
PHINode *PHI = PHINode::Create(Initial->getType(), 2, "store_forwarded");
455456
PHI->insertBefore(L->getHeader()->begin());

llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ static void buildPartialUnswitchConditionalBranch(
274274
BasicBlock &UnswitchedSucc, BasicBlock &NormalSucc, bool InsertFreeze,
275275
const Instruction *I, AssumptionCache *AC, const DominatorTree &DT) {
276276
IRBuilder<> IRB(&BB);
277+
IRB.SetCurrentDebugLocation(DebugLoc::getCompilerGenerated());
277278

278279
SmallVector<Value *> FrozenInvariants;
279280
for (Value *Inv : Invariants) {
@@ -330,6 +331,7 @@ static void buildPartialInvariantUnswitchConditionalBranch(
330331
}
331332

332333
IRBuilder<> IRB(&BB);
334+
IRB.SetCurrentDebugLocation(DebugLoc::getCompilerGenerated());
333335
Value *Cond = VMap[ToDuplicate[0]];
334336
IRB.CreateCondBr(Cond, Direction ? &UnswitchedSucc : &NormalSucc,
335337
Direction ? &NormalSucc : &UnswitchedSucc);
@@ -2369,6 +2371,7 @@ static void unswitchNontrivialInvariants(
23692371
// BI (`dyn_cast<BranchInst>(TI)`) is an in-loop instruction hoisted
23702372
// out of the loop.
23712373
Cond = new FreezeInst(Cond, Cond->getName() + ".fr", BI->getIterator());
2374+
cast<Instruction>(Cond)->setDebugLoc(DebugLoc::getDropped());
23722375
}
23732376
BI->setCondition(Cond);
23742377
DTUpdates.push_back({DominatorTree::Insert, SplitBB, ClonedPH});

llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,8 @@ void TailRecursionEliminator::createTailRecurseLoopHeader(CallInst *CI) {
515515
BasicBlock *NewEntry = BasicBlock::Create(F.getContext(), "", &F, HeaderBB);
516516
NewEntry->takeName(HeaderBB);
517517
HeaderBB->setName("tailrecurse");
518-
BranchInst::Create(HeaderBB, NewEntry);
518+
auto *BI = BranchInst::Create(HeaderBB, NewEntry);
519+
BI->setDebugLoc(DebugLoc::getCompilerGenerated());
519520
// If the new branch preserves the debug location of CI, it could result in
520521
// misleading stepping, if CI is located in a conditional branch.
521522
// So, here we don't give any debug location to the new branch.
@@ -801,6 +802,7 @@ void TailRecursionEliminator::cleanupAndFinalize() {
801802
SelectInst *SI =
802803
SelectInst::Create(RetKnownPN, RetPN, RI->getOperand(0),
803804
"current.ret.tr", RI->getIterator());
805+
SI->setDebugLoc(DebugLoc::getCompilerGenerated());
804806
RetSelects.push_back(SI);
805807
RI->setOperand(0, SI);
806808
}

0 commit comments

Comments
 (0)