Index: docs/LangRef.rst =================================================================== --- docs/LangRef.rst +++ docs/LangRef.rst @@ -4380,7 +4380,7 @@ The current supported vocabulary is limited: -- ``DW_OP_deref`` dereferences the working expression. +- ``DW_OP_deref`` dereferences the top of the expression stack. - ``DW_OP_plus, 93`` adds ``93`` to the working expression. - ``DW_OP_LLVM_fragment, 16, 8`` specifies the offset and size (``16`` and ``8`` here, respectively) of the variable fragment from the working expression. Note @@ -4396,12 +4396,17 @@ location descriptions that describe constant values. This form is used to describe global constants that have been optimized away. All other expressions are modifiers to another location: A debug intrinsic ties a location and a -DIExpression together. Contrary to DWARF expressions, a DIExpression always -describes the *value* of a source variable and never its *address*. In DWARF -terminology, a DIExpression can always be considered an implicit location -description regardless whether it contains a ``DW_OP_stack_value`` or not. +DIExpression together. -.. code-block:: text +DWARF specifies three kinds of simple location descriptions: Register, memory, +and implicit location descriptions. Register and memory location descriptions +describe the *location* of a source variable (in the sense that a debugger might +modify its value), whereas implicit locations describe merely the *value* of a +source variable. DIExpressions also follow this model: A DIExpression that +doesn't have a trailing ``DW_OP_stack_value`` will describe an *address* when +combined with a concrete location. + +.. code-block:: llvm !0 = !DIExpression(DW_OP_deref) !1 = !DIExpression(DW_OP_plus, 3) Index: docs/SourceLevelDebugging.rst =================================================================== --- docs/SourceLevelDebugging.rst +++ docs/SourceLevelDebugging.rst @@ -180,11 +180,27 @@ void @llvm.dbg.declare(metadata, metadata, metadata) -This intrinsic provides information about a local element (e.g., variable). -The first argument is metadata holding the alloca for the variable. The second +This intrinsic provides information about a local element (e.g., variable). The +first argument is metadata holding the alloca for the variable. The second argument is a `local variable `_ containing a description of the variable. The third argument is a `complex expression -`_. +`_. An `llvm.dbg.declare` instrinsic describes the +*location* of a source variable. + +.. code-block:: llvm + + %i.addr = alloca i32, align 4 + call void @llvm.dbg.declare(metadata i32* %i.addr, metadata !1, metadata !2), !dbg !3 + !1 = !DILocalVariable(name: "i", ...) ; int i + !2 = !DIExpression() + !3 = !DILocation(...) + ... + %buffer = alloca [256 x i8], align 8 + ; The address of i is buffer+64. + call void @llvm.dbg.declare(metadata [256 x i8]* %buffer, metadata !1, metadata !2) + !1 = !DILocalVariable(name: "i", ...) ; int i + !2 = !DIExpression(DW_OP_plus, 64) + ``llvm.dbg.value`` ^^^^^^^^^^^^^^^^^^ Index: include/llvm/CodeGen/MachineInstrBuilder.h =================================================================== --- include/llvm/CodeGen/MachineInstrBuilder.h +++ include/llvm/CodeGen/MachineInstrBuilder.h @@ -413,6 +413,11 @@ unsigned Reg, unsigned Offset, const MDNode *Variable, const MDNode *Expr); +/// Clone a DBG_VALUE whose value has been spilled to FrameIndex. +MachineInstr *buildDbgValueForSpill(MachineBasicBlock &BB, + MachineBasicBlock::iterator I, + const MachineInstr &Orig, int FrameIndex); + inline unsigned getDefRegState(bool B) { return B ? RegState::Define : 0; } Index: include/llvm/IR/DebugInfoMetadata.h =================================================================== --- include/llvm/IR/DebugInfoMetadata.h +++ include/llvm/IR/DebugInfoMetadata.h @@ -2232,6 +2232,9 @@ expr_op_iterator expr_op_end() const { return expr_op_iterator(elements_end()); } + iterator_range expr_ops() const { + return {expr_op_begin(), expr_op_end()}; + } /// @} bool isValid() const; @@ -2240,7 +2243,7 @@ return MD->getMetadataID() == DIExpressionKind; } - /// Is the first element a DW_OP_deref?. + /// Return whether the first element a DW_OP_deref. bool startsWithDeref() const { return getNumElements() > 0 && getElement(0) == dwarf::DW_OP_deref; } Index: lib/Bitcode/Reader/BitcodeReader.cpp =================================================================== --- lib/Bitcode/Reader/BitcodeReader.cpp +++ lib/Bitcode/Reader/BitcodeReader.cpp @@ -2535,6 +2535,7 @@ // Look for intrinsic functions which need to be upgraded at some point for (Function &F : *TheModule) { + MDLoader->upgradeDebugIntrinsics(F); Function *NewFn; if (UpgradeIntrinsicFunction(&F, NewFn)) UpgradedIntrinsics[&F] = NewFn; Index: lib/Bitcode/Reader/MetadataLoader.h =================================================================== --- lib/Bitcode/Reader/MetadataLoader.h +++ lib/Bitcode/Reader/MetadataLoader.h @@ -79,6 +79,9 @@ unsigned size() const; void shrinkTo(unsigned N); + + /// Perform bitcode upgrades on llvm.dbg.* calls. + void upgradeDebugIntrinsics(Function &F); }; } Index: lib/Bitcode/Reader/MetadataLoader.cpp =================================================================== --- lib/Bitcode/Reader/MetadataLoader.cpp +++ lib/Bitcode/Reader/MetadataLoader.cpp @@ -54,6 +54,7 @@ #include "llvm/IR/Instruction.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/Intrinsics.h" +#include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" #include "llvm/IR/ModuleSummaryIndex.h" @@ -452,6 +453,7 @@ bool StripTBAA = false; bool HasSeenOldLoopTags = false; bool NeedUpgradeToDIGlobalVariableExpression = false; + bool NeedDeclareExpressionUpgrade = false; /// True if metadata is being parsed for a module being ThinLTO imported. bool IsImporting = false; @@ -511,6 +513,26 @@ } } + /// Remove a leading DW_OP_deref from DIExpressions in a dbg.declare that + /// describes a function argument. + void upgradeDeclareExpressions(Function &F) { + if (!NeedDeclareExpressionUpgrade) + return; + + for (auto &BB : F) + for (auto &I : BB) + if (auto *DDI = dyn_cast(&I)) + if (auto *DIExpr = DDI->getExpression()) + if (DIExpr->startsWithDeref() && + dyn_cast_or_null(DDI->getAddress())) { + 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)); + } + } + void upgradeDebugInfo() { upgradeCUSubprograms(); upgradeCUVariables(); @@ -565,6 +587,7 @@ unsigned size() const { return MetadataList.size(); } void shrinkTo(unsigned N) { MetadataList.shrinkTo(N); } + void upgradeDebugIntrinsics(Function &F) { upgradeDeclareExpressions(F); } }; static Error error(const Twine &Message) { @@ -1520,12 +1543,32 @@ return error("Invalid record"); IsDistinct = Record[0] & 1; - bool HasOpFragment = Record[0] & 2; + uint64_t Version = Record[0] >> 1; auto Elts = MutableArrayRef(Record).slice(1); - if (!HasOpFragment) - if (unsigned N = Elts.size()) - if (N >= 3 && Elts[N - 3] == dwarf::DW_OP_bit_piece) - Elts[N - 3] = dwarf::DW_OP_LLVM_fragment; + unsigned N = Elts.size(); + // Perform various upgrades. + switch (Version) { + case 0: + if (N >= 3 && Elts[N - 3] == dwarf::DW_OP_bit_piece) + Elts[N - 3] = dwarf::DW_OP_LLVM_fragment; + LLVM_FALLTHROUGH; + case 1: + // Move DW_OP_deref to the end. + if (N && Elts[0] == dwarf::DW_OP_deref) { + auto End = Elts.end(); + if (Elts.size() >= 3 && *std::prev(End, 3) == dwarf::DW_OP_LLVM_fragment) + End = std::prev(End, 3); + std::move(std::next(Elts.begin()), End, Elts.begin()); + *std::prev(End) = dwarf::DW_OP_deref; + } + NeedDeclareExpressionUpgrade = true; + LLVM_FALLTHROUGH; + case 2: + // Up-to-date! + break; + default: + return error("Invalid record"); + } MetadataList.assignValue( GET_OR_DISTINCT(DIExpression, (Context, makeArrayRef(Record).slice(1))), @@ -1858,3 +1901,7 @@ unsigned MetadataLoader::size() const { return Pimpl->size(); } void MetadataLoader::shrinkTo(unsigned N) { return Pimpl->shrinkTo(N); } + +void MetadataLoader::upgradeDebugIntrinsics(Function &F) { + return Pimpl->upgradeDebugIntrinsics(F); +} Index: lib/Bitcode/Writer/BitcodeWriter.cpp =================================================================== --- lib/Bitcode/Writer/BitcodeWriter.cpp +++ lib/Bitcode/Writer/BitcodeWriter.cpp @@ -1757,9 +1757,8 @@ SmallVectorImpl &Record, unsigned Abbrev) { Record.reserve(N->getElements().size() + 1); - - const uint64_t HasOpFragmentFlag = 1 << 1; - Record.push_back((uint64_t)N->isDistinct() | HasOpFragmentFlag); + const uint64_t Version = 2 << 1; + Record.push_back((uint64_t)N->isDistinct() | Version); Record.append(N->elements_begin(), N->elements_end()); Stream.EmitRecord(bitc::METADATA_EXPRESSION, Record, Abbrev); Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp =================================================================== --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -813,9 +813,9 @@ OS << " <- "; // The second operand is only an offset if it's an immediate. - bool Deref = MI->getOperand(0).isReg() && MI->getOperand(1).isImm(); - int64_t Offset = Deref ? MI->getOperand(1).getImm() : 0; - + bool Deref = false; + bool MemLoc = MI->getOperand(0).isReg() && MI->getOperand(1).isImm(); + int64_t Offset = MemLoc ? MI->getOperand(1).getImm() : 0; for (unsigned i = 0; i < Expr->getNumElements(); ++i) { uint64_t Op = Expr->getElement(i); if (Op == dwarf::DW_OP_LLVM_fragment) { @@ -823,7 +823,7 @@ break; } else if (Deref) { // We currently don't support extra Offsets or derefs after the first - // one. Bail out early instead of emitting an incorrect comment + // one. Bail out early instead of emitting an incorrect comment. OS << " [complex expression]"; AP.OutStreamer->emitRawComment(OS.str()); return true; @@ -878,12 +878,12 @@ AP.OutStreamer->emitRawComment(OS.str()); return true; } - if (Deref) + if (MemLoc || Deref) OS << '['; OS << PrintReg(Reg, AP.MF->getSubtarget().getRegisterInfo()); } - if (Deref) + if (MemLoc || Deref) OS << '+' << Offset << ']'; // NOTE: Want this comment at start of line, don't emit with AddComment. Index: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp =================================================================== --- lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp +++ lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp @@ -547,18 +547,19 @@ DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc); for (auto &Fragment : DV.getFrameIndexExprs()) { unsigned FrameReg = 0; + const DIExpression *Expr = Fragment.Expr; const TargetFrameLowering *TFI = Asm->MF->getSubtarget().getFrameLowering(); int Offset = TFI->getFrameIndexReference(*Asm->MF, Fragment.FI, FrameReg); - DwarfExpr.addFragmentOffset(Fragment.Expr); + DwarfExpr.addFragmentOffset(Expr); SmallVector Ops; Ops.push_back(dwarf::DW_OP_plus); Ops.push_back(Offset); - Ops.push_back(dwarf::DW_OP_deref); - Ops.append(Fragment.Expr->elements_begin(), Fragment.Expr->elements_end()); - DIExpressionCursor Expr(Ops); - DwarfExpr.addMachineRegExpression( - *Asm->MF->getSubtarget().getRegisterInfo(), Expr, FrameReg); - DwarfExpr.addExpression(std::move(Expr)); + Ops.append(Expr->elements_begin(), Expr->elements_end()); + DIExpressionCursor Cursor(Ops); + DwarfExpr.addMachineLocExpression( + *Asm->MF->getSubtarget().getRegisterInfo(), Cursor, + MachineLocation(FrameReg)); + DwarfExpr.addExpression(std::move(Cursor)); } addBlock(*VariableDie, dwarf::DW_AT_location, DwarfExpr.finalize()); @@ -781,14 +782,13 @@ DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc); SmallVector Ops; - if (Location.isIndirect()) { + if (Location.isIndirect() && Location.getOffset()) { Ops.push_back(dwarf::DW_OP_plus); Ops.push_back(Location.getOffset()); - Ops.push_back(dwarf::DW_OP_deref); } DIExpressionCursor Cursor(Ops); const TargetRegisterInfo &TRI = *Asm->MF->getSubtarget().getRegisterInfo(); - if (!DwarfExpr.addMachineRegExpression(TRI, Cursor, Location.getReg())) + if (!DwarfExpr.addMachineLocExpression(TRI, Cursor, Location)) return; DwarfExpr.addExpression(std::move(Cursor)); @@ -809,15 +809,14 @@ DwarfExpr.addFragmentOffset(DIExpr); SmallVector Ops; - if (Location.isIndirect()) { + if (Location.isIndirect() && Location.getOffset()) { Ops.push_back(dwarf::DW_OP_plus); Ops.push_back(Location.getOffset()); - Ops.push_back(dwarf::DW_OP_deref); } Ops.append(DIExpr->elements_begin(), DIExpr->elements_end()); DIExpressionCursor Cursor(Ops); const TargetRegisterInfo &TRI = *Asm->MF->getSubtarget().getRegisterInfo(); - if (!DwarfExpr.addMachineRegExpression(TRI, Cursor, Location.getReg())) + if (!DwarfExpr.addMachineLocExpression(TRI, Cursor, Location)) return; DwarfExpr.addExpression(std::move(Cursor)); Index: lib/CodeGen/AsmPrinter/DwarfDebug.cpp =================================================================== --- lib/CodeGen/AsmPrinter/DwarfDebug.cpp +++ lib/CodeGen/AsmPrinter/DwarfDebug.cpp @@ -1517,18 +1517,15 @@ DwarfExpr.addUnsignedConstant(Value.getInt()); } else if (Value.isLocation()) { MachineLocation Location = Value.getLoc(); - SmallVector Ops; - // FIXME: Should this condition be Location.isIndirect() instead? - if (Location.getOffset()) { + if (Location.isIndirect() && Location.getOffset()) { Ops.push_back(dwarf::DW_OP_plus); Ops.push_back(Location.getOffset()); - Ops.push_back(dwarf::DW_OP_deref); } Ops.append(DIExpr->elements_begin(), DIExpr->elements_end()); DIExpressionCursor Cursor(Ops); const TargetRegisterInfo &TRI = *AP.MF->getSubtarget().getRegisterInfo(); - if (!DwarfExpr.addMachineRegExpression(TRI, Cursor, Location.getReg())) + if (!DwarfExpr.addMachineLocExpression(TRI, Cursor, Location)) return; return DwarfExpr.addExpression(std::move(Cursor)); } else if (Value.isConstantFP()) { Index: lib/CodeGen/AsmPrinter/DwarfExpression.h =================================================================== --- lib/CodeGen/AsmPrinter/DwarfExpression.h +++ lib/CodeGen/AsmPrinter/DwarfExpression.h @@ -16,6 +16,7 @@ #include "llvm/IR/DebugInfo.h" #include "llvm/Support/DataTypes.h" +#include "llvm/MC/MachineLocation.h" namespace llvm { @@ -102,6 +103,9 @@ unsigned SubRegisterSizeInBits = 0; unsigned SubRegisterOffsetInBits = 0; + /// The kind of location description being produced. + enum { Unknown = 0, Register, Memory, Implicit } LocationKind = Unknown; + /// Push a DW_OP_piece / DW_OP_bit_piece for emitting later, if one is needed /// to represent a subregister. void setSubRegisterPiece(unsigned SizeInBits, unsigned OffsetInBits) { @@ -109,6 +113,8 @@ SubRegisterOffsetInBits = OffsetInBits; } + void setMemoryLocationKind(); + /// Add masking operations to stencil out a subregister. void maskSubRegister(); @@ -122,7 +128,8 @@ /// current function. virtual bool isFrameRegister(const TargetRegisterInfo &TRI, unsigned MachineReg) = 0; - /// Emit a DW_OP_reg operation. + /// Emit a DW_OP_reg operation. Note that this is only legal inside a DWARF + /// register location description. void addReg(int DwarfReg, const char *Comment = nullptr); /// Emit a DW_OP_breg operation. void addBReg(int DwarfReg, int Offset); @@ -194,8 +201,8 @@ /// fragment inside the entire variable. /// \return false if no DWARF register exists /// for MachineReg. - bool addMachineRegExpression(const TargetRegisterInfo &TRI, - DIExpressionCursor &Expr, unsigned MachineReg, + bool addMachineLocExpression(const TargetRegisterInfo &TRI, + DIExpressionCursor &Expr, MachineLocation Loc, unsigned FragmentOffsetInBits = 0); /// Emit all remaining operations in the DIExpressionCursor. /// Index: lib/CodeGen/AsmPrinter/DwarfExpression.cpp =================================================================== --- lib/CodeGen/AsmPrinter/DwarfExpression.cpp +++ lib/CodeGen/AsmPrinter/DwarfExpression.cpp @@ -23,9 +23,12 @@ using namespace llvm; void DwarfExpression::addReg(int DwarfReg, const char *Comment) { - assert(DwarfReg >= 0 && "invalid negative dwarf register number"); - if (DwarfReg < 32) { - emitOp(dwarf::DW_OP_reg0 + DwarfReg, Comment); + assert(DwarfReg >= 0 && "invalid negative dwarf register number"); + assert((LocationKind == Unknown || LocationKind == Register) && + "location description already locked down"); + LocationKind = Register; + if (DwarfReg < 32) { + emitOp(dwarf::DW_OP_reg0 + DwarfReg, Comment); } else { emitOp(dwarf::DW_OP_regx, Comment); emitUnsigned(DwarfReg); @@ -34,6 +37,7 @@ void DwarfExpression::addBReg(int DwarfReg, int Offset) { assert(DwarfReg >= 0 && "invalid negative dwarf register number"); + assert(LocationKind != Register && "location description already locked down"); if (DwarfReg < 32) { emitOp(dwarf::DW_OP_breg0 + DwarfReg); } else { @@ -156,18 +160,23 @@ } void DwarfExpression::addSignedConstant(int64_t Value) { + assert(LocationKind == Implicit || LocationKind == Unknown); + LocationKind = Implicit; emitOp(dwarf::DW_OP_consts); emitSigned(Value); - addStackValue(); } void DwarfExpression::addUnsignedConstant(uint64_t Value) { + assert(LocationKind == Implicit || LocationKind == Unknown); + LocationKind = Implicit; emitOp(dwarf::DW_OP_constu); emitUnsigned(Value); - addStackValue(); } void DwarfExpression::addUnsignedConstant(const APInt &Value) { + assert(LocationKind == Implicit || LocationKind == Unknown); + LocationKind = Implicit; + unsigned Size = Value.getBitWidth(); const uint64_t *Data = Value.getRawData(); @@ -178,15 +187,20 @@ addUnsignedConstant(*Data++); if (Offset == 0 && Size <= 64) break; - addOpPiece(std::min(Size-Offset, 64u), Offset); + addStackValue(); + addOpPiece(std::min(Size - Offset, 64u), Offset); Offset += 64; } } -bool DwarfExpression::addMachineRegExpression(const TargetRegisterInfo &TRI, +bool DwarfExpression::addMachineLocExpression(const TargetRegisterInfo &TRI, DIExpressionCursor &ExprCursor, - unsigned MachineReg, + MachineLocation Loc, unsigned FragmentOffsetInBits) { + if (Loc.isIndirect()) + LocationKind = Memory; + + unsigned MachineReg = Loc.getReg(); auto Fragment = ExprCursor.getFragmentInfo(); if (!addMachineReg(TRI, MachineReg, Fragment ? Fragment->SizeInBits : ~1U)) return false; @@ -206,7 +220,7 @@ } // Handle simple register locations. - if (!HasComplexExpression) { + if (LocationKind != Memory && !HasComplexExpression) { for (auto &Reg : DwarfRegs) { if (Reg.DwarfRegNo >= 0) addReg(Reg.DwarfRegNo, Reg.Comment); @@ -216,62 +230,58 @@ return true; } + // FIXME: + // Don't emit locations that cannot be expressed without DW_OP_stack_value. + assert(DwarfRegs.size() == 1); auto Reg = DwarfRegs[0]; - bool FBReg = isFrameRegister(TRI, MachineReg); + bool FBReg = isFrameRegister(TRI, MachineReg); + int SignedOffset = 0; assert(Reg.Size == 0 && "subregister has same size as superregister"); // Pattern-match combinations for which more efficient representations exist. - switch (Op->getOp()) { - default: { - if (FBReg) - addFBReg(0); - else - addReg(Reg.DwarfRegNo, 0); - break; + // [Reg, Offset, DW_OP_plus] --> [DW_OP_breg, Offset]. + // [Reg, Offset, DW_OP_minus] --> [DW_OP_breg, -Offset]. + // If Reg is a subregister we need to mask it out before subtracting. + if (Op && ((Op->getOp() == dwarf::DW_OP_plus) || + (Op->getOp() == dwarf::DW_OP_minus && !SubRegisterSizeInBits))) { + int Offset = Op->getArg(0); + SignedOffset = (Op->getOp() == dwarf::DW_OP_plus) ? Offset : -Offset; + ExprCursor.take(); } - case dwarf::DW_OP_plus: - case dwarf::DW_OP_minus: { - // [DW_OP_reg,Offset,DW_OP_plus, DW_OP_deref] --> [DW_OP_breg, Offset]. - // [DW_OP_reg,Offset,DW_OP_minus,DW_OP_deref] --> [DW_OP_breg,-Offset]. - auto N = ExprCursor.peekNext(); - if (N && N->getOp() == dwarf::DW_OP_deref) { - int Offset = Op->getArg(0); - int SignedOffset = (Op->getOp() == dwarf::DW_OP_plus) ? Offset : -Offset; - if (FBReg) - addFBReg(SignedOffset); - else - addBReg(Reg.DwarfRegNo, SignedOffset); + if (FBReg) + addFBReg(SignedOffset); + else + addBReg(Reg.DwarfRegNo, SignedOffset); + DwarfRegs.clear(); + return true; +} - ExprCursor.consume(2); +/// Assuming a well-formed expression, match "DW_OP_deref* DW_OP_LLVM_fragment?". +static bool isMemoryLocation(DIExpressionCursor ExprCursor) { + while (ExprCursor) { + auto Op = ExprCursor.take(); + switch (Op->getOp()) { + case dwarf::DW_OP_deref: + case dwarf::DW_OP_LLVM_fragment: break; + default: + return false; } - addReg(Reg.DwarfRegNo, 0); - break; - } - case dwarf::DW_OP_deref: - // [DW_OP_reg,DW_OP_deref] --> [DW_OP_breg]. - if (FBReg) - addFBReg(0); - else - addBReg(Reg.DwarfRegNo, 0); - ExprCursor.take(); - break; } - DwarfRegs.clear(); return true; } void DwarfExpression::addExpression(DIExpressionCursor &&ExprCursor, unsigned FragmentOffsetInBits) { + // If we need to mask out a subregister, do it now, unless the next + // operation would emit an OpPiece anyway. + auto N = ExprCursor.peek(); + if (SubRegisterSizeInBits && N && (N->getOp() != dwarf::DW_OP_LLVM_fragment)) + maskSubRegister(); + while (ExprCursor) { auto Op = ExprCursor.take(); - - // If we need to mask out a subregister, do it now, unless the next - // operation would emit an OpPiece anyway. - if (SubRegisterSizeInBits && Op->getOp() != dwarf::DW_OP_LLVM_fragment) - maskSubRegister(); - switch (Op->getOp()) { case dwarf::DW_OP_LLVM_fragment: { unsigned SizeInBits = Op->getArg(1); @@ -281,50 +291,75 @@ // location. assert(OffsetInBits >= FragmentOffset && "fragment offset not added?"); - // If \a addMachineReg already emitted DW_OP_piece operations to represent + // If addMachineReg already emitted DW_OP_piece operations to represent // a super-register by splicing together sub-registers, subtract the size // of the pieces that was already emitted. SizeInBits -= OffsetInBits - FragmentOffset; - // If \a addMachineReg requested a DW_OP_bit_piece to stencil out a + // If addMachineReg requested a DW_OP_bit_piece to stencil out a // sub-register that is smaller than the current fragment's size, use it. if (SubRegisterSizeInBits) SizeInBits = std::min(SizeInBits, SubRegisterSizeInBits); + // Emit the DW_OP_piece. addOpPiece(SizeInBits, SubRegisterOffsetInBits); setSubRegisterPiece(0, 0); - break; + // Reset the location description kind. + LocationKind = Unknown; + return; } case dwarf::DW_OP_plus: + assert(LocationKind != Register); emitOp(dwarf::DW_OP_plus_uconst); emitUnsigned(Op->getArg(0)); break; case dwarf::DW_OP_minus: - // There is no OP_minus_uconst. + assert(LocationKind != Register); + // There is no DW_OP_minus_uconst. emitOp(dwarf::DW_OP_constu); emitUnsigned(Op->getArg(0)); emitOp(dwarf::DW_OP_minus); break; - case dwarf::DW_OP_deref: - emitOp(dwarf::DW_OP_deref); + case dwarf::DW_OP_deref: { + assert(LocationKind != Register); + if (LocationKind != Memory && isMemoryLocation(ExprCursor)) + // Turning this into a memory location description makes the deref + // implicit. + LocationKind = Memory; + else + emitOp(dwarf::DW_OP_deref); break; + } case dwarf::DW_OP_constu: + assert(LocationKind != Register); emitOp(dwarf::DW_OP_constu); emitUnsigned(Op->getArg(0)); break; case dwarf::DW_OP_stack_value: - addStackValue(); + assert(LocationKind == Unknown || LocationKind == Implicit); + LocationKind = Implicit; + if (LocationKind == Implicit) + // Turn this into an implicit location description. + addStackValue(); + break; case dwarf::DW_OP_swap: + assert(LocationKind != Register); emitOp(dwarf::DW_OP_swap); break; case dwarf::DW_OP_xderef: + assert(LocationKind != Register); emitOp(dwarf::DW_OP_xderef); break; default: llvm_unreachable("unhandled opcode found in expression"); } } + + if (LocationKind != Implicit) + return; + // Turn this into an implicit location description. + addStackValue(); } /// add masking operations to stencil out a subregister. Index: lib/CodeGen/AsmPrinter/DwarfUnit.cpp =================================================================== --- lib/CodeGen/AsmPrinter/DwarfUnit.cpp +++ lib/CodeGen/AsmPrinter/DwarfUnit.cpp @@ -473,10 +473,9 @@ DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc); SmallVector Ops; - if (Location.isIndirect()) { + if (Location.isIndirect() && Location.getOffset()) { Ops.push_back(dwarf::DW_OP_plus); Ops.push_back(Location.getOffset()); - Ops.push_back(dwarf::DW_OP_deref); } // If we started with a pointer to the __Block_byref... struct, then // the first thing we need to do is dereference the pointer (DW_OP_deref). @@ -505,7 +504,7 @@ DIExpressionCursor Cursor(Ops); const TargetRegisterInfo &TRI = *Asm->MF->getSubtarget().getRegisterInfo(); - if (!DwarfExpr.addMachineRegExpression(TRI, Cursor, Location.getReg())) + if (!DwarfExpr.addMachineLocExpression(TRI, Cursor, Location)) return; DwarfExpr.addExpression(std::move(Cursor)); Index: lib/CodeGen/InlineSpiller.cpp =================================================================== --- lib/CodeGen/InlineSpiller.cpp +++ lib/CodeGen/InlineSpiller.cpp @@ -888,20 +888,10 @@ // Debug values are not allowed to affect codegen. if (MI->isDebugValue()) { // Modify DBG_VALUE now that the value is in a spill slot. - bool IsIndirect = MI->isIndirectDebugValue(); - uint64_t Offset = IsIndirect ? MI->getOperand(1).getImm() : 0; - const MDNode *Var = MI->getDebugVariable(); - const MDNode *Expr = MI->getDebugExpression(); - DebugLoc DL = MI->getDebugLoc(); - DEBUG(dbgs() << "Modifying debug info due to spill:" << "\t" << *MI); MachineBasicBlock *MBB = MI->getParent(); - assert(cast(Var)->isValidLocationForIntrinsic(DL) && - "Expected inlined-at fields to agree"); - BuildMI(*MBB, MBB->erase(MI), DL, TII.get(TargetOpcode::DBG_VALUE)) - .addFrameIndex(StackSlot) - .addImm(Offset) - .addMetadata(Var) - .addMetadata(Expr); + DEBUG(dbgs() << "Modifying debug info due to spill:\t" << *MI); + buildDbgValueForSpill(*MBB, MI, *MI, StackSlot); + MBB->erase(MI); continue; } Index: lib/CodeGen/MachineInstr.cpp =================================================================== --- lib/CodeGen/MachineInstr.cpp +++ lib/CodeGen/MachineInstr.cpp @@ -2351,3 +2351,31 @@ BB.insert(I, MI); return MachineInstrBuilder(MF, MI); } + +MachineInstr *llvm::buildDbgValueForSpill(MachineBasicBlock &BB, + MachineBasicBlock::iterator I, + const MachineInstr &Orig, + int FrameIndex) { + const MDNode *Var = Orig.getDebugVariable(); + auto *Expr = cast_or_null(Orig.getDebugExpression()); + bool IsIndirect = Orig.isIndirectDebugValue(); + uint64_t Offset = IsIndirect ? Orig.getOperand(1).getImm() : 0; + DebugLoc DL = Orig.getDebugLoc(); + assert(cast(Var)->isValidLocationForIntrinsic(DL) && + "Expected inlined-at fields to agree"); + // If the DBG_VALUE already was a memory location, add an extra + // DW_OP_deref. Otherwise just turning this from a register into a + // memory/indirect location is sufficient. + if (IsIndirect) { + SmallVector Ops; + Ops.push_back(dwarf::DW_OP_deref); + if (Expr) + Ops.append(Expr->elements_begin(), Expr->elements_end()); + Expr = DIExpression::get(Expr->getContext(), Ops); + } + return BuildMI(BB, I, DL, Orig.getDesc()) + .addFrameIndex(FrameIndex) + .addImm(Offset) + .addMetadata(Var) + .addMetadata(Expr); +} Index: lib/CodeGen/RegAllocFast.cpp =================================================================== --- lib/CodeGen/RegAllocFast.cpp +++ lib/CodeGen/RegAllocFast.cpp @@ -304,19 +304,7 @@ LiveDbgValueMap[LRI->VirtReg]; for (unsigned li = 0, le = LRIDbgValues.size(); li != le; ++li) { MachineInstr *DBG = LRIDbgValues[li]; - const MDNode *Var = DBG->getDebugVariable(); - const MDNode *Expr = DBG->getDebugExpression(); - bool IsIndirect = DBG->isIndirectDebugValue(); - uint64_t Offset = IsIndirect ? DBG->getOperand(1).getImm() : 0; - DebugLoc DL = DBG->getDebugLoc(); - assert(cast(Var)->isValidLocationForIntrinsic(DL) && - "Expected inlined-at fields to agree"); - MachineInstr *NewDV = - BuildMI(*MBB, MI, DL, TII->get(TargetOpcode::DBG_VALUE)) - .addFrameIndex(FI) - .addImm(Offset) - .addMetadata(Var) - .addMetadata(Expr); + MachineInstr *NewDV = buildDbgValueForSpill(*MBB, MI, *DBG, FI); assert(NewDV->getParent() == MBB && "dangling parent pointer"); (void)NewDV; DEBUG(dbgs() << "Inserting debug info due to spill:" << "\n" << *NewDV); Index: lib/CodeGen/SafeStack.cpp =================================================================== --- lib/CodeGen/SafeStack.cpp +++ lib/CodeGen/SafeStack.cpp @@ -550,7 +550,7 @@ // Replace alloc with the new location. replaceDbgDeclare(Arg, BasePointer, BasePointer->getNextNode(), DIB, - /*Deref=*/true, -Offset); + /*Deref=*/false, -Offset); Arg->replaceAllUsesWith(NewArg); IRB.SetInsertPoint(cast(NewArg)->getNextNode()); IRB.CreateMemCpy(Off, Arg, Size, Arg->getParamAlignment()); @@ -565,7 +565,7 @@ if (Size == 0) Size = 1; // Don't create zero-sized stack objects. - replaceDbgDeclareForAlloca(AI, BasePointer, DIB, /*Deref=*/true, -Offset); + replaceDbgDeclareForAlloca(AI, BasePointer, DIB, /*Deref=*/false, -Offset); replaceDbgValueForAlloca(AI, BasePointer, DIB, -Offset); // Replace uses of the alloca with the new location. @@ -655,7 +655,7 @@ if (AI->hasName() && isa(NewAI)) NewAI->takeName(AI); - replaceDbgDeclareForAlloca(AI, NewAI, DIB, /*Deref=*/true); + replaceDbgDeclareForAlloca(AI, NewAI, DIB, /*Deref=*/false); AI->replaceAllUsesWith(NewAI); AI->eraseFromParent(); } Index: lib/CodeGen/SelectionDAG/FastISel.cpp =================================================================== --- lib/CodeGen/SelectionDAG/FastISel.cpp +++ lib/CodeGen/SelectionDAG/FastISel.cpp @@ -1167,7 +1167,7 @@ if (Op->isReg()) { Op->setIsDebug(true); BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, - TII.get(TargetOpcode::DBG_VALUE), false, Op->getReg(), 0, + TII.get(TargetOpcode::DBG_VALUE), true, Op->getReg(), 0, DI->getVariable(), DI->getExpression()); } else BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, Index: lib/Transforms/Instrumentation/AddressSanitizer.cpp =================================================================== --- lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -2586,7 +2586,7 @@ Value *NewAllocaPtr = IRB.CreateIntToPtr( IRB.CreateAdd(LocalStackBase, ConstantInt::get(IntptrTy, Desc.Offset)), AI->getType()); - replaceDbgDeclareForAlloca(AI, NewAllocaPtr, DIB, /*Deref=*/true); + replaceDbgDeclareForAlloca(AI, NewAllocaPtr, DIB, /*Deref=*/false); AI->replaceAllUsesWith(NewAllocaPtr); } Index: lib/Transforms/Utils/Local.cpp =================================================================== --- lib/Transforms/Utils/Local.cpp +++ lib/Transforms/Utils/Local.cpp @@ -1227,13 +1227,9 @@ // This is a call by-value or some other instruction that // takes a pointer to the variable. Insert a *value* // intrinsic that describes the alloca. - SmallVector NewDIExpr; - auto *DIExpr = DDI->getExpression(); - NewDIExpr.push_back(dwarf::DW_OP_deref); - NewDIExpr.append(DIExpr->elements_begin(), DIExpr->elements_end()); DIB.insertDbgValueIntrinsic(AI, 0, DDI->getVariable(), - DIB.createExpression(NewDIExpr), - DDI->getDebugLoc(), CI); + DDI->getExpression(), DDI->getDebugLoc(), + CI); } } DDI->eraseFromParent(); Index: test/Bitcode/DIExpression-aggresult.ll =================================================================== --- /dev/null +++ test/Bitcode/DIExpression-aggresult.ll @@ -0,0 +1,36 @@ +; RUN: llvm-dis -o - %s.bc | FileCheck %s +%class.A = type { i32, i32, i32, i32 } + +define void @_Z3fooi(%class.A* sret %agg.result) #0 !dbg !3 { + ; CHECK: call void @llvm.dbg.declare({{.*}}, metadata ![[EXPR:[0-9]+]]), !dbg + ; CHECK: ![[EXPR]] = !DIExpression() + call void @llvm.dbg.declare(metadata %class.A* %agg.result, metadata !13, metadata !16), !dbg !17 + ret void, !dbg !17 +} + +declare void @llvm.dbg.declare(metadata, metadata, metadata) #1 + +attributes #0 = { ssp } +attributes #1 = { nounwind readnone } + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!2} + +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug) +!1 = !DIFile(filename: "a.cc", directory: "/tmp") +!2 = !{i32 1, !"Debug Info Version", i32 3} +!3 = distinct !DISubprogram(name: "foo", linkageName: "_Z3fooi", scope: !1, file: !1, line: 4, type: !4, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !0) +!4 = !DISubroutineType(types: !5) +!5 = !{!6} +!6 = !DICompositeType(tag: DW_TAG_class_type, name: "A", scope: !0, file: !1, line: 2, size: 128, align: 32, elements: !7) +!7 = !{!8, !10, !11, !12} +!8 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !1, file: !1, line: 2, baseType: !9, size: 32, align: 32) +!9 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed) +!10 = !DIDerivedType(tag: DW_TAG_member, name: "y", scope: !1, file: !1, line: 2, baseType: !9, size: 32, align: 32, offset: 32) +!11 = !DIDerivedType(tag: DW_TAG_member, name: "z", scope: !1, file: !1, line: 2, baseType: !9, size: 32, align: 32, offset: 64) +!12 = !DIDerivedType(tag: DW_TAG_member, name: "o", scope: !1, file: !1, line: 2, baseType: !9, size: 32, align: 32, offset: 96) +!13 = !DILocalVariable(name: "my_a", scope: !14, file: !1, line: 9, type: !15) +!14 = distinct !DILexicalBlock(scope: !3, file: !1, line: 4, column: 14) +!15 = !DIDerivedType(tag: DW_TAG_reference_type, file: !1, baseType: !6) +!16 = !DIExpression(DW_OP_deref) +!17 = !DILocation(line: 9, column: 5, scope: !3) Index: test/Bitcode/DIExpression-deref.ll =================================================================== --- /dev/null +++ test/Bitcode/DIExpression-deref.ll @@ -0,0 +1,27 @@ +; RUN: llvm-dis -o - %s.bc | FileCheck %s + +!llvm.dbg.cu = !{!1} +!llvm.module.flags = !{!20, !21} + +!0 = distinct !DIGlobalVariable(name: "g", scope: !1, file: !2, line: 1, type: !5, isLocal: false, isDefinition: true) +!1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang (llvm/trunk 288154)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !3, globals: !4) +!2 = !DIFile(filename: "a.c", directory: "/") +!3 = !{} +!4 = !{!10, !11, !12, !13} +!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) +; DW_OP_deref should be moved to the back of the expression. +; +; CHECK: !DIExpression(DW_OP_plus, 0, DW_OP_deref, DW_OP_LLVM_fragment, 8, 32) +!6 = !DIExpression(DW_OP_deref, DW_OP_plus, 0, DW_OP_LLVM_fragment, 8, 32) +; CHECK: !DIExpression(DW_OP_plus, 0, DW_OP_deref) +!7 = !DIExpression(DW_OP_deref, DW_OP_plus, 0) +; CHECK: !DIExpression(DW_OP_plus, 1, DW_OP_deref) +!8 = !DIExpression(DW_OP_plus, 1, DW_OP_deref) +; CHECK: !DIExpression(DW_OP_deref) +!9 = !DIExpression(DW_OP_deref) +!10 = !DIGlobalVariableExpression(var: !0, expr: !6) +!11 = !DIGlobalVariableExpression(var: !0, expr: !7) +!12 = !DIGlobalVariableExpression(var: !0, expr: !8) +!13 = !DIGlobalVariableExpression(var: !0, expr: !9) +!20 = !{i32 2, !"Dwarf Version", i32 4} +!21 = !{i32 2, !"Debug Info Version", i32 3} Index: test/DebugInfo/ARM/selectiondag-deadcode.ll =================================================================== --- test/DebugInfo/ARM/selectiondag-deadcode.ll +++ test/DebugInfo/ARM/selectiondag-deadcode.ll @@ -13,7 +13,7 @@ ; and SelectionDAGISel crashes. It should definitely not ; crash. Drop the dbg_value instead. ; CHECK-NOT: "matrix" - tail call void @llvm.dbg.declare(metadata %class.Matrix3.0.6.10* %agg.result, metadata !45, metadata !DIExpression(DW_OP_deref)) + tail call void @llvm.dbg.declare(metadata %class.Matrix3.0.6.10* %agg.result, metadata !45, metadata !DIExpression()) %2 = getelementptr inbounds %class.Matrix3.0.6.10, %class.Matrix3.0.6.10* %agg.result, i32 0, i32 0, i32 8 ret void } Index: test/DebugInfo/Generic/block-asan.ll =================================================================== --- test/DebugInfo/Generic/block-asan.ll +++ test/DebugInfo/Generic/block-asan.ll @@ -13,7 +13,7 @@ ; Check that the location of the ASAN instrumented __block variable is ; correct. -; CHECK: !DIExpression(DW_OP_deref, DW_OP_plus, 8, DW_OP_deref, DW_OP_plus, 24) +; CHECK: !DIExpression(DW_OP_plus, 8, DW_OP_deref, DW_OP_plus, 24) target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" Index: test/DebugInfo/X86/dbg_value_direct.ll =================================================================== --- test/DebugInfo/X86/dbg_value_direct.ll +++ test/DebugInfo/X86/dbg_value_direct.ll @@ -70,7 +70,7 @@ ;