From bfb6dad2ae3914f0cdc64d78cabdc8fc5bb1e140 Mon Sep 17 00:00:00 2001 From: gbtozers Date: Wed, 30 Sep 2020 16:30:14 +0100 Subject: [PATCH] [DebugInfo] Support DIArgList in DbgVariableIntrinsic This patch updates DbgVariableIntrinsics to support use of a DIArgList for the location operand, resulting in a significant change to its interface. This patch does not update all IR passes to support multiple location operands in a dbg.value; the only change is to update the DbgVariableIntrinsic interface and its uses. All code outside of the intrinsic classes assumes that an intrinsic will always have exactly one location operand; they will still support DIArgLists, but only if they contain exactly one Value. Among other changes, the setOperand and setArgOperand functions in DbgVariableIntrinsic have been made private. This is to prevent code from setting the operands of these intrinsics directly, which could easily result in incorrect/invalid operands being set. This does not prevent these functions from being called on a debug intrinsic at all, as they can still be called on any CallInst pointer; it is assumed that any code directly setting the operands on a generic call instruction is doing so safely. The intention for making these functions private is to prevent DIArgLists from being overwritten by code that's naively trying to replace one of the Values it points to, and also to fail fast if a DbgVariableIntrinsic is updated to use a DIArgList without a valid corresponding DIExpression. --- include/llvm/IR/IntrinsicInst.h | 121 +++++++++++++++++- lib/Bitcode/Reader/MetadataLoader.cpp | 3 +- lib/CodeGen/CodeGenPrepare.cpp | 12 +- .../SelectionDAG/SelectionDAGBuilder.cpp | 7 +- lib/IR/Constants.cpp | 6 + lib/IR/IntrinsicInst.cpp | 65 ++++++++-- lib/IR/Verifier.cpp | 2 +- lib/Target/AArch64/AArch64StackTagging.cpp | 8 +- lib/Transforms/Coroutines/CoroFrame.cpp | 7 +- .../InstCombine/InstructionCombining.cpp | 4 - .../Instrumentation/HWAddressSanitizer.cpp | 14 +- lib/Transforms/Scalar/ADCE.cpp | 2 +- lib/Transforms/Scalar/LoopStrengthReduce.cpp | 9 +- .../Scalar/SpeculativeExecution.cpp | 2 +- lib/Transforms/Utils/CodeExtractor.cpp | 4 +- lib/Transforms/Utils/LCSSA.cpp | 3 +- lib/Transforms/Utils/Local.cpp | 34 ++--- lib/Transforms/Utils/LoopRotationUtils.cpp | 7 +- unittests/Transforms/Utils/LocalTest.cpp | 14 +- 19 files changed, 234 insertions(+), 90 deletions(-) diff --git a/include/llvm/IR/IntrinsicInst.h b/include/llvm/IR/IntrinsicInst.h index a7dec8837bf..a138e4bea8c 100644 --- a/include/llvm/IR/IntrinsicInst.h +++ b/include/llvm/IR/IntrinsicInst.h @@ -24,6 +24,7 @@ #define LLVM_IR_INTRINSICINST_H #include "llvm/IR/Constants.h" +#include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/FPEnv.h" #include "llvm/IR/Function.h" @@ -146,10 +147,78 @@ public: /// This is the common base class for debug info intrinsics for variables. class DbgVariableIntrinsic : public DbgInfoIntrinsic { public: - /// Get the location corresponding to the variable referenced by the debug + // Iterator for ValueAsMetadata that internally uses direct pointer iteration + // over either a ValueAsMetadata* or a ValueAsMetadata**, dereferencing to the + // ValueAsMetadata . + class location_op_iterator + : public iterator_facade_base { + PointerUnion I; + + public: + location_op_iterator(ValueAsMetadata *SingleIter) : I(SingleIter) {} + location_op_iterator(ValueAsMetadata **MultiIter) : I(MultiIter) {} + + location_op_iterator(const location_op_iterator &R) : I(R.I) {} + location_op_iterator &operator=(const location_op_iterator &R) { + I = R.I; + return *this; + } + bool operator==(const location_op_iterator &RHS) const { + return I == RHS.I; + } + const Value *operator*() const { + ValueAsMetadata *VAM = I.is() + ? I.get() + : *I.get(); + return VAM->getValue(); + }; + Value *operator*() { + ValueAsMetadata *VAM = I.is() + ? I.get() + : *I.get(); + return VAM->getValue(); + } + location_op_iterator &operator++() { + if (I.is()) + I = I.get() + 1; + else + I = I.get() + 1; + return *this; + } + location_op_iterator &operator--() { + if (I.is()) + I = I.get() - 1; + else + I = I.get() - 1; + return *this; + } + }; + + /// Get the locations corresponding to the variable referenced by the debug /// info intrinsic. Depending on the intrinsic, this could be the /// variable's value or its address. - Value *getVariableLocation(bool AllowNullOp = true) const; + iterator_range location_ops() const; + + Value *getVariableLocationOp(unsigned OpIdx) const; + + void replaceVariableLocationOp(Value *OldValue, Value *NewValue); + + void setVariable(DILocalVariable *NewVar) { + setArgOperand(1, MetadataAsValue::get(NewVar->getContext(), NewVar)); + } + + void setExpression(DIExpression *NewExpr) { + setArgOperand(2, MetadataAsValue::get(NewExpr->getContext(), NewExpr)); + } + + unsigned getNumVariableLocationOps() const { + if (hasArgList()) + return cast(getRawLocation())->getArgs().size(); + return 1; + } + + bool hasArgList() const { return isa(getRawLocation()); } /// Does this describe the address of a local variable. True for dbg.addr /// and dbg.declare, but not dbg.value, which describes its value. @@ -157,6 +226,24 @@ public: return getIntrinsicID() != Intrinsic::dbg_value; } + void setUndef() { + // TODO: When/if we remove duplicate values from DIArgLists, we don't need + // this set anymore. + SmallPtrSet RemovedValues; + for (Value *OldValue : location_ops()) { + if (!RemovedValues.insert(OldValue).second) + continue; + Value *Undef = UndefValue::get(OldValue->getType()); + replaceVariableLocationOp(OldValue, Undef); + } + } + + bool isUndef() const { + return (getNumVariableLocationOps() == 0 && + !getExpression()->isComplex()) || + any_of(location_ops(), [](Value *V) { return isa(V); }); + } + DILocalVariable *getVariable() const { return cast(getRawVariable()); } @@ -165,6 +252,10 @@ public: return cast(getRawExpression()); } + Metadata *getRawLocation() const { + return cast(getArgOperand(0))->getMetadata(); + } + Metadata *getRawVariable() const { return cast(getArgOperand(1))->getMetadata(); } @@ -193,12 +284,21 @@ public: return isa(V) && classof(cast(V)); } /// @} +private: + void setArgOperand(unsigned i, Value *v) { + DbgInfoIntrinsic::setArgOperand(i, v); + } + void setOperand(unsigned i, Value *v) { DbgInfoIntrinsic::setOperand(i, v); } }; /// This represents the llvm.dbg.declare instruction. class DbgDeclareInst : public DbgVariableIntrinsic { public: - Value *getAddress() const { return getVariableLocation(); } + Value *getAddress() const { + assert(getNumVariableLocationOps() == 1 && + "dbg.declare must have exactly 1 location operand."); + return getVariableLocationOp(0); + } /// \name Casting methods /// @{ @@ -214,7 +314,11 @@ public: /// This represents the llvm.dbg.addr instruction. class DbgAddrIntrinsic : public DbgVariableIntrinsic { public: - Value *getAddress() const { return getVariableLocation(); } + Value *getAddress() const { + assert(getNumVariableLocationOps() == 1 && + "dbg.addr must have exactly 1 location operand."); + return getVariableLocationOp(0); + } /// \name Casting methods /// @{ @@ -229,8 +333,13 @@ public: /// This represents the llvm.dbg.value instruction. class DbgValueInst : public DbgVariableIntrinsic { public: - Value *getValue() const { - return getVariableLocation(/* AllowNullOp = */ false); + // The default argument should only be used in ISel, and the default option + // should be removed once ISel support for multiple location ops is complete. + Value *getValue(unsigned OpIdx = 0) const { + return getVariableLocationOp(OpIdx); + } + iterator_range getValues() const { + return location_ops(); } /// \name Casting methods diff --git a/lib/Bitcode/Reader/MetadataLoader.cpp b/lib/Bitcode/Reader/MetadataLoader.cpp index 0567f96c9c2..0cf547c57d9 100644 --- a/lib/Bitcode/Reader/MetadataLoader.cpp +++ b/lib/Bitcode/Reader/MetadataLoader.cpp @@ -550,8 +550,7 @@ class MetadataLoader::MetadataLoaderImpl { SmallVector Ops; Ops.append(std::next(DIExpr->elements_begin()), DIExpr->elements_end()); - auto *E = DIExpression::get(Context, Ops); - DDI->setOperand(2, MetadataAsValue::get(Context, E)); + DDI->setExpression(DIExpression::get(Context, Ops)); } } diff --git a/lib/CodeGen/CodeGenPrepare.cpp b/lib/CodeGen/CodeGenPrepare.cpp index f4bf55d773a..6e806b0c73a 100644 --- a/lib/CodeGen/CodeGenPrepare.cpp +++ b/lib/CodeGen/CodeGenPrepare.cpp @@ -2883,11 +2883,8 @@ class TypePromotionTransaction { // including the debug uses. Since we are undoing the replacements, // the original debug uses must also be reinstated to maintain the // correctness and utility of debug value instructions. - for (auto *DVI: DbgValues) { - LLVMContext &Ctx = Inst->getType()->getContext(); - auto *MV = MetadataAsValue::get(Ctx, ValueAsMetadata::get(Inst)); - DVI->setOperand(0, MV); - } + for (auto *DVI : DbgValues) + DVI->replaceVariableLocationOp(DVI->getVariableLocationOp(0), Inst); } }; @@ -7878,7 +7875,7 @@ bool CodeGenPrepare::fixupDbgValue(Instruction *I) { DbgValueInst &DVI = *cast(I); // Does this dbg.value refer to a sunk address calculation? - Value *Location = DVI.getVariableLocation(); + Value *Location = DVI.getVariableLocationOp(0); WeakTrackingVH SunkAddrVH = SunkAddrs[Location]; Value *SunkAddr = SunkAddrVH.pointsToAliveValue() ? SunkAddrVH : nullptr; if (SunkAddr) { @@ -7886,8 +7883,7 @@ bool CodeGenPrepare::fixupDbgValue(Instruction *I) { // 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.setOperand(0, MetadataAsValue::get(DVI.getContext(), - ValueAsMetadata::get(SunkAddr))); + DVI.replaceVariableLocationOp(Location, SunkAddr); return true; } return false; diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index 39730d03502..f869b2c41f9 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -1191,8 +1191,7 @@ void SelectionDAGBuilder::resolveDanglingDebugInfo(const Value *V, << "in EmitFuncArgumentDbgValue\n"); } else { LLVM_DEBUG(dbgs() << "Dropping debug info for " << *DI << "\n"); - auto Undef = - UndefValue::get(DDI.getDI()->getVariableLocation()->getType()); + auto Undef = UndefValue::get(DDI.getDI()->getValue()->getType()); auto SDV = DAG.getConstantDbgValue(Variable, Expr, Undef, dl, DbgSDNodeOrder); DAG.AddDbgValue(SDV, nullptr, false); @@ -1246,7 +1245,7 @@ void SelectionDAGBuilder::salvageUnresolvedDbgValue(DanglingDebugInfo &DDI) { // This was the final opportunity to salvage this debug information, and it // couldn't be done. Place an undef DBG_VALUE at this location to terminate // any earlier variable location. - auto Undef = UndefValue::get(DDI.getDI()->getVariableLocation()->getType()); + auto Undef = UndefValue::get(DDI.getDI()->getValue()->getType()); auto SDV = DAG.getConstantDbgValue(Var, Expr, Undef, DL, SDNodeOrder); DAG.AddDbgValue(SDV, nullptr, false); @@ -5885,7 +5884,7 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, LLVM_DEBUG(dbgs() << "SelectionDAG visiting debug intrinsic: " << DI << "\n"); // Check if address has undef value. - const Value *Address = DI.getVariableLocation(); + const Value *Address = DI.getVariableLocationOp(0); if (!Address || isa(Address) || (Address->use_empty() && !isa(Address))) { LLVM_DEBUG(dbgs() << "Dropping debug info for " << DI diff --git a/lib/IR/Constants.cpp b/lib/IR/Constants.cpp index ef0e5f6dd08..62760ff7aec 100644 --- a/lib/IR/Constants.cpp +++ b/lib/IR/Constants.cpp @@ -721,6 +721,12 @@ static bool removeDeadUsersOfConstant(const Constant *C) { return false; // Constant wasn't dead } + // If C is only used by metadata, it should not be preserved but should have + // its uses replaced. + if (C->isUsedByMetadata()) { + const_cast(C)->replaceAllUsesWith( + UndefValue::get(C->getType())); + } const_cast(C)->destroyConstant(); return true; } diff --git a/lib/IR/IntrinsicInst.cpp b/lib/IR/IntrinsicInst.cpp index 3d1ea285359..3d3f734ba5e 100644 --- a/lib/IR/IntrinsicInst.cpp +++ b/lib/IR/IntrinsicInst.cpp @@ -38,18 +38,65 @@ using namespace llvm; /// intrinsics for variables. /// -Value *DbgVariableIntrinsic::getVariableLocation(bool AllowNullOp) const { - Value *Op = getArgOperand(0); - if (AllowNullOp && !Op) +iterator_range +DbgVariableIntrinsic::location_ops() const { + auto *MD = getRawLocation(); + assert(MD && "First operand of DbgVariableIntrinsic should be non-null."); + + // If operand is ValueAsMetadata, return a range over just that operand. + if (auto *VAM = dyn_cast(MD)) { + return {location_op_iterator(VAM), location_op_iterator(VAM + 1)}; + } + // If operand is DIArgList, return a range over its args. + if (auto *AL = dyn_cast(MD)) + return {location_op_iterator(AL->args_begin()), + location_op_iterator(AL->args_end())}; + // Operand must be an empty metadata tuple, so return empty iterator. + return {location_op_iterator(static_cast(nullptr)), + location_op_iterator(static_cast(nullptr))}; +} + +Value *DbgVariableIntrinsic::getVariableLocationOp(unsigned OpIdx) const { + auto *MD = getRawLocation(); + assert(MD && "First operand of DbgVariableIntrinsic should be non-null."); + if (auto *AL = dyn_cast(MD)) + return AL->getArgs()[OpIdx]->getValue(); + if (isa(MD)) return nullptr; + assert( + isa(MD) && + "Attempted to get location operand from DbgVariableIntrinsic with none."); + auto *V = cast(MD); + assert(OpIdx == 0 && "Operand Index must be 0 for a debug intrinsic with a " + "single location operand."); + return V->getValue(); +} - auto *MD = cast(Op)->getMetadata(); - if (auto *V = dyn_cast(MD)) - return V->getValue(); +static ValueAsMetadata *getAsMetadata(Value *V) { + return isa(V) ? dyn_cast( + cast(V)->getMetadata()) + : ValueAsMetadata::get(V); +} - // When the value goes to null, it gets replaced by an empty MDNode. - assert(!cast(MD)->getNumOperands() && "Expected an empty MDNode"); - return nullptr; +void DbgVariableIntrinsic::replaceVariableLocationOp(Value *OldValue, + Value *NewValue) { + assert(NewValue && "Values must be non-null"); + auto Locations = location_ops(); + auto OldIt = find(Locations, OldValue); + assert(OldIt != Locations.end() && "OldValue must be a current location"); + if (!hasArgList()) { + Value *NewOperand = isa(NewValue) + ? NewValue + : MetadataAsValue::get( + getContext(), ValueAsMetadata::get(NewValue)); + return setArgOperand(0, NewOperand); + } + SmallVector MDs; + ValueAsMetadata *NewOperand = getAsMetadata(NewValue); + for (auto *VMD : Locations) + MDs.push_back(VMD == *OldIt ? NewOperand : getAsMetadata(VMD)); + setArgOperand( + 0, MetadataAsValue::get(getContext(), DIArgList::get(getContext(), MDs))); } Optional DbgVariableIntrinsic::getFragmentSizeInBits() const { diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index be8663553b5..b7a002b0573 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -5371,7 +5371,7 @@ void Verifier::visitConstrainedFPIntrinsic(ConstrainedFPIntrinsic &FPI) { } void Verifier::visitDbgIntrinsic(StringRef Kind, DbgVariableIntrinsic &DII) { - auto *MD = cast(DII.getArgOperand(0))->getMetadata(); + auto *MD = DII.getRawLocation(); AssertDI(isa(MD) || isa(MD) || (isa(MD) && !cast(MD)->getNumOperands()), "invalid llvm.dbg." + Kind + " intrinsic address/value", &DII, MD); diff --git a/lib/Target/AArch64/AArch64StackTagging.cpp b/lib/Target/AArch64/AArch64StackTagging.cpp index ab49e0c3f93..793db06f79a 100644 --- a/lib/Target/AArch64/AArch64StackTagging.cpp +++ b/lib/Target/AArch64/AArch64StackTagging.cpp @@ -284,6 +284,7 @@ public: class AArch64StackTagging : public FunctionPass { struct AllocaInfo { AllocaInst *AI; + TrackingVH OldAI; // Track through RAUW to replace debug uses. SmallVector LifetimeStart; SmallVector LifetimeEnd; SmallVector DbgVariableIntrinsics; @@ -557,12 +558,13 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) { Instruction *I = &*IT; if (auto *AI = dyn_cast(I)) { Allocas[AI].AI = AI; + Allocas[AI].OldAI = AI; continue; } if (auto *DVI = dyn_cast(I)) { if (auto *AI = - dyn_cast_or_null(DVI->getVariableLocation())) { + dyn_cast_or_null(DVI->getVariableLocationOp(0))) { Allocas[AI].DbgVariableIntrinsics.push_back(DVI); } continue; @@ -705,9 +707,7 @@ bool AArch64StackTagging::runOnFunction(Function &Fn) { // Fixup debug intrinsics to point to the new alloca. for (auto DVI : Info.DbgVariableIntrinsics) - DVI->setArgOperand( - 0, - MetadataAsValue::get(F->getContext(), LocalAsMetadata::get(Info.AI))); + DVI->replaceVariableLocationOp(Info.OldAI, Info.AI); } // If we have instrumented at least one alloca, all unrecognized lifetime diff --git a/lib/Transforms/Coroutines/CoroFrame.cpp b/lib/Transforms/Coroutines/CoroFrame.cpp index 16b59f279ff..accd3a6ce16 100644 --- a/lib/Transforms/Coroutines/CoroFrame.cpp +++ b/lib/Transforms/Coroutines/CoroFrame.cpp @@ -2156,6 +2156,7 @@ void coro::salvageDebugInfo( // function argument and convert into a DIExpression. bool OutermostLoad = true; Value *Storage = DDI->getAddress(); + Value *OriginalStorage = Storage; while (Storage) { if (auto *LdInst = dyn_cast(Storage)) { Storage = LdInst->getOperand(0); @@ -2206,10 +2207,8 @@ void coro::salvageDebugInfo( if (Expr && Expr->isComplex()) Expr = DIExpression::prepend(Expr, DIExpression::DerefBefore); } - auto &VMContext = DDI->getFunction()->getContext(); - DDI->setOperand( - 0, MetadataAsValue::get(VMContext, ValueAsMetadata::get(Storage))); - DDI->setOperand(2, MetadataAsValue::get(VMContext, Expr)); + DDI->replaceVariableLocationOp(OriginalStorage, Storage); + DDI->setExpression(Expr); if (auto *InsertPt = dyn_cast_or_null(Storage)) DDI->moveAfter(InsertPt); } diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp index 54811628de9..62f76199525 100644 --- a/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -3582,10 +3582,6 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) { if (!isa(DII)) return false; - if (isa(I)) - DII->setOperand( - 0, MetadataAsValue::get(I->getContext(), - ValueAsMetadata::get(I->getOperand(0)))); return true; }; diff --git a/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp index fedd9bfc977..e02076c7442 100644 --- a/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp +++ b/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp @@ -1153,9 +1153,10 @@ bool HWAddressSanitizer::instrumentStack( // to put it at the beginning of the expression. SmallVector NewOps = {dwarf::DW_OP_LLVM_tag_offset, RetagMask(N)}; - DDI->setArgOperand( - 2, MetadataAsValue::get(*C, DIExpression::prependOpcodes( - DDI->getExpression(), NewOps))); + auto Locations = DDI->location_ops(); + unsigned LocNo = std::distance(Locations.begin(), find(Locations, AI)); + DDI->setExpression( + DIExpression::appendOpsToArg(DDI->getExpression(), NewOps, LocNo)); } size_t Size = getAllocaSizeInBytes(*AI); @@ -1219,7 +1220,7 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) { if (auto *DDI = dyn_cast(&Inst)) if (auto *Alloca = - dyn_cast_or_null(DDI->getVariableLocation())) + dyn_cast_or_null(DDI->getVariableLocationOp(0))) AllocaDbgMap[Alloca].push_back(DDI); if (InstrumentLandingPads && isa(Inst)) @@ -1300,10 +1301,9 @@ bool HWAddressSanitizer::sanitizeFunction(Function &F) { for (auto &Inst : BB) if (auto *DVI = dyn_cast(&Inst)) if (auto *AI = - dyn_cast_or_null(DVI->getVariableLocation())) + dyn_cast_or_null(DVI->getVariableLocationOp(0))) if (auto *NewAI = AllocaToPaddedAllocaMap.lookup(AI)) - DVI->setArgOperand( - 0, MetadataAsValue::get(*C, LocalAsMetadata::get(NewAI))); + DVI->replaceVariableLocationOp(AI, NewAI); for (auto &P : AllocaToPaddedAllocaMap) P.first->eraseFromParent(); } diff --git a/lib/Transforms/Scalar/ADCE.cpp b/lib/Transforms/Scalar/ADCE.cpp index ce4e5e575fb..c8fb9d106e3 100644 --- a/lib/Transforms/Scalar/ADCE.cpp +++ b/lib/Transforms/Scalar/ADCE.cpp @@ -521,7 +521,7 @@ bool AggressiveDeadCodeElimination::removeDeadInstructions() { // If intrinsic is pointing at a live SSA value, there may be an // earlier optimization bug: if we know the location of the variable, // why isn't the scope of the location alive? - if (Value *V = DII->getVariableLocation()) + if (Value *V = DII->getVariableLocationOp(0)) if (Instruction *II = dyn_cast(V)) if (isLive(II)) dbgs() << "Dropping debug info for " << *DII << "\n"; diff --git a/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/lib/Transforms/Scalar/LoopStrengthReduce.cpp index 3e26ef205d1..facbfce86d8 100644 --- a/lib/Transforms/Scalar/LoopStrengthReduce.cpp +++ b/lib/Transforms/Scalar/LoopStrengthReduce.cpp @@ -5836,7 +5836,7 @@ static void DbgGatherEqualValues(Loop *L, ScalarEvolution &SE, auto DVI = dyn_cast(&I); if (!DVI) continue; - auto V = DVI->getVariableLocation(); + auto V = DVI->getVariableLocationOp(0); if (!V || !SE.isSCEVable(V->getType())) continue; auto DbgValueSCEV = SE.getSCEV(V); @@ -5862,7 +5862,7 @@ static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet) { for (auto A : DbgValueToEqualSet) { auto DVI = A.first; // Only update those that are now undef. - if (!isa_and_nonnull(DVI->getVariableLocation())) + if (!isa_and_nonnull(DVI->getVariableLocationOp(0))) continue; for (auto EV : A.second) { auto V = std::get(EV); @@ -5870,14 +5870,13 @@ static void DbgApplyEqualValues(EqualValuesMap &DbgValueToEqualSet) { continue; auto DbgDIExpr = std::get(EV); auto Offset = std::get(EV); - auto &Ctx = DVI->getContext(); - DVI->setOperand(0, MetadataAsValue::get(Ctx, ValueAsMetadata::get(V))); + DVI->replaceVariableLocationOp(DVI->getVariableLocationOp(0), V); if (Offset) { SmallVector Ops; DIExpression::appendOffset(Ops, Offset); DbgDIExpr = DIExpression::prependOpcodes(DbgDIExpr, Ops, true); } - DVI->setOperand(2, MetadataAsValue::get(Ctx, DbgDIExpr)); + DVI->setExpression(DbgDIExpr); break; } } diff --git a/lib/Transforms/Scalar/SpeculativeExecution.cpp b/lib/Transforms/Scalar/SpeculativeExecution.cpp index ff913220623..0cb9771b6e8 100644 --- a/lib/Transforms/Scalar/SpeculativeExecution.cpp +++ b/lib/Transforms/Scalar/SpeculativeExecution.cpp @@ -267,7 +267,7 @@ bool SpeculativeExecutionPass::considerHoistingFromTo( // 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->getVariableLocation())) + dyn_cast_or_null(DVI->getVariableLocationOp(0))) if (NotHoisted.count(I) == 0) return true; return false; diff --git a/lib/Transforms/Utils/CodeExtractor.cpp b/lib/Transforms/Utils/CodeExtractor.cpp index b9f886d6217..2b0ee77f4c9 100644 --- a/lib/Transforms/Utils/CodeExtractor.cpp +++ b/lib/Transforms/Utils/CodeExtractor.cpp @@ -1514,7 +1514,7 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc, // If the location isn't a constant or an instruction, delete the // intrinsic. auto *DVI = cast(DII); - Value *Location = DVI->getVariableLocation(); + Value *Location = DVI->getVariableLocationOp(0); if (!Location || (!isa(Location) && !isa(Location))) { DebugIntrinsicsToDelete.push_back(DVI); @@ -1537,7 +1537,7 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc, NewSP, OldVar->getName(), OldVar->getFile(), OldVar->getLine(), OldVar->getType(), /*AlwaysPreserve=*/false, DINode::FlagZero, OldVar->getAlignInBits()); - DVI->setArgOperand(1, MetadataAsValue::get(Ctx, NewVar)); + DVI->setVariable(cast(NewVar)); } for (auto *DII : DebugIntrinsicsToDelete) DII->eraseFromParent(); diff --git a/lib/Transforms/Utils/LCSSA.cpp b/lib/Transforms/Utils/LCSSA.cpp index 7437701f533..c5328784d81 100644 --- a/lib/Transforms/Utils/LCSSA.cpp +++ b/lib/Transforms/Utils/LCSSA.cpp @@ -236,7 +236,6 @@ bool llvm::formLCSSAForInstructions(SmallVectorImpl &Worklist, llvm::findDbgValues(DbgValues, I); // Update pre-existing debug value uses that reside outside the loop. - auto &Ctx = I->getContext(); for (auto DVI : DbgValues) { BasicBlock *UserBB = DVI->getParent(); if (InstBB == UserBB || L->contains(UserBB)) @@ -247,7 +246,7 @@ bool llvm::formLCSSAForInstructions(SmallVectorImpl &Worklist, Value *V = AddedPHIs.size() == 1 ? AddedPHIs[0] : SSAUpdate.FindValueForBlock(UserBB); if (V) - DVI->setOperand(0, MetadataAsValue::get(Ctx, ValueAsMetadata::get(V))); + DVI->replaceVariableLocationOp(I, V); } // SSAUpdater might have inserted phi-nodes inside other loops. We'll need diff --git a/lib/Transforms/Utils/Local.cpp b/lib/Transforms/Utils/Local.cpp index c6775d4f3f0..93e5b941c20 100644 --- a/lib/Transforms/Utils/Local.cpp +++ b/lib/Transforms/Utils/Local.cpp @@ -566,8 +566,7 @@ bool llvm::replaceDbgUsesWithUndef(Instruction *I) { findDbgUsers(DbgUsers, I); for (auto *DII : DbgUsers) { Value *Undef = UndefValue::get(I->getType()); - DII->setOperand(0, MetadataAsValue::get(DII->getContext(), - ValueAsMetadata::get(Undef))); + DII->replaceVariableLocationOp(I, Undef); } return !DbgUsers.empty(); } @@ -1389,7 +1388,7 @@ static bool valueCoversEntireFragment(Type *ValTy, DbgVariableIntrinsic *DII) { // 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->getVariableLocation())) + 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."); @@ -1597,7 +1596,8 @@ void llvm::insertDebugValuesForPHIs(BasicBlock *BB, ValueToValueMapTy DbgValueMap; for (auto &I : *BB) { if (auto DbgII = dyn_cast(&I)) { - if (auto *Loc = dyn_cast_or_null(DbgII->getVariableLocation())) + if (auto *Loc = + dyn_cast_or_null(DbgII->getVariableLocationOp(0))) DbgValueMap.insert({Loc, DbgII}); } } @@ -1607,19 +1607,18 @@ void llvm::insertDebugValuesForPHIs(BasicBlock *BB, // 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. - LLVMContext &C = BB->getContext(); for (auto PHI : InsertedPHIs) { BasicBlock *Parent = PHI->getParent(); // Avoid inserting an intrinsic into an EH block. if (Parent->getFirstNonPHI()->isEHPad()) continue; - auto PhiMAV = MetadataAsValue::get(C, ValueAsMetadata::get(PHI)); for (auto VI : PHI->operand_values()) { auto V = DbgValueMap.find(VI); if (V != DbgValueMap.end()) { auto *DbgII = cast(V->second); - Instruction *NewDbgII = DbgII->clone(); - NewDbgII->setOperand(0, PhiMAV); + DbgVariableIntrinsic *NewDbgII = + cast(DbgII->clone()); + NewDbgII->replaceVariableLocationOp(VI, PHI); auto InsertionPt = Parent->getFirstInsertionPt(); assert(InsertionPt != Parent->end() && "Ill-formed basic block"); NewDbgII->insertBefore(&*InsertionPt); @@ -1736,11 +1735,6 @@ void llvm::replaceDbgValueForAlloca(AllocaInst *AI, Value *NewAllocaAddress, replaceOneDbgValueForAlloca(DVI, NewAllocaAddress, Builder, Offset); } -/// Wrap \p V in a ValueAsMetadata instance. -static MetadataAsValue *wrapValueInMetadata(LLVMContext &C, Value *V) { - return MetadataAsValue::get(C, ValueAsMetadata::get(V)); -} - /// Where possible to salvage debug information for \p I do so /// and return True. If not possible mark undef and return False. void llvm::salvageDebugInfo(Instruction &I) { @@ -1751,9 +1745,7 @@ void llvm::salvageDebugInfo(Instruction &I) { void llvm::salvageDebugInfoForDbgValues( Instruction &I, ArrayRef DbgUsers) { - auto &Ctx = I.getContext(); bool Salvaged = false; - auto wrapMD = [&](Value *V) { return wrapValueInMetadata(Ctx, V); }; for (auto *DII : DbgUsers) { // Do not add DW_OP_stack_value for DbgDeclare and DbgAddr, because they @@ -1769,8 +1761,8 @@ void llvm::salvageDebugInfoForDbgValues( if (!DIExpr) break; - DII->setOperand(0, wrapMD(I.getOperand(0))); - DII->setOperand(2, MetadataAsValue::get(Ctx, DIExpr)); + DII->replaceVariableLocationOp(&I, I.getOperand(0)); + DII->setExpression(DIExpr); LLVM_DEBUG(dbgs() << "SALVAGE: " << *DII << '\n'); Salvaged = true; } @@ -1780,8 +1772,7 @@ void llvm::salvageDebugInfoForDbgValues( for (auto *DII : DbgUsers) { Value *Undef = UndefValue::get(I.getType()); - DII->setOperand(0, MetadataAsValue::get(DII->getContext(), - ValueAsMetadata::get(Undef))); + DII->replaceVariableLocationOp(&I, Undef); } } @@ -1925,13 +1916,12 @@ static bool rewriteDebugUsers( if (UndefOrSalvage.count(DII)) continue; - LLVMContext &Ctx = DII->getContext(); DbgValReplacement DVR = RewriteExpr(*DII); if (!DVR) continue; - DII->setOperand(0, wrapValueInMetadata(Ctx, &To)); - DII->setOperand(2, MetadataAsValue::get(Ctx, *DVR)); + DII->replaceVariableLocationOp(&From, &To); + DII->setExpression(*DVR); LLVM_DEBUG(dbgs() << "REWRITE: " << *DII << '\n'); Changed = true; } diff --git a/lib/Transforms/Utils/LoopRotationUtils.cpp b/lib/Transforms/Utils/LoopRotationUtils.cpp index b678efdc8d8..784d0e437ba 100644 --- a/lib/Transforms/Utils/LoopRotationUtils.cpp +++ b/lib/Transforms/Utils/LoopRotationUtils.cpp @@ -179,9 +179,7 @@ static void RewriteUsesOfClonedInstructions(BasicBlock *OrigHeader, NewVal = SSA.GetValueInMiddleOfBlock(UserBB); else NewVal = UndefValue::get(OrigHeaderVal->getType()); - DbgValue->setOperand(0, - MetadataAsValue::get(OrigHeaderVal->getContext(), - ValueAsMetadata::get(NewVal))); + DbgValue->replaceVariableLocationOp(OrigHeaderVal, NewVal); } } } @@ -390,7 +388,8 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) { using DbgIntrinsicHash = std::pair, DIExpression *>; auto makeHash = [](DbgVariableIntrinsic *D) -> DbgIntrinsicHash { - return {{D->getVariableLocation(), D->getVariable()}, D->getExpression()}; + return {{D->getVariableLocationOp(0), D->getVariable()}, + D->getExpression()}; }; SmallDenseSet DbgIntrinsics; for (auto I = std::next(OrigPreheader->rbegin()), E = OrigPreheader->rend(); diff --git a/unittests/Transforms/Utils/LocalTest.cpp b/unittests/Transforms/Utils/LocalTest.cpp index a57a18ce0be..f2fce5e7d55 100644 --- a/unittests/Transforms/Utils/LocalTest.cpp +++ b/unittests/Transforms/Utils/LocalTest.cpp @@ -524,7 +524,9 @@ struct SalvageDebugInfoTest : ::testing::Test { } 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 @@ struct SalvageDebugInfoTest : ::testing::Test { } bool doesDebugValueDescribeY(const DbgValueInst &DI) { - const auto &CI = *cast(DI.getValue()); + if (DI.getNumVariableLocationOps() != 1) + return false; + const auto &CI = *cast(DI.getVariableLocationOp(0)); if (CI.isZero()) return DI.getExpression()->getElements().equals( {dwarf::DW_OP_plus_uconst, 1, dwarf::DW_OP_plus_uconst, 2, @@ -758,13 +762,15 @@ TEST(Local, ReplaceAllDbgUsesWith) { EXPECT_TRUE(replaceAllDbgUsesWith(A, F_, F_, DT)); auto *ADbgVal = cast(A.getNextNode()); - EXPECT_EQ(ConstantInt::get(A.getType(), 0), ADbgVal->getVariableLocation()); + EXPECT_EQ(ADbgVal->getNumVariableLocationOps(), 1); + EXPECT_EQ(ConstantInt::get(A.getType(), 0), ADbgVal->getVariableLocationOp(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->getVariableLocation())); + EXPECT_EQ(FDbgVal->getNumVariableLocationOps(), 1); + EXPECT_TRUE(FDbgVal->isUndef()); SmallVector FDbgVals; findDbgValues(FDbgVals, &F_);