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 @@ -203,6 +203,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 @@ -311,9 +311,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 @@ -2855,11 +2855,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. @@ -2885,7 +2890,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); } }; @@ -7876,18 +7881,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 @@ -7906,30 +7914,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 @@ -1251,7 +1251,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. @@ -6044,7 +6045,7 @@ DILocalVariable *Variable = DI.getVariable(); DIExpression *Expression = DI.getExpression(); dropDanglingDebugInfo(Variable, Expression); - SmallVector 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/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp --- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp +++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp @@ -2173,7 +2173,7 @@ Storage = StInst->getOperand(0); } else if (auto *GEPInst = dyn_cast(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 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -3576,15 +3576,6 @@ 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(DII)) - return false; - - return true; - }; - SmallVector DIIClones; SmallSet SunkVariables; for (auto User : DbgUsersToSink) { @@ -3592,7 +3583,7 @@ // 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(User)) continue; DebugVariable DbgUserVariable = @@ -3603,6 +3594,8 @@ 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 @@ -1218,10 +1218,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); @@ -1297,13 +1297,18 @@ } 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 &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(); } 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 @@ -5826,57 +5826,71 @@ AU.addPreserved(); } -using EqualValues = SmallVector, 4>; -using EqualValuesMap = DenseMap; +using EqualValues = SmallVector, 4>; +using EqualValuesMap = + 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; - 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 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 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(DVI->getVariableLocationOp(0))) + if (!isa_and_nonnull(DVI->getVariableLocationOp(Idx))) continue; for (auto EV : A.second) { - auto V = std::get(EV); - if (!V) + 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(DVI->getVariableLocationOp(0), V); + DVI->replaceVariableLocationOp(Idx, EVHandle); if (Offset) { SmallVector Ops; DIExpression::appendOffset(Ops, Offset); - DbgDIExpr = DIExpression::prependOpcodes(DbgDIExpr, Ops, true); + DbgDIExpr = DIExpression::appendOpsToArg(DbgDIExpr, Ops, Idx, true); } DVI->setExpression(DbgDIExpr); + DbgValueToExpression[DVI] = DbgDIExpr; break; } } @@ -5900,7 +5914,8 @@ // Debug preservation - before we start removing anything create equivalence // sets for the llvm.dbg.value intrinsics. EqualValuesMap DbgValueToEqualSet; - DbgGatherEqualValues(L, SE, DbgValueToEqualSet); + ExpressionMap DbgValueToExpression; + DbgGatherEqualValues(L, SE, DbgValueToEqualSet, DbgValueToExpression); // Remove any extra phis created by processing inner loops. Changed |= DeleteDeadPHIs(L->getHeader(), &TLI, MSSAU.get()); @@ -5920,7 +5935,7 @@ } } - 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 @@ -266,11 +266,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 @@ -407,7 +407,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(), @@ -416,10 +417,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 @@ -1511,20 +1511,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 @@ -492,15 +492,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 @@ -411,7 +411,7 @@ return true; } if (DbgValueInst *DVI = dyn_cast(I)) { - if (DVI->getValue()) + if (DVI->hasArgList() || DVI->getValue(0)) return false; return true; } @@ -1360,7 +1360,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; } @@ -1387,13 +1387,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; } @@ -1596,17 +1602,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. @@ -1616,15 +1631,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 @@ -1665,11 +1692,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, @@ -1678,11 +1719,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, @@ -1752,9 +1807,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. @@ -1778,7 +1838,7 @@ DIExpression *llvm::salvageDebugInfoImpl(Instruction &I, DIExpression *SrcDIExpr, - bool WithStackValue) { + bool WithStackValue, unsigned LocNo) { auto &M = *I.getModule(); auto &DL = M.getDataLayout(); @@ -1786,7 +1846,7 @@ 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 @@ -384,11 +384,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(), 1u); + EXPECT_TRUE(isa(DVIs[0]->getValue(0))); } TEST(DIBuilder, CreateFortranArrayTypeWithAttributes) {