[llvm] 3bfddc2 - Reapply "[DebugInfo] Handle multiple variable location operands in IR"
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 17 09:46:49 PDT 2021
Author: Stephen Tozer
Date: 2021-03-17T16:45:25Z
New Revision: 3bfddc25931d44da9b26c092f4e15634712b1459
URL: https://github.com/llvm/llvm-project/commit/3bfddc25931d44da9b26c092f4e15634712b1459
DIFF: https://github.com/llvm/llvm-project/commit/3bfddc25931d44da9b26c092f4e15634712b1459.diff
LOG: Reapply "[DebugInfo] Handle multiple variable location operands in IR"
Fixed section of code that iterated through a SmallDenseMap and added
instructions in each iteration, causing non-deterministic code; replaced
SmallDenseMap with MapVector to prevent non-determinism.
This reverts commit 01ac6d1587e8613ba4278786e8341f8b492ac941.
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 407c176d7b85..8e24c86dfcf2 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -2873,11 +2873,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.
@@ -2903,7 +2908,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);
}
};
@@ -7903,18 +7908,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
@@ -7933,30 +7941,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 b1f61c688ebc..ffd4778a4a42 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -1260,7 +1260,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.
@@ -6053,7 +6054,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 d0c1cdb3188e..c8fb8aebe53a 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3586,15 +3586,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) {
@@ -3602,7 +3593,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 =
@@ -3613,6 +3604,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 c71bb199b1ca..ae1ed681d998 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -5829,57 +5829,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;
}
}
@@ -5903,7 +5917,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());
@@ -5923,7 +5938,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 fc42f2c51648..73a3a4063f6b 100644
--- a/llvm/lib/Transforms/Utils/Debugify.cpp
+++ b/llvm/lib/Transforms/Utils/Debugify.cpp
@@ -554,15 +554,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 e899bb13a41a..e1ea7c8e27a9 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.
+ MapVector<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.
@@ -1847,7 +1907,7 @@ bool getSalvageOpsForBinOp(BinaryOperator *BI,
DIExpression *llvm::salvageDebugInfoImpl(Instruction &I,
DIExpression *SrcDIExpr,
- bool WithStackValue) {
+ bool WithStackValue, unsigned LocNo) {
auto &M = *I.getModule();
auto &DL = M.getDataLayout();
@@ -1855,7 +1915,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