[llvm] 01ac6d1 - Revert "[DebugInfo] Handle multiple variable location operands in IR"
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 17 05:44:30 PDT 2021
Author: Hans Wennborg
Date: 2021-03-17T13:36:48+01:00
New Revision: 01ac6d1587e8613ba4278786e8341f8b492ac941
URL: https://github.com/llvm/llvm-project/commit/01ac6d1587e8613ba4278786e8341f8b492ac941
DIFF: https://github.com/llvm/llvm-project/commit/01ac6d1587e8613ba4278786e8341f8b492ac941.diff
LOG: Revert "[DebugInfo] Handle multiple variable location operands in IR"
This caused non-deterministic compiler output; see comment on the
code review.
> 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
This reverts commit df69c69427dea7f5b3b3a4d4564bc77b0926ec88.
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 4a2a747dd4bb..a138e4bea8c0 100644
--- a/llvm/include/llvm/IR/IntrinsicInst.h
+++ b/llvm/include/llvm/IR/IntrinsicInst.h
@@ -203,7 +203,6 @@ 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 f7efeeb56fd3..dfcf289a30ec 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -311,10 +311,9 @@ 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. \p LocNo: the index of the location operand to
-/// which \p I applies, should be 0 for debug info without a DIArgList.
+/// appended to the expression.
DIExpression *salvageDebugInfoImpl(Instruction &I, DIExpression *DIExpr,
- bool StackVal, unsigned LocNo);
+ bool StackVal);
/// 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 8e24c86dfcf2..407c176d7b85 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -2873,16 +2873,11 @@ 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), New(New) {
+ UsesReplacer(Instruction *Inst, Value *New) : TypePromotionAction(Inst) {
LLVM_DEBUG(dbgs() << "Do: UsersReplacer: " << *Inst << " with " << *New
<< "\n");
// Record the original uses.
@@ -2908,7 +2903,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(New, Inst);
+ DVI->replaceVariableLocationOp(DVI->getVariableLocationOp(0), Inst);
}
};
@@ -7908,21 +7903,18 @@ bool CodeGenPrepare::fixupDbgValue(Instruction *I) {
DbgValueInst &DVI = *cast<DbgValueInst>(I);
// Does this dbg.value refer to a sunk address calculation?
- 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;
+ 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;
}
// A llvm.dbg.value may be using a value before its definition, due to
@@ -7941,51 +7933,30 @@ bool CodeGenPrepare::placeDbgValues(Function &F) {
if (!DVI)
continue;
- 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;
+ Instruction *VI = dyn_cast_or_null<Instruction>(DVI->getValue());
- // 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 || VI->isTerminator())
+ 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 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 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;
- }
+ // If the defining instruction dominates the dbg.value, we do not need
+ // to move the dbg.value.
+ if (DT.dominates(VI, DVI))
+ continue;
- 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 ffd4778a4a42..b1f61c688ebc 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -1260,8 +1260,7 @@ void SelectionDAGBuilder::salvageUnresolvedDbgValue(DanglingDebugInfo &DDI) {
// variable. FIXME: Further work could recover those too.
while (isa<Instruction>(V)) {
Instruction &VAsInst = *cast<Instruction>(V);
- // Temporary "0", awaiting real implementation.
- DIExpression *NewExpr = salvageDebugInfoImpl(VAsInst, Expr, StackValue, 0);
+ DIExpression *NewExpr = salvageDebugInfoImpl(VAsInst, Expr, StackValue);
// If we cannot salvage any further, and haven't yet found a suitable debug
// expression, bail out.
@@ -6054,7 +6053,7 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
DILocalVariable *Variable = DI.getVariable();
DIExpression *Expression = DI.getExpression();
dropDanglingDebugInfo(Variable, Expression);
- SmallVector<Value *, 4> Values(DI.getValues());
+ SmallVector<Value *> Values(DI.getValues());
if (Values.empty())
return;
diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp
index 55bc314f9ab3..3d3f734ba5e0 100644
--- a/llvm/lib/IR/IntrinsicInst.cpp
+++ b/llvm/lib/IR/IntrinsicInst.cpp
@@ -98,24 +98,6 @@ 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 f4143163ab13..9105c6fbd230 100644
--- a/llvm/lib/IR/User.cpp
+++ b/llvm/lib/IR/User.cpp
@@ -31,10 +31,6 @@ 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 e3938d9fe6fe..793db06f79ad 100644
--- a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
+++ b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp
@@ -563,9 +563,10 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) {
}
if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(I)) {
- for (Value *V : DVI->location_ops())
- if (auto *AI = dyn_cast_or_null<AllocaInst>(V))
- Allocas[AI].DbgVariableIntrinsics.push_back(DVI);
+ if (auto *AI =
+ dyn_cast_or_null<AllocaInst>(DVI->getVariableLocationOp(0))) {
+ Allocas[AI].DbgVariableIntrinsics.push_back(DVI);
+ }
continue;
}
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index a6c6c4adb87f..accd3a6ce16a 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, 0);
+ /*WithStackValue=*/false);
if (!Expr)
return;
Storage = GEPInst->getOperand(0);
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index c8fb8aebe53a..d0c1cdb3188e 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3586,6 +3586,15 @@ 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) {
@@ -3593,7 +3602,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 (isa<DbgDeclareInst>(User))
+ if (updateDbgDeclare(User))
continue;
DebugVariable DbgUserVariable =
@@ -3604,8 +3613,6 @@ 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 27715ff86ff2..e02076c74420 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 *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 (auto *DDI = dyn_cast<DbgVariableIntrinsic>(&Inst))
+ if (auto *Alloca =
+ dyn_cast_or_null<AllocaInst>(DDI->getVariableLocationOp(0)))
+ AllocaDbgMap[Alloca].push_back(DDI);
if (InstrumentLandingPads && isa<LandingPadInst>(Inst))
LandingPadVec.push_back(&Inst);
@@ -1297,18 +1297,13 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) {
}
if (!AllocaToPaddedAllocaMap.empty()) {
- 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 &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 &P : AllocaToPaddedAllocaMap)
P.first->eraseFromParent();
}
diff --git a/llvm/lib/Transforms/Scalar/ADCE.cpp b/llvm/lib/Transforms/Scalar/ADCE.cpp
index 6a31e71d80d0..c8fb9d106e32 100644
--- a/llvm/lib/Transforms/Scalar/ADCE.cpp
+++ b/llvm/lib/Transforms/Scalar/ADCE.cpp
@@ -521,14 +521,10 @@ 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?
- for (Value *V : DII->location_ops()) {
- if (Instruction *II = dyn_cast<Instruction>(V)) {
- if (isLive(II)) {
+ if (Value *V = DII->getVariableLocationOp(0))
+ 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 ae1ed681d998..c71bb199b1ca 100644
--- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -5829,71 +5829,57 @@ void LoopStrengthReduce::getAnalysisUsage(AnalysisUsage &AU) const {
AU.addPreserved<MemorySSAWrapperPass>();
}
-using EqualValues = SmallVector<std::tuple<WeakVH, int64_t>, 4>;
-using EqualValuesMap =
- DenseMap<std::pair<DbgValueInst *, unsigned>, EqualValues>;
-using ExpressionMap = DenseMap<DbgValueInst *, DIExpression *>;
+using EqualValues = SmallVector<std::tuple<WeakVH, int64_t, DIExpression *>, 4>;
+using EqualValuesMap = DenseMap<DbgValueInst *, EqualValues>;
static void DbgGatherEqualValues(Loop *L, ScalarEvolution &SE,
- EqualValuesMap &DbgValueToEqualSet,
- ExpressionMap &DbgValueToExpression) {
+ EqualValuesMap &DbgValueToEqualSet) {
for (auto &B : L->getBlocks()) {
for (auto &I : *B) {
auto DVI = dyn_cast<DbgValueInst>(&I);
if (!DVI)
continue;
- 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()))
+ 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;
- 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();
+ 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(), DVI->getExpression()));
}
+ DbgValueToEqualSet[DVI] = std::move(EqSet);
}
}
}
-static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet,
- ExpressionMap &DbgValueToExpression) {
+static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet) {
for (auto A : DbgValueToEqualSet) {
- auto DVI = A.first.first;
- auto Idx = A.first.second;
+ auto DVI = A.first;
// Only update those that are now undef.
- if (!isa_and_nonnull<UndefValue>(DVI->getVariableLocationOp(Idx)))
+ if (!isa_and_nonnull<UndefValue>(DVI->getVariableLocationOp(0)))
continue;
for (auto EV : A.second) {
- auto EVHandle = std::get<WeakVH>(EV);
- if (!EVHandle)
+ auto V = std::get<WeakVH>(EV);
+ if (!V)
continue;
- // 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 DbgDIExpr = std::get<DIExpression *>(EV);
auto Offset = std::get<int64_t>(EV);
- DVI->replaceVariableLocationOp(Idx, EVHandle);
+ DVI->replaceVariableLocationOp(DVI->getVariableLocationOp(0), V);
if (Offset) {
SmallVector<uint64_t, 8> Ops;
DIExpression::appendOffset(Ops, Offset);
- DbgDIExpr = DIExpression::appendOpsToArg(DbgDIExpr, Ops, Idx, true);
+ DbgDIExpr = DIExpression::prependOpcodes(DbgDIExpr, Ops, true);
}
DVI->setExpression(DbgDIExpr);
- DbgValueToExpression[DVI] = DbgDIExpr;
break;
}
}
@@ -5917,8 +5903,7 @@ 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;
- ExpressionMap DbgValueToExpression;
- DbgGatherEqualValues(L, SE, DbgValueToEqualSet, DbgValueToExpression);
+ DbgGatherEqualValues(L, SE, DbgValueToEqualSet);
// Remove any extra phis created by processing inner loops.
Changed |= DeleteDeadPHIs(L->getHeader(), &TLI, MSSAU.get());
@@ -5938,7 +5923,7 @@ static bool ReduceLoopStrength(Loop *L, IVUsers &IU, ScalarEvolution &SE,
}
}
- DbgApplyEqualValues(DbgValueToEqualSet, DbgValueToExpression);
+ DbgApplyEqualValues(DbgValueToEqualSet);
return Changed;
}
diff --git a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
index 809e62b330dd..0cb9771b6e86 100644
--- a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
+++ b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
@@ -266,13 +266,11 @@ 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)) {
- 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;
- });
+ if (const auto *I =
+ dyn_cast_or_null<Instruction>(DVI->getVariableLocationOp(0)))
+ 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 d1a3ae5c0ed0..9697e7d72c2c 100644
--- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
+++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
@@ -407,8 +407,7 @@ static bool removeRedundantDbgInstrsUsingBackwardScan(BasicBlock *BB) {
/// - Keep track of non-overlapping fragments.
static bool removeRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) {
SmallVector<DbgValueInst *, 8> ToBeRemoved;
- DenseMap<DebugVariable, std::pair<SmallVector<Value *, 4>, DIExpression *>>
- VariableMap;
+ DenseMap<DebugVariable, std::pair<Value *, DIExpression *> > VariableMap;
for (auto &I : *BB) {
if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(&I)) {
DebugVariable Key(DVI->getVariable(),
@@ -417,10 +416,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.
- SmallVector<Value *, 4> Values(DVI->getValues());
- if (VMI == VariableMap.end() || VMI->second.first != Values ||
+ if (VMI == VariableMap.end() ||
+ VMI->second.first != DVI->getValue() ||
VMI->second.second != DVI->getExpression()) {
- VariableMap[Key] = {Values, DVI->getExpression()};
+ VariableMap[Key] = { DVI->getValue(), 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 4ad33c14585d..2b0ee77f4c9b 100644
--- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -1511,19 +1511,20 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc,
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 location isn't a constant or an instruction, delete the
+ // intrinsic.
auto *DVI = cast<DbgVariableIntrinsic>(DII);
- // If any of the used locations are invalid, delete the intrinsic.
- if (any_of(DVI->location_ops(), IsInvalidLocation)) {
+ Value *Location = DVI->getVariableLocationOp(0);
+ if (!Location ||
+ (!isa<Constant>(Location) && !isa<Instruction>(Location))) {
+ DebugIntrinsicsToDelete.push_back(DVI);
+ continue;
+ }
+
+ // 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) {
DebugIntrinsicsToDelete.push_back(DVI);
continue;
}
diff --git a/llvm/lib/Transforms/Utils/Debugify.cpp b/llvm/lib/Transforms/Utils/Debugify.cpp
index 73a3a4063f6b..fc42f2c51648 100644
--- a/llvm/lib/Transforms/Utils/Debugify.cpp
+++ b/llvm/lib/Transforms/Utils/Debugify.cpp
@@ -554,16 +554,15 @@ 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 64c204227bc4..e899bb13a41a 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->hasArgList() || DVI->getValue(0))
+ if (DVI->getValue())
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(is_contained(DVI->getValues(), APN));
+ assert(DVI->getValue() == APN);
if ((DVI->getVariable() == DIVar) && (DVI->getExpression() == DIExpr))
return true;
}
@@ -1387,19 +1387,13 @@ 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()) {
- // 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 (DII->isAddressOfVariable())
+ 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;
}
@@ -1602,26 +1596,17 @@ void llvm::insertDebugValuesForPHIs(BasicBlock *BB,
ValueToValueMapTy DbgValueMap;
for (auto &I : *BB) {
if (auto DbgII = dyn_cast<DbgVariableIntrinsic>(&I)) {
- for (Value *V : DbgII->location_ops())
- if (auto *Loc = dyn_cast_or_null<PHINode>(V))
- DbgValueMap.insert({Loc, DbgII});
+ if (auto *Loc =
+ dyn_cast_or_null<PHINode>(DbgII->getVariableLocationOp(0)))
+ 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, 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.
+ // previously mapped PHIs. If so, insert a new dbg.value intrinsic that will
+ // propagate the info through the new PHI.
for (auto PHI : InsertedPHIs) {
BasicBlock *Parent = PHI->getParent();
// Avoid inserting an intrinsic into an EH block.
@@ -1631,27 +1616,15 @@ void llvm::insertDebugValuesForPHIs(BasicBlock *BB,
auto V = DbgValueMap.find(VI);
if (V != DbgValueMap.end()) {
auto *DbgII = cast<DbgVariableIntrinsic>(V->second);
- 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);
+ 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);
}
}
}
- // 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
@@ -1692,25 +1665,11 @@ void llvm::findDbgValues(SmallVectorImpl<DbgValueInst *> &DbgValues, Value *V) {
// DenseMap lookup.
if (!V->isUsedByMetadata())
return;
- // 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)) {
+ 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,
@@ -1719,25 +1678,11 @@ void llvm::findDbgUsers(SmallVectorImpl<DbgVariableIntrinsic *> &DbgUsers,
// DenseMap lookup.
if (!V->isUsedByMetadata())
return;
- // 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)) {
+ 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,
@@ -1807,14 +1752,9 @@ 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, LocNo);
+ salvageDebugInfoImpl(I, DII->getExpression(), StackValue);
// salvageDebugInfoImpl should fail on examining the first element of
// DbgUsers, or none of them.
@@ -1907,7 +1847,7 @@ bool getSalvageOpsForBinOp(BinaryOperator *BI,
DIExpression *llvm::salvageDebugInfoImpl(Instruction &I,
DIExpression *SrcDIExpr,
- bool WithStackValue, unsigned LocNo) {
+ bool WithStackValue) {
auto &M = *I.getModule();
auto &DL = M.getDataLayout();
@@ -1915,7 +1855,7 @@ DIExpression *llvm::salvageDebugInfoImpl(Instruction &I,
auto doSalvage = [&](SmallVectorImpl<uint64_t> &Ops) -> DIExpression * {
DIExpression *DIExpr = SrcDIExpr;
if (!Ops.empty()) {
- DIExpr = DIExpression::appendOpsToArg(DIExpr, Ops, LocNo, WithStackValue);
+ DIExpr = DIExpression::prependOpcodes(DIExpr, Ops, WithStackValue);
}
return DIExpr;
};
diff --git a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
index 2c68e4b3c32e..784d0e437ba0 100644
--- a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
+++ b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
@@ -384,14 +384,11 @@ 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<hash_code, DILocalVariable *>, DIExpression *>;
+ std::pair<std::pair<Value *, DILocalVariable *>, DIExpression *>;
auto makeHash = [](DbgVariableIntrinsic *D) -> DbgIntrinsicHash {
- auto VarLocOps = D->location_ops();
- return {{hash_combine_range(VarLocOps.begin(), VarLocOps.end()),
- D->getVariable()},
+ return {{D->getVariableLocationOp(0), D->getVariable()},
D->getExpression()};
};
SmallDenseSet<DbgIntrinsicHash, 8> DbgIntrinsics;
diff --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp
index 58936fb2b00c..e7cfce48f8cf 100644
--- a/llvm/unittests/IR/DebugInfoTest.cpp
+++ b/llvm/unittests/IR/DebugInfoTest.cpp
@@ -183,8 +183,7 @@ TEST(MetadataTest, DeleteInstUsedByDbgValue) {
// Delete %b. The dbg.value should now point to undef.
I.eraseFromParent();
- EXPECT_EQ(DVIs[0]->getNumVariableLocationOps(), 1u);
- EXPECT_TRUE(isa<UndefValue>(DVIs[0]->getValue(0)));
+ EXPECT_TRUE(isa<UndefValue>(DVIs[0]->getValue()));
}
TEST(DIBuilder, CreateFortranArrayTypeWithAttributes) {
More information about the llvm-commits
mailing list