[llvm] df69c69 - [DebugInfo] Handle multiple variable location operands in IR
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 9 08:45:37 PST 2021
Author: gbtozers
Date: 2021-03-09T16:44:38Z
New Revision: df69c69427dea7f5b3b3a4d4564bc77b0926ec88
URL: https://github.com/llvm/llvm-project/commit/df69c69427dea7f5b3b3a4d4564bc77b0926ec88
DIFF: https://github.com/llvm/llvm-project/commit/df69c69427dea7f5b3b3a4d4564bc77b0926ec88.diff
LOG: [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
Added:
Modified:
llvm/include/llvm/IR/IntrinsicInst.h
llvm/include/llvm/Transforms/Utils/Local.h
llvm/lib/CodeGen/CodeGenPrepare.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/IR/IntrinsicInst.cpp
llvm/lib/IR/User.cpp
llvm/lib/Target/AArch64/AArch64StackTagging.cpp
llvm/lib/Transforms/Coroutines/CoroFrame.cpp
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
llvm/lib/Transforms/Scalar/ADCE.cpp
llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp
llvm/lib/Transforms/Utils/Debugify.cpp
llvm/lib/Transforms/Utils/Local.cpp
llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
llvm/unittests/IR/DebugInfoTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h
index a138e4bea8c0..4a2a747dd4bb 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -203,6 +203,7 @@ class DbgVariableIntrinsic : public DbgInfoIntrinsic {
Value *getVariableLocationOp(unsigned OpIdx) const;
void replaceVariableLocationOp(Value *OldValue, Value *NewValue);
+ void replaceVariableLocationOp(unsigned OpIdx, Value *NewValue);
void setVariable(DILocalVariable *NewVar) {
setArgOperand(1, MetadataAsValue::get(NewVar->getContext(), NewVar));
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index dfcf289a30ec..f7efeeb56fd3 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -311,9 +311,10 @@ void salvageDebugInfoForDbgValues(Instruction &I,
/// Given an instruction \p I and DIExpression \p DIExpr operating on it, write
/// the effects of \p I into the returned DIExpression, or return nullptr if
/// it cannot be salvaged. \p StackVal: whether DW_OP_stack_value should be
-/// appended to the expression.
+/// appended to the expression. \p LocNo: the index of the location operand to
+/// which \p I applies, should be 0 for debug info without a DIArgList.
DIExpression *salvageDebugInfoImpl(Instruction &I, DIExpression *DIExpr,
- bool StackVal);
+ bool StackVal, unsigned LocNo);
/// Point debug users of \p From to \p To or salvage them. Use this function
/// only when replacing all uses of \p From with \p To, with a guarantee that
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index e745a3743434..7c9775601773 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -2855,11 +2855,16 @@ class TypePromotionTransaction {
/// Keep track of the debug users.
SmallVector<DbgValueInst *, 1> DbgValues;
+ /// Keep track of the new value so that we can undo it by replacing
+ /// instances of the new value with the original value.
+ Value *New;
+
using use_iterator = SmallVectorImpl<InstructionAndIdx>::iterator;
public:
/// Replace all the use of \p Inst by \p New.
- UsesReplacer(Instruction *Inst, Value *New) : TypePromotionAction(Inst) {
+ UsesReplacer(Instruction *Inst, Value *New)
+ : TypePromotionAction(Inst), New(New) {
LLVM_DEBUG(dbgs() << "Do: UsersReplacer: " << *Inst << " with " << *New
<< "\n");
// Record the original uses.
@@ -2885,7 +2890,7 @@ class TypePromotionTransaction {
// the original debug uses must also be reinstated to maintain the
// correctness and utility of debug value instructions.
for (auto *DVI : DbgValues)
- DVI->replaceVariableLocationOp(DVI->getVariableLocationOp(0), Inst);
+ DVI->replaceVariableLocationOp(New, Inst);
}
};
@@ -7876,18 +7881,21 @@ bool CodeGenPrepare::fixupDbgValue(Instruction *I) {
DbgValueInst &DVI = *cast<DbgValueInst>(I);
// Does this dbg.value refer to a sunk address calculation?
- Value *Location = DVI.getVariableLocationOp(0);
- WeakTrackingVH SunkAddrVH = SunkAddrs[Location];
- Value *SunkAddr = SunkAddrVH.pointsToAliveValue() ? SunkAddrVH : nullptr;
- if (SunkAddr) {
- // Point dbg.value at locally computed address, which should give the best
- // opportunity to be accurately lowered. This update may change the type of
- // pointer being referred to; however this makes no
diff erence to debugging
- // information, and we can't generate bitcasts that may affect codegen.
- DVI.replaceVariableLocationOp(Location, SunkAddr);
- return true;
- }
- return false;
+ bool AnyChange = false;
+ for (Value *Location : DVI.getValues()) {
+ WeakTrackingVH SunkAddrVH = SunkAddrs[Location];
+ Value *SunkAddr = SunkAddrVH.pointsToAliveValue() ? SunkAddrVH : nullptr;
+ if (SunkAddr) {
+ // Point dbg.value at locally computed address, which should give the best
+ // opportunity to be accurately lowered. This update may change the type
+ // of pointer being referred to; however this makes no
diff erence to
+ // debugging information, and we can't generate bitcasts that may affect
+ // codegen.
+ DVI.replaceVariableLocationOp(Location, SunkAddr);
+ AnyChange = true;
+ }
+ }
+ return AnyChange;
}
// A llvm.dbg.value may be using a value before its definition, due to
@@ -7906,30 +7914,51 @@ bool CodeGenPrepare::placeDbgValues(Function &F) {
if (!DVI)
continue;
- Instruction *VI = dyn_cast_or_null<Instruction>(DVI->getValue());
+ SmallVector<Instruction *, 4> VIs;
+ for (Value *V : DVI->getValues())
+ if (Instruction *VI = dyn_cast_or_null<Instruction>(V))
+ VIs.push_back(VI);
+
+ // This DVI may depend on multiple instructions, complicating any
+ // potential sink. This block takes the defensive approach, opting to
+ // "undef" the DVI if it has more than one instruction and any of them do
+ // not dominate DVI.
+ for (Instruction *VI : VIs) {
+ if (VI->isTerminator())
+ continue;
- if (!VI || VI->isTerminator())
- continue;
+ // If VI is a phi in a block with an EHPad terminator, we can't insert
+ // after it.
+ if (isa<PHINode>(VI) && VI->getParent()->getTerminator()->isEHPad())
+ continue;
- // If VI is a phi in a block with an EHPad terminator, we can't insert
- // after it.
- if (isa<PHINode>(VI) && VI->getParent()->getTerminator()->isEHPad())
- continue;
+ // If the defining instruction dominates the dbg.value, we do not need
+ // to move the dbg.value.
+ if (DT.dominates(VI, DVI))
+ continue;
- // If the defining instruction dominates the dbg.value, we do not need
- // to move the dbg.value.
- if (DT.dominates(VI, DVI))
- continue;
+ // If we depend on multiple instructions and any of them doesn't
+ // dominate this DVI, we probably can't salvage it: moving it to
+ // after any of the instructions could cause us to lose the others.
+ if (VIs.size() > 1) {
+ LLVM_DEBUG(
+ dbgs()
+ << "Unable to find valid location for Debug Value, undefing:\n"
+ << *DVI);
+ DVI->setUndef();
+ break;
+ }
- LLVM_DEBUG(dbgs() << "Moving Debug Value before :\n"
- << *DVI << ' ' << *VI);
- DVI->removeFromParent();
- if (isa<PHINode>(VI))
- DVI->insertBefore(&*VI->getParent()->getFirstInsertionPt());
- else
- DVI->insertAfter(VI);
- MadeChange = true;
- ++NumDbgValueMoved;
+ LLVM_DEBUG(dbgs() << "Moving Debug Value before :\n"
+ << *DVI << ' ' << *VI);
+ DVI->removeFromParent();
+ if (isa<PHINode>(VI))
+ DVI->insertBefore(&*VI->getParent()->getFirstInsertionPt());
+ else
+ DVI->insertAfter(VI);
+ MadeChange = true;
+ ++NumDbgValueMoved;
+ }
}
}
return MadeChange;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 6b3ccc3dc0e2..eb5a9676ecc3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -1251,7 +1251,8 @@ void SelectionDAGBuilder::salvageUnresolvedDbgValue(DanglingDebugInfo &DDI) {
// variable. FIXME: Further work could recover those too.
while (isa<Instruction>(V)) {
Instruction &VAsInst = *cast<Instruction>(V);
- DIExpression *NewExpr = salvageDebugInfoImpl(VAsInst, Expr, StackValue);
+ // Temporary "0", awaiting real implementation.
+ DIExpression *NewExpr = salvageDebugInfoImpl(VAsInst, Expr, StackValue, 0);
// If we cannot salvage any further, and haven't yet found a suitable debug
// expression, bail out.
@@ -6044,7 +6045,7 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
DILocalVariable *Variable = DI.getVariable();
DIExpression *Expression = DI.getExpression();
dropDanglingDebugInfo(Variable, Expression);
- SmallVector<Value *> Values(DI.getValues());
+ SmallVector<Value *, 4> Values(DI.getValues());
if (Values.empty())
return;
diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp
index 3d3f734ba5e0..55bc314f9ab3 100644
--- a/llvm/lib/IR/IntrinsicInst.cpp
+++ b/llvm/lib/IR/IntrinsicInst.cpp
@@ -98,6 +98,24 @@ void DbgVariableIntrinsic::replaceVariableLocationOp(Value *OldValue,
setArgOperand(
0, MetadataAsValue::get(getContext(), DIArgList::get(getContext(), MDs)));
}
+void DbgVariableIntrinsic::replaceVariableLocationOp(unsigned OpIdx,
+ Value *NewValue) {
+ assert(OpIdx < getNumVariableLocationOps() && "Invalid Operand Index");
+ if (!hasArgList()) {
+ Value *NewOperand = isa<MetadataAsValue>(NewValue)
+ ? NewValue
+ : MetadataAsValue::get(
+ getContext(), ValueAsMetadata::get(NewValue));
+ return setArgOperand(0, NewOperand);
+ }
+ SmallVector<ValueAsMetadata *, 4> MDs;
+ ValueAsMetadata *NewOperand = getAsMetadata(NewValue);
+ for (unsigned Idx = 0; Idx < getNumVariableLocationOps(); ++Idx)
+ MDs.push_back(Idx == OpIdx ? NewOperand
+ : getAsMetadata(getVariableLocationOp(Idx)));
+ setArgOperand(
+ 0, MetadataAsValue::get(getContext(), DIArgList::get(getContext(), MDs)));
+}
Optional<uint64_t> DbgVariableIntrinsic::getFragmentSizeInBits() const {
if (auto Fragment = getExpression()->getFragmentInfo())
diff --git a/llvm/lib/IR/User.cpp b/llvm/lib/IR/User.cpp
index 9105c6fbd230..f4143163ab13 100644
--- a/llvm/lib/IR/User.cpp
+++ b/llvm/lib/IR/User.cpp
@@ -31,6 +31,10 @@ void User::replaceUsesOfWith(Value *From, Value *To) {
// most importantly, removing "this" from the use list of "From".
setOperand(i, To);
}
+ if (auto DVI = dyn_cast_or_null<DbgVariableIntrinsic>(this)) {
+ if (is_contained(DVI->location_ops(), From))
+ DVI->replaceVariableLocationOp(From, To);
+ }
}
//===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
index 793db06f79ad..e3938d9fe6fe 100644
--- a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
+++ b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -563,10 +563,9 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) {
}
if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(I)) {
- if (auto *AI =
- dyn_cast_or_null<AllocaInst>(DVI->getVariableLocationOp(0))) {
- Allocas[AI].DbgVariableIntrinsics.push_back(DVI);
- }
+ for (Value *V : DVI->location_ops())
+ if (auto *AI = dyn_cast_or_null<AllocaInst>(V))
+ Allocas[AI].DbgVariableIntrinsics.push_back(DVI);
continue;
}
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index accd3a6ce16a..a6c6c4adb87f 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -2173,7 +2173,7 @@ void coro::salvageDebugInfo(
Storage = StInst->getOperand(0);
} else if (auto *GEPInst = dyn_cast<GetElementPtrInst>(Storage)) {
Expr = llvm::salvageDebugInfoImpl(*GEPInst, Expr,
- /*WithStackValue=*/false);
+ /*WithStackValue=*/false, 0);
if (!Expr)
return;
Storage = GEPInst->getOperand(0);
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index d7ff12dcfa65..975ace6799d1 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3576,15 +3576,6 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
llvm::sort(DbgUsersToSink,
[](auto *A, auto *B) { return B->comesBefore(A); });
- // Update the arguments of a dbg.declare instruction, so that it
- // does not point into a sunk instruction.
- auto updateDbgDeclare = [](DbgVariableIntrinsic *DII) {
- if (!isa<DbgDeclareInst>(DII))
- return false;
-
- return true;
- };
-
SmallVector<DbgVariableIntrinsic *, 2> DIIClones;
SmallSet<DebugVariable, 4> SunkVariables;
for (auto User : DbgUsersToSink) {
@@ -3592,7 +3583,7 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
// one per variable fragment. It should be left in the original place
// because the sunk instruction is not an alloca (otherwise we could not be
// here).
- if (updateDbgDeclare(User))
+ if (isa<DbgDeclareInst>(User))
continue;
DebugVariable DbgUserVariable =
@@ -3603,6 +3594,8 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
continue;
DIIClones.emplace_back(cast<DbgVariableIntrinsic>(User->clone()));
+ if (isa<DbgDeclareInst>(User) && isa<CastInst>(I))
+ DIIClones.back()->replaceVariableLocationOp(I, I->getOperand(0));
LLVM_DEBUG(dbgs() << "CLONE: " << *DIIClones.back() << '\n');
}
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index e02076c74420..27715ff86ff2 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1218,10 +1218,10 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) {
isa<CleanupReturnInst>(Inst))
RetVec.push_back(&Inst);
- if (auto *DDI = dyn_cast<DbgVariableIntrinsic>(&Inst))
- if (auto *Alloca =
- dyn_cast_or_null<AllocaInst>(DDI->getVariableLocationOp(0)))
- AllocaDbgMap[Alloca].push_back(DDI);
+ if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst))
+ for (Value *V : DVI->location_ops())
+ if (auto *Alloca = dyn_cast_or_null<AllocaInst>(V))
+ AllocaDbgMap[Alloca].push_back(DVI);
if (InstrumentLandingPads && isa<LandingPadInst>(Inst))
LandingPadVec.push_back(&Inst);
@@ -1297,13 +1297,18 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) {
}
if (!AllocaToPaddedAllocaMap.empty()) {
- for (auto &BB : F)
- for (auto &Inst : BB)
- if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst))
- if (auto *AI =
- dyn_cast_or_null<AllocaInst>(DVI->getVariableLocationOp(0)))
- if (auto *NewAI = AllocaToPaddedAllocaMap.lookup(AI))
- DVI->replaceVariableLocationOp(AI, NewAI);
+ for (auto &BB : F) {
+ for (auto &Inst : BB) {
+ if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&Inst)) {
+ for (Value *V : DVI->location_ops()) {
+ if (auto *AI = dyn_cast_or_null<AllocaInst>(V)) {
+ if (auto *NewAI = AllocaToPaddedAllocaMap.lookup(AI))
+ DVI->replaceVariableLocationOp(V, NewAI);
+ }
+ }
+ }
+ }
+ }
for (auto &P : AllocaToPaddedAllocaMap)
P.first->eraseFromParent();
}
diff --git a/llvm/lib/Transforms/Scalar/ADCE.cpp b/llvm/lib/Transforms/Scalar/ADCE.cpp
index c8fb9d106e32..6a31e71d80d0 100644
--- a/llvm/lib/Transforms/Scalar/ADCE.cpp
+++ b/llvm/lib/Transforms/Scalar/ADCE.cpp
@@ -521,10 +521,14 @@ bool AggressiveDeadCodeElimination::removeDeadInstructions() {
// If intrinsic is pointing at a live SSA value, there may be an
// earlier optimization bug: if we know the location of the variable,
// why isn't the scope of the location alive?
- if (Value *V = DII->getVariableLocationOp(0))
- if (Instruction *II = dyn_cast<Instruction>(V))
- if (isLive(II))
+ for (Value *V : DII->location_ops()) {
+ if (Instruction *II = dyn_cast<Instruction>(V)) {
+ if (isLive(II)) {
dbgs() << "Dropping debug info for " << *DII << "\n";
+ break;
+ }
+ }
+ }
}
}
});
diff --git a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index facbfce86d8e..d67266330396 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -5826,57 +5826,71 @@ void LoopStrengthReduce::getAnalysisUsage(AnalysisUsage &AU) const {
AU.addPreserved<MemorySSAWrapperPass>();
}
-using EqualValues = SmallVector<std::tuple<WeakVH, int64_t, DIExpression *>, 4>;
-using EqualValuesMap = DenseMap<DbgValueInst *, EqualValues>;
+using EqualValues = SmallVector<std::tuple<WeakVH, int64_t>, 4>;
+using EqualValuesMap =
+ DenseMap<std::pair<DbgValueInst *, unsigned>, EqualValues>;
+using ExpressionMap = DenseMap<DbgValueInst *, DIExpression *>;
static void DbgGatherEqualValues(Loop *L, ScalarEvolution &SE,
- EqualValuesMap &DbgValueToEqualSet) {
+ EqualValuesMap &DbgValueToEqualSet,
+ ExpressionMap &DbgValueToExpression) {
for (auto &B : L->getBlocks()) {
for (auto &I : *B) {
auto DVI = dyn_cast<DbgValueInst>(&I);
if (!DVI)
continue;
- auto V = DVI->getVariableLocationOp(0);
- if (!V || !SE.isSCEVable(V->getType()))
- continue;
- auto DbgValueSCEV = SE.getSCEV(V);
- EqualValues EqSet;
- for (PHINode &Phi : L->getHeader()->phis()) {
- if (V->getType() != Phi.getType())
- continue;
- if (!SE.isSCEVable(Phi.getType()))
+ for (unsigned Idx = 0; Idx < DVI->getNumVariableLocationOps(); ++Idx) {
+ // TODO: We can duplicate results if the same arg appears more than
+ // once.
+ Value *V = DVI->getVariableLocationOp(Idx);
+ if (!V || !SE.isSCEVable(V->getType()))
continue;
- auto PhiSCEV = SE.getSCEV(&Phi);
- Optional<APInt> Offset =
- SE.computeConstantDifference(DbgValueSCEV, PhiSCEV);
- if (Offset && Offset->getMinSignedBits() <= 64)
- EqSet.emplace_back(std::make_tuple(
- &Phi, Offset.getValue().getSExtValue(), DVI->getExpression()));
+ auto DbgValueSCEV = SE.getSCEV(V);
+ EqualValues EqSet;
+ for (PHINode &Phi : L->getHeader()->phis()) {
+ if (V->getType() != Phi.getType())
+ continue;
+ if (!SE.isSCEVable(Phi.getType()))
+ continue;
+ auto PhiSCEV = SE.getSCEV(&Phi);
+ Optional<APInt> Offset =
+ SE.computeConstantDifference(DbgValueSCEV, PhiSCEV);
+ if (Offset && Offset->getMinSignedBits() <= 64)
+ EqSet.emplace_back(
+ std::make_tuple(&Phi, Offset.getValue().getSExtValue()));
+ }
+ DbgValueToEqualSet[{DVI, Idx}] = std::move(EqSet);
+ DbgValueToExpression[DVI] = DVI->getExpression();
}
- DbgValueToEqualSet[DVI] = std::move(EqSet);
}
}
}
-static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet) {
+static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet,
+ ExpressionMap &DbgValueToExpression) {
for (auto A : DbgValueToEqualSet) {
- auto DVI = A.first;
+ auto DVI = A.first.first;
+ auto Idx = A.first.second;
// Only update those that are now undef.
- if (!isa_and_nonnull<UndefValue>(DVI->getVariableLocationOp(0)))
+ if (!isa_and_nonnull<UndefValue>(DVI->getVariableLocationOp(Idx)))
continue;
for (auto EV : A.second) {
- auto V = std::get<WeakVH>(EV);
- if (!V)
+ auto EVHandle = std::get<WeakVH>(EV);
+ if (!EVHandle)
continue;
- auto DbgDIExpr = std::get<DIExpression *>(EV);
+ // The dbg.value may have had its value changed by LSR; refresh it from
+ // the map, but continue to update the mapped expression as it may be
+ // updated multiple times in this function.
+ auto DbgDIExpr = DbgValueToExpression[DVI];
auto Offset = std::get<int64_t>(EV);
- DVI->replaceVariableLocationOp(DVI->getVariableLocationOp(0), V);
+ DVI->replaceVariableLocationOp(Idx, EVHandle);
if (Offset) {
SmallVector<uint64_t, 8> Ops;
DIExpression::appendOffset(Ops, Offset);
- DbgDIExpr = DIExpression::prependOpcodes(DbgDIExpr, Ops, true);
+ DbgDIExpr = DIExpression::appendOpsToArg(DbgDIExpr, Ops, Idx, true);
}
DVI->setExpression(DbgDIExpr);
+ DbgValueToExpression[DVI] = DbgDIExpr;
break;
}
}
@@ -5900,7 +5914,8 @@ static bool ReduceLoopStrength(Loop *L, IVUsers &IU, ScalarEvolution &SE,
// Debug preservation - before we start removing anything create equivalence
// sets for the llvm.dbg.value intrinsics.
EqualValuesMap DbgValueToEqualSet;
- DbgGatherEqualValues(L, SE, DbgValueToEqualSet);
+ ExpressionMap DbgValueToExpression;
+ DbgGatherEqualValues(L, SE, DbgValueToEqualSet, DbgValueToExpression);
// Remove any extra phis created by processing inner loops.
Changed |= DeleteDeadPHIs(L->getHeader(), &TLI, MSSAU.get());
@@ -5920,7 +5935,7 @@ static bool ReduceLoopStrength(Loop *L, IVUsers &IU, ScalarEvolution &SE,
}
}
- DbgApplyEqualValues(DbgValueToEqualSet);
+ DbgApplyEqualValues(DbgValueToEqualSet, DbgValueToExpression);
return Changed;
}
diff --git a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
index 0cb9771b6e86..809e62b330dd 100644
--- a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
+++ b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
@@ -266,11 +266,13 @@ bool SpeculativeExecutionPass::considerHoistingFromTo(
const auto AllPrecedingUsesFromBlockHoisted = [&NotHoisted](const User *U) {
// Debug variable has special operand to check it's not hoisted.
if (const auto *DVI = dyn_cast<DbgVariableIntrinsic>(U)) {
- if (const auto *I =
- dyn_cast_or_null<Instruction>(DVI->getVariableLocationOp(0)))
- if (NotHoisted.count(I) == 0)
- return true;
- return false;
+ return all_of(DVI->location_ops(), [&NotHoisted](Value *V) {
+ if (const auto *I = dyn_cast_or_null<Instruction>(V)) {
+ if (NotHoisted.count(I) == 0)
+ return true;
+ }
+ return false;
+ });
}
// Usially debug label instrinsic corresponds to label in LLVM IR. In these
diff --git a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
index 9697e7d72c2c..d1a3ae5c0ed0 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -407,7 +407,8 @@ static bool removeRedundantDbgInstrsUsingBackwardScan(BasicBlock *BB) {
/// - Keep track of non-overlapping fragments.
static bool removeRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) {
SmallVector<DbgValueInst *, 8> ToBeRemoved;
- DenseMap<DebugVariable, std::pair<Value *, DIExpression *> > VariableMap;
+ DenseMap<DebugVariable, std::pair<SmallVector<Value *, 4>, DIExpression *>>
+ VariableMap;
for (auto &I : *BB) {
if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(&I)) {
DebugVariable Key(DVI->getVariable(),
@@ -416,10 +417,10 @@ static bool removeRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) {
auto VMI = VariableMap.find(Key);
// Update the map if we found a new value/expression describing the
// variable, or if the variable wasn't mapped already.
- if (VMI == VariableMap.end() ||
- VMI->second.first != DVI->getValue() ||
+ SmallVector<Value *, 4> Values(DVI->getValues());
+ if (VMI == VariableMap.end() || VMI->second.first != Values ||
VMI->second.second != DVI->getExpression()) {
- VariableMap[Key] = { DVI->getValue(), DVI->getExpression() };
+ VariableMap[Key] = {Values, DVI->getExpression()};
continue;
}
// Found an identical mapping. Remember the instruction for later removal.
diff --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
index 2b0ee77f4c9b..4ad33c14585d 100644
--- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -1511,20 +1511,19 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc,
continue;
}
- // If the location isn't a constant or an instruction, delete the
- // intrinsic.
- auto *DVI = cast<DbgVariableIntrinsic>(DII);
- Value *Location = DVI->getVariableLocationOp(0);
- if (!Location ||
- (!isa<Constant>(Location) && !isa<Instruction>(Location))) {
- DebugIntrinsicsToDelete.push_back(DVI);
- continue;
- }
+ auto IsInvalidLocation = [&NewFunc](Value *Location) {
+ // Location is invalid if it isn't a constant or an instruction, or is an
+ // instruction but isn't in the new function.
+ if (!Location ||
+ (!isa<Constant>(Location) && !isa<Instruction>(Location)))
+ return true;
+ Instruction *LocationInst = dyn_cast<Instruction>(Location);
+ return LocationInst && LocationInst->getFunction() != &NewFunc;
+ };
- // If the variable location is an instruction but isn't in the new
- // function, delete the intrinsic.
- Instruction *LocationInst = dyn_cast<Instruction>(Location);
- if (LocationInst && LocationInst->getFunction() != &NewFunc) {
+ auto *DVI = cast<DbgVariableIntrinsic>(DII);
+ // If any of the used locations are invalid, delete the intrinsic.
+ if (any_of(DVI->location_ops(), IsInvalidLocation)) {
DebugIntrinsicsToDelete.push_back(DVI);
continue;
}
diff --git a/llvm/lib/Transforms/Utils/Debugify.cpp b/llvm/lib/Transforms/Utils/Debugify.cpp
index 5c7780f019af..7e6ce8c166e6 100644
--- a/llvm/lib/Transforms/Utils/Debugify.cpp
+++ b/llvm/lib/Transforms/Utils/Debugify.cpp
@@ -492,15 +492,16 @@ bool diagnoseMisSizedDbgValue(Module &M, DbgValueInst *DVI) {
//
// TODO: This, along with a check for non-null value operands, should be
// promoted to verifier failures.
- Value *V = DVI->getValue();
- if (!V)
- return false;
// For now, don't try to interpret anything more complicated than an empty
// DIExpression. Eventually we should try to handle OP_deref and fragments.
if (DVI->getExpression()->getNumElements())
return false;
+ Value *V = DVI->getVariableLocationOp(0);
+ if (!V)
+ return false;
+
Type *Ty = V->getType();
uint64_t ValueOperandSize = getAllocSizeInBits(M, Ty);
Optional<uint64_t> DbgVarSize = DVI->getFragmentSizeInBits();
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 446149f42ce3..066826b6f36a 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -411,7 +411,7 @@ bool llvm::wouldInstructionBeTriviallyDead(Instruction *I,
return true;
}
if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(I)) {
- if (DVI->getValue())
+ if (DVI->hasArgList() || DVI->getValue(0))
return false;
return true;
}
@@ -1360,7 +1360,7 @@ static bool PhiHasDebugValue(DILocalVariable *DIVar,
SmallVector<DbgValueInst *, 1> DbgValues;
findDbgValues(DbgValues, APN);
for (auto *DVI : DbgValues) {
- assert(DVI->getValue() == APN);
+ assert(is_contained(DVI->getValues(), APN));
if ((DVI->getVariable() == DIVar) && (DVI->getExpression() == DIExpr))
return true;
}
@@ -1387,13 +1387,19 @@ static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) {
// We can't always calculate the size of the DI variable (e.g. if it is a
// VLA). Try to use the size of the alloca that the dbg intrinsic describes
// intead.
- if (DII->isAddressOfVariable())
- if (auto *AI = dyn_cast_or_null<AllocaInst>(DII->getVariableLocationOp(0)))
+ if (DII->isAddressOfVariable()) {
+ // DII should have exactly 1 location when it is an address.
+ assert(DII->getNumVariableLocationOps() == 1 &&
+ "address of variable must have exactly 1 location operand.");
+ if (auto *AI =
+ dyn_cast_or_null<AllocaInst>(DII->getVariableLocationOp(0))) {
if (Optional<TypeSize> FragmentSize = AI->getAllocationSizeInBits(DL)) {
assert(ValueSize.isScalable() == FragmentSize->isScalable() &&
"Both sizes should agree on the scalable flag.");
return TypeSize::isKnownGE(ValueSize, *FragmentSize);
}
+ }
+ }
// Could not determine size of variable. Conservatively return false.
return false;
}
@@ -1596,17 +1602,26 @@ void llvm::insertDebugValuesForPHIs(BasicBlock *BB,
ValueToValueMapTy DbgValueMap;
for (auto &I : *BB) {
if (auto DbgII = dyn_cast<DbgVariableIntrinsic>(&I)) {
- if (auto *Loc =
- dyn_cast_or_null<PHINode>(DbgII->getVariableLocationOp(0)))
- DbgValueMap.insert({Loc, DbgII});
+ for (Value *V : DbgII->location_ops())
+ if (auto *Loc = dyn_cast_or_null<PHINode>(V))
+ DbgValueMap.insert({Loc, DbgII});
}
}
if (DbgValueMap.size() == 0)
return;
+ // Map a pair of the destination BB and old dbg.value to the new dbg.value,
+ // so that if a dbg.value is being rewritten to use more than one of the
+ // inserted PHIs in the same destination BB, we can update the same dbg.value
+ // with all the new PHIs instead of creating one copy for each.
+ SmallDenseMap<std::pair<BasicBlock *, DbgVariableIntrinsic *>,
+ DbgVariableIntrinsic *>
+ NewDbgValueMap;
// Then iterate through the new PHIs and look to see if they use one of the
- // previously mapped PHIs. If so, insert a new dbg.value intrinsic that will
- // propagate the info through the new PHI.
+ // previously mapped PHIs. If so, create a new dbg.value intrinsic that will
+ // propagate the info through the new PHI. If we use more than one new PHI in
+ // a single destination BB with the same old dbg.value, merge the updates so
+ // that we get a single new dbg.value with all the new PHIs.
for (auto PHI : InsertedPHIs) {
BasicBlock *Parent = PHI->getParent();
// Avoid inserting an intrinsic into an EH block.
@@ -1616,15 +1631,27 @@ void llvm::insertDebugValuesForPHIs(BasicBlock *BB,
auto V = DbgValueMap.find(VI);
if (V != DbgValueMap.end()) {
auto *DbgII = cast<DbgVariableIntrinsic>(V->second);
- DbgVariableIntrinsic *NewDbgII =
- cast<DbgVariableIntrinsic>(DbgII->clone());
- NewDbgII->replaceVariableLocationOp(VI, PHI);
- auto InsertionPt = Parent->getFirstInsertionPt();
- assert(InsertionPt != Parent->end() && "Ill-formed basic block");
- NewDbgII->insertBefore(&*InsertionPt);
+ auto NewDI = NewDbgValueMap.find({Parent, DbgII});
+ if (NewDI == NewDbgValueMap.end()) {
+ auto *NewDbgII = cast<DbgVariableIntrinsic>(DbgII->clone());
+ NewDI = NewDbgValueMap.insert({{Parent, DbgII}, NewDbgII}).first;
+ }
+ DbgVariableIntrinsic *NewDbgII = NewDI->second;
+ // If PHI contains VI as an operand more than once, we may
+ // replaced it in NewDbgII; confirm that it is present.
+ if (is_contained(NewDbgII->location_ops(), VI))
+ NewDbgII->replaceVariableLocationOp(VI, PHI);
}
}
}
+ // Insert thew new dbg.values into their destination blocks.
+ for (auto DI : NewDbgValueMap) {
+ BasicBlock *Parent = DI.first.first;
+ auto *NewDbgII = DI.second;
+ auto InsertionPt = Parent->getFirstInsertionPt();
+ assert(InsertionPt != Parent->end() && "Ill-formed basic block");
+ NewDbgII->insertBefore(&*InsertionPt);
+ }
}
/// Finds all intrinsics declaring local variables as living in the memory that
@@ -1665,11 +1692,25 @@ void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues, Value *V) {
// DenseMap lookup.
if (!V->isUsedByMetadata())
return;
- if (auto *L = LocalAsMetadata::getIfExists(V))
- if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), L))
+ // TODO: If this value appears multiple times in a DIArgList, we should still
+ // only add the owning DbgValueInst once; use this set to track ArgListUsers.
+ // This behaviour can be removed when we can automatically remove duplicates.
+ SmallPtrSet<DbgValueInst *, 4> EncounteredDbgValues;
+ if (auto *L = LocalAsMetadata::getIfExists(V)) {
+ if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), L)) {
for (User *U : MDV->users())
if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(U))
DbgValues.push_back(DVI);
+ }
+ for (Metadata *AL : L->getAllArgListUsers()) {
+ if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), AL)) {
+ for (User *U : MDV->users())
+ if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(U))
+ if (EncounteredDbgValues.insert(DVI).second)
+ DbgValues.push_back(DVI);
+ }
+ }
+ }
}
void llvm::findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgUsers,
@@ -1678,11 +1719,25 @@ void llvm::findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgUsers,
// DenseMap lookup.
if (!V->isUsedByMetadata())
return;
- if (auto *L = LocalAsMetadata::getIfExists(V))
- if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), L))
+ // TODO: If this value appears multiple times in a DIArgList, we should still
+ // only add the owning DbgValueInst once; use this set to track ArgListUsers.
+ // This behaviour can be removed when we can automatically remove duplicates.
+ SmallPtrSet<DbgVariableIntrinsic *, 4> EncounteredDbgValues;
+ if (auto *L = LocalAsMetadata::getIfExists(V)) {
+ if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), L)) {
for (User *U : MDV->users())
if (DbgVariableIntrinsic *DII = dyn_cast<DbgVariableIntrinsic>(U))
DbgUsers.push_back(DII);
+ }
+ for (Metadata *AL : L->getAllArgListUsers()) {
+ if (auto *MDV = MetadataAsValue::getIfExists(V->getContext(), AL)) {
+ for (User *U : MDV->users())
+ if (DbgVariableIntrinsic *DII = dyn_cast<DbgVariableIntrinsic>(U))
+ if (EncounteredDbgValues.insert(DII).second)
+ DbgUsers.push_back(DII);
+ }
+ }
+ }
}
bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress,
@@ -1752,9 +1807,14 @@ void llvm::salvageDebugInfoForDbgValues(
// are implicitly pointing out the value as a DWARF memory location
// description.
bool StackValue = isa<DbgValueInst>(DII);
+ auto DIILocation = DII->location_ops();
+ assert(
+ is_contained(DIILocation, &I) &&
+ "DbgVariableIntrinsic must use salvaged instruction as its location");
+ unsigned LocNo = std::distance(DIILocation.begin(), find(DIILocation, &I));
DIExpression *DIExpr =
- salvageDebugInfoImpl(I, DII->getExpression(), StackValue);
+ salvageDebugInfoImpl(I, DII->getExpression(), StackValue, LocNo);
// salvageDebugInfoImpl should fail on examining the first element of
// DbgUsers, or none of them.
@@ -1778,7 +1838,7 @@ void llvm::salvageDebugInfoForDbgValues(
DIExpression *llvm::salvageDebugInfoImpl(Instruction &I,
DIExpression *SrcDIExpr,
- bool WithStackValue) {
+ bool WithStackValue, unsigned LocNo) {
auto &M = *I.getModule();
auto &DL = M.getDataLayout();
@@ -1786,7 +1846,7 @@ DIExpression *llvm::salvageDebugInfoImpl(Instruction &I,
auto doSalvage = [&](SmallVectorImpl<uint64_t> &Ops) -> DIExpression * {
DIExpression *DIExpr = SrcDIExpr;
if (!Ops.empty()) {
- DIExpr = DIExpression::prependOpcodes(DIExpr, Ops, WithStackValue);
+ DIExpr = DIExpression::appendOpsToArg(DIExpr, Ops, LocNo, WithStackValue);
}
return DIExpr;
};
diff --git a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
index 784d0e437ba0..2c68e4b3c32e 100644
--- a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
+++ b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
@@ -384,11 +384,14 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
// possible or create a clone in the OldPreHeader if not.
Instruction *LoopEntryBranch = OrigPreheader->getTerminator();
- // Record all debug intrinsics preceding LoopEntryBranch to avoid duplication.
+ // Record all debug intrinsics preceding LoopEntryBranch to avoid
+ // duplication.
using DbgIntrinsicHash =
- std::pair<std::pair<Value *, DILocalVariable *>, DIExpression *>;
+ std::pair<std::pair<hash_code, DILocalVariable *>, DIExpression *>;
auto makeHash = [](DbgVariableIntrinsic *D) -> DbgIntrinsicHash {
- return {{D->getVariableLocationOp(0), D->getVariable()},
+ auto VarLocOps = D->location_ops();
+ return {{hash_combine_range(VarLocOps.begin(), VarLocOps.end()),
+ D->getVariable()},
D->getExpression()};
};
SmallDenseSet<DbgIntrinsicHash, 8> DbgIntrinsics;
diff --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp
index e7cfce48f8cf..58936fb2b00c 100644
--- a/llvm/unittests/IR/DebugInfoTest.cpp
+++ b/llvm/unittests/IR/DebugInfoTest.cpp
@@ -183,7 +183,8 @@ TEST(MetadataTest, DeleteInstUsedByDbgValue) {
// Delete %b. The dbg.value should now point to undef.
I.eraseFromParent();
- EXPECT_TRUE(isa<UndefValue>(DVIs[0]->getValue()));
+ EXPECT_EQ(DVIs[0]->getNumVariableLocationOps(), 1u);
+ EXPECT_TRUE(isa<UndefValue>(DVIs[0]->getValue(0)));
}
TEST(DIBuilder, CreateFortranArrayTypeWithAttributes) {
More information about the llvm-commits
mailing list