diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h --- a/llvm/include/llvm/IR/IntrinsicInst.h +++ b/llvm/include/llvm/IR/IntrinsicInst.h @@ -180,6 +180,7 @@ 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 --- a/llvm/include/llvm/Transforms/Utils/Local.h +++ b/llvm/include/llvm/Transforms/Utils/Local.h @@ -312,9 +312,10 @@ /// 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 --- a/llvm/lib/CodeGen/CodeGenPrepare.cpp +++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp @@ -2775,11 +2775,16 @@ /// Keep track of the debug users. SmallVector 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::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. @@ -2808,7 +2813,7 @@ // 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); } }; @@ -7730,18 +7735,21 @@ DbgValueInst &DVI = *cast(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 difference 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 difference 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 @@ -7760,30 +7768,51 @@ if (!DVI) continue; - Instruction *VI = dyn_cast_or_null(DVI->getValue()); + SmallVector VIs; + for (Value *V : DVI->getValues()) + if (Instruction *VI = dyn_cast_or_null(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(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(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(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(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 --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -1217,11 +1217,7 @@ << "in EmitFuncArgumentDbgValue\n"); } else { LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI << "\n"); -<<<<<<< HEAD - auto Undef = UndefValue::get(DDI.getDI()->getValue()->getType()); -======= auto Undef = UndefValue::get(DDI.getDI()->getValue(0)->getType()); ->>>>>>> 8c5ce9d8aad6... D88585 auto SDV = DAG.getConstantDbgValue(Variable, Expr, Undef, dl, DbgSDNodeOrder); DAG.AddDbgValue(SDV, false); @@ -1253,7 +1249,8 @@ // variable. FIXME: Further work could recover those too. while (isa(V)) { Instruction &VAsInst = *cast(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. @@ -6027,7 +6024,7 @@ DILocalVariable *Variable = DI.getVariable(); DIExpression *Expression = DI.getExpression(); dropDanglingDebugInfo(Variable, Expression); - auto Values = DI.getValues(); + SmallVector Values(DI.getValues()); if (Values.empty()) return; diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp --- a/llvm/lib/IR/IntrinsicInst.cpp +++ b/llvm/lib/IR/IntrinsicInst.cpp @@ -98,6 +98,24 @@ 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(NewValue) + ? NewValue + : MetadataAsValue::get( + getContext(), ValueAsMetadata::get(NewValue)); + return setArgOperand(0, NewOperand); + } + SmallVector 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 DbgVariableIntrinsic::getFragmentSizeInBits() const { if (auto Fragment = getExpression()->getFragmentInfo()) diff --git a/llvm/lib/IR/User.cpp b/llvm/lib/IR/User.cpp --- a/llvm/lib/IR/User.cpp +++ b/llvm/lib/IR/User.cpp @@ -31,6 +31,10 @@ // most importantly, removing "this" from the use list of "From". setOperand(i, To); } + if (auto DVI = dyn_cast_or_null(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 --- a/llvm/lib/Target/AArch64/AArch64StackTagging.cpp +++ b/llvm/lib/Target/AArch64/AArch64StackTagging.cpp @@ -563,10 +563,9 @@ } if (auto *DVI = dyn_cast(I)) { - if (auto *AI = - dyn_cast_or_null(DVI->getVariableLocationOp(0))) { - Allocas[AI].DbgVariableIntrinsics.push_back(DVI); - } + for (Value *V : DVI->location_ops()) + if (auto *AI = dyn_cast_or_null(V)) + Allocas[AI].DbgVariableIntrinsics.push_back(DVI); continue; } diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -3541,28 +3541,18 @@ SmallVector DbgUsers; findDbgUsers(DbgUsers, I); - // Update the arguments of a dbg.declare instruction, so that it - // does not point into a sunk instruction. - auto updateDbgDeclare = [&I](DbgVariableIntrinsic *DII) { - if (!isa(DII)) - return false; - - if (isa(I)) - DII->replaceVariableLocationOp(DII->getVariableLocationOp(0), - I->getOperand(0)); - return true; - }; - SmallVector DIIClones; for (auto User : DbgUsers) { // A dbg.declare instruction should not be cloned, since there can only be // 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 (User->getParent() != SrcBlock || updateDbgDeclare(User)) + if (User->getParent() != SrcBlock || isa(User)) continue; DIIClones.emplace_back(cast(User->clone())); + if (isa(User) && isa(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 --- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp @@ -1216,10 +1216,10 @@ isa(Inst)) RetVec.push_back(&Inst); - if (auto *DDI = dyn_cast(&Inst)) - if (auto *Alloca = - dyn_cast_or_null(DDI->getVariableLocationOp(0))) - AllocaDbgMap[Alloca].push_back(DDI); + if (auto *DVI = dyn_cast(&Inst)) + for (Value *V : DVI->location_ops()) + if (auto *Alloca = dyn_cast_or_null(V)) + AllocaDbgMap[Alloca].push_back(DVI); if (InstrumentLandingPads && isa(Inst)) LandingPadVec.push_back(&Inst); @@ -1295,15 +1295,20 @@ } if (!AllocaToPaddedAllocaMap.empty()) { - for (auto &BB : F) - for (auto &Inst : BB) - if (auto *DVI = dyn_cast(&Inst)) - if (auto *AI = - dyn_cast_or_null(DVI->getVariableLocationOp(0))) - if (auto *NewAI = AllocaToPaddedAllocaMap.lookup(AI)) - DVI->replaceVariableLocationOp(AI, NewAI); - for (auto &P : AllocaToPaddedAllocaMap) - P.first->eraseFromParent(); + for (auto &BB : F) { + for (auto &Inst : BB) { + if (auto *DVI = dyn_cast(&Inst)) { + for (Value *V : DVI->location_ops()) { + if (auto *AI = dyn_cast_or_null(V)) { + if (auto *NewAI = AllocaToPaddedAllocaMap.lookup(AI)) + DVI->replaceVariableLocationOp(V, NewAI); + } + } + } + } + for (auto &P : AllocaToPaddedAllocaMap) + P.first->eraseFromParent(); + } } // If we split the entry block, move any allocas that were originally in the diff --git a/llvm/lib/Transforms/Scalar/ADCE.cpp b/llvm/lib/Transforms/Scalar/ADCE.cpp --- a/llvm/lib/Transforms/Scalar/ADCE.cpp +++ b/llvm/lib/Transforms/Scalar/ADCE.cpp @@ -521,10 +521,14 @@ // 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(V)) - if (isLive(II)) + for (Value *V : DII->location_ops()) { + if (Instruction *II = dyn_cast(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 --- a/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp +++ b/llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp @@ -5773,22 +5773,25 @@ AU.addPreserved(); } -using EqualValues = SmallVector, 4>; +using EqualValues = SmallVector, 4>; using EqualValuesMap = - DenseMap, EqualValues>; + DenseMap, EqualValues>; +using ExpressionMap = DenseMap; 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(&I); if (!DVI) continue; - for (auto V : DVI->location_ops()) { + 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; - if (DbgValueToEqualSet.count({DVI, V})) - continue; auto DbgValueSCEV = SE.getSCEV(V); EqualValues EqSet; for (PHINode &Phi : L->getHeader()->phis()) { @@ -5800,35 +5803,42 @@ Optional Offset = SE.computeConstantDifference(DbgValueSCEV, PhiSCEV); if (Offset && Offset->getMinSignedBits() <= 64) - EqSet.emplace_back(std::make_tuple( - &Phi, Offset.getValue().getSExtValue(), DVI->getExpression())); + EqSet.emplace_back( + std::make_tuple(&Phi, Offset.getValue().getSExtValue())); } - DbgValueToEqualSet[{DVI, V}] = std::move(EqSet); + DbgValueToEqualSet[{DVI, Idx}] = std::move(EqSet); + DbgValueToExpression[DVI] = DVI->getExpression(); } } } } -static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet) { +static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet, + ExpressionMap &DbgValueToExpression) { for (auto A : DbgValueToEqualSet) { auto DVI = A.first.first; - auto V = A.first.second; + auto Idx = A.first.second; // Only update those that are now undef. - if (!isa_and_nonnull(V)) + if (!isa_and_nonnull(DVI->getVariableLocationOp(Idx))) continue; for (auto EV : A.second) { auto EVHandle = std::get(EV); if (!EVHandle) continue; - auto DbgDIExpr = std::get(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(EV); - DVI->replaceVariableLocationOp(V, EVHandle); + auto &Ctx = DVI->getContext(); + DVI->replaceVariableLocationOp(Idx, EVHandle); if (Offset) { SmallVector Ops; DIExpression::appendOffset(Ops, Offset); - DbgDIExpr = DIExpression::appendOpsToArg(DbgDIExpr, Ops, 0); + DbgDIExpr = DIExpression::appendOpsToArg(DbgDIExpr, Ops, Idx, true); } DVI->setExpression(DbgDIExpr); + DbgValueToExpression[DVI] = DbgDIExpr; break; } } @@ -5852,7 +5862,8 @@ // 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()); @@ -5872,7 +5883,7 @@ } } - DbgApplyEqualValues(DbgValueToEqualSet); + DbgApplyEqualValues(DbgValueToEqualSet, DbgValueToExpression); return Changed; } diff --git a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp --- a/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp +++ b/llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp @@ -265,11 +265,13 @@ const auto AllPrecedingUsesFromBlockHoisted = [&NotHoisted](const User *U) { // Debug variable has special operand to check it's not hoisted. if (const auto *DVI = dyn_cast(U)) { - if (const auto *I = - dyn_cast_or_null(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(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 --- a/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp +++ b/llvm/lib/Transforms/Utils/BasicBlockUtils.cpp @@ -409,7 +409,8 @@ /// - Keep track of non-overlapping fragments. static bool removeRedundantDbgInstrsUsingForwardScan(BasicBlock *BB) { SmallVector ToBeRemoved; - DenseMap > VariableMap; + DenseMap, DIExpression *>> + VariableMap; for (auto &I : *BB) { if (DbgValueInst *DVI = dyn_cast(&I)) { DebugVariable Key(DVI->getVariable(), @@ -418,10 +419,10 @@ 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 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 --- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp +++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp @@ -1513,20 +1513,19 @@ continue; } - // If the location isn't a constant or an instruction, delete the - // intrinsic. - auto *DVI = cast(DII); - Value *Location = DVI->getVariableLocationOp(0); - if (!Location || - (!isa(Location) && !isa(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(Location) && !isa(Location))) + return true; + Instruction *LocationInst = dyn_cast(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(Location); - if (LocationInst && LocationInst->getFunction() != &NewFunc) { + auto *DVI = cast(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 --- a/llvm/lib/Transforms/Utils/Debugify.cpp +++ b/llvm/lib/Transforms/Utils/Debugify.cpp @@ -266,15 +266,16 @@ // // 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 DbgVarSize = DVI->getFragmentSizeInBits(); diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -410,7 +410,7 @@ return true; } if (DbgValueInst *DVI = dyn_cast(I)) { - if (DVI->getValue()) + if (DVI->hasArgList() || DVI->getValue(0)) return false; return true; } @@ -1330,7 +1330,7 @@ SmallVector 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; } @@ -1357,13 +1357,19 @@ // 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(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(DII->getVariableLocationOp(0))) { if (Optional 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; } @@ -1566,17 +1572,26 @@ ValueToValueMapTy DbgValueMap; for (auto &I : *BB) { if (auto DbgII = dyn_cast(&I)) { - if (auto *Loc = - dyn_cast_or_null(DbgII->getVariableLocationOp(0))) - DbgValueMap.insert({Loc, DbgII}); + for (Value *V : DbgII->location_ops()) + if (auto *Loc = dyn_cast_or_null(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, + 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. @@ -1586,15 +1601,27 @@ auto V = DbgValueMap.find(VI); if (V != DbgValueMap.end()) { auto *DbgII = cast(V->second); - DbgVariableIntrinsic *NewDbgII = - cast(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(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 @@ -1635,11 +1662,25 @@ // 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 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(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(U)) + if (EncounteredDbgValues.insert(DVI).second) + DbgValues.push_back(DVI); + } + } + } } void llvm::findDbgUsers(SmallVectorImpl &DbgUsers, @@ -1648,11 +1689,25 @@ // 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 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(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(U)) + if (EncounteredDbgValues.insert(DII).second) + DbgUsers.push_back(DII); + } + } + } } bool llvm::replaceDbgDeclare(Value *Address, Value *NewAddress, @@ -1729,9 +1784,14 @@ // are implicitly pointing out the value as a DWARF memory location // description. bool StackValue = isa(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. @@ -1755,7 +1815,7 @@ DIExpression *llvm::salvageDebugInfoImpl(Instruction &I, DIExpression *SrcDIExpr, - bool WithStackValue) { + bool WithStackValue, unsigned LocNo) { auto &M = *I.getModule(); auto &DL = M.getDataLayout(); @@ -1763,7 +1823,7 @@ auto doSalvage = [&](SmallVectorImpl &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 --- a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp +++ b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp @@ -376,11 +376,14 @@ // 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, DIExpression *>; + std::pair, 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 DbgIntrinsics; diff --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp --- a/llvm/unittests/IR/DebugInfoTest.cpp +++ b/llvm/unittests/IR/DebugInfoTest.cpp @@ -183,7 +183,8 @@ // Delete %b. The dbg.value should now point to undef. I.eraseFromParent(); - EXPECT_TRUE(isa(DVIs[0]->getValue())); + EXPECT_EQ(DVIs[0]->getNumVariableLocationOps(), 1); + EXPECT_TRUE(isa(DVIs[0]->getValue(0))); } TEST(DIBuilder, CreateFortranArrayTypeWithAttributes) { diff --git a/llvm/unittests/Transforms/Utils/LocalTest.cpp b/llvm/unittests/Transforms/Utils/LocalTest.cpp --- a/llvm/unittests/Transforms/Utils/LocalTest.cpp +++ b/llvm/unittests/Transforms/Utils/LocalTest.cpp @@ -524,7 +524,9 @@ } bool doesDebugValueDescribeX(const DbgValueInst &DI) { - const auto &CI = *cast(DI.getValue()); + if (DI.getNumVariableLocationOps() != 1) + return false; + const auto &CI = *cast(DI.getValue(0)); if (CI.isZero()) return DI.getExpression()->getElements().equals( {dwarf::DW_OP_plus_uconst, 1, dwarf::DW_OP_stack_value}); @@ -534,7 +536,9 @@ } bool doesDebugValueDescribeY(const DbgValueInst &DI) { - const auto &CI = *cast(DI.getValue()); + if (DI.getNumVariableLocationOps() != 1) + return false; + const auto &CI = *cast(DI.getValues(0)); if (CI.isZero()) return DI.getExpression()->getElements().equals( {dwarf::DW_OP_plus_uconst, 1, dwarf::DW_OP_plus_uconst, 2, @@ -758,14 +762,15 @@ EXPECT_TRUE(replaceAllDbgUsesWith(A, F_, F_, DT)); auto *ADbgVal = cast(A.getNextNode()); - EXPECT_EQ(ConstantInt::get(A.getType(), 0), - ADbgVal->getVariableLocationOp(0)); + EXPECT_EQ(ADbgVal->getNumVariableLocationOps(), 1); + EXPECT_EQ(ConstantInt::get(A.getType(), 0), ADbgVal->getValues(0)); // Introduce a use-before-def. Check that the dbg.values for %f become undef. EXPECT_TRUE(replaceAllDbgUsesWith(F_, G, G, DT)); auto *FDbgVal = cast(F_.getNextNode()); - EXPECT_TRUE(isa(FDbgVal->getVariableLocationOp(0))); + EXPECT_EQ(FDbgVal->getNumVariableLocationOps(), 1); + EXPECT_TRUE(FDbgVal->isUndef()); SmallVector FDbgVals; findDbgValues(FDbgVals, &F_);