From 9997e0397156ff7e01aecbd17bdeb7bfe5fb15b0 Mon Sep 17 00:00:00 2001 From: Orlando Cazalet-Hyams Date: Tue, 12 Mar 2024 10:25:58 +0000 Subject: [PATCH] [RemoveDIs] Update DIBuilder to conditionally insert DbgRecords (#84739) Have DIBuilder conditionally insert either debug intrinsics or DbgRecord depending on the module's IsNewDbgInfoFormat flag. The insertion methods now return a `DbgInstPtr` (a `PointerUnion`). Add a unittest for both modes (I couldn't find an existing test testing insertion behaviours specifically). This patch changes the existing assumption that DbgRecords are only ever inserted if there's an instruction to insert-before because clang currently inserts debug intrinsics while CodeGening (like any other instruction) meaning it'll try inserting to the end of a block without a terminator. We already have machinery in place to maintain the DbgRecords when a terminator is removed - these become "trailing DbgRecords" which are re-attached when a new instruction is inserted. All I've done is allow this state to occur while inserting DbgRecords too, i.e., it's not only removing terminators that causes this valid transient state, but inserting DbgRecords into incomplete blocks too. The C API will be updated in follow up patches. --- Note: this doesn't mean clang is emitting DbgRecords yet, because the modules it creates are still always in the old debug mode. That will come in a future patch. --- llvm/include/llvm/IR/DIBuilder.h | 73 +++++---- llvm/lib/IR/BasicBlock.cpp | 13 +- llvm/lib/IR/DIBuilder.cpp | 131 ++++++++++----- llvm/lib/IR/DebugInfo.cpp | 89 +++++----- llvm/lib/IR/Instruction.cpp | 3 +- llvm/lib/Transforms/Scalar/SROA.cpp | 47 +++--- llvm/lib/Transforms/Utils/Local.cpp | 12 +- .../Utils/PromoteMemoryToRegister.cpp | 25 ++- llvm/unittests/IR/IRBuilderTest.cpp | 152 +++++++++++++++--- 9 files changed, 358 insertions(+), 187 deletions(-) diff --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h index edec161b397155..94af17af8160e9 100644 --- a/llvm/include/llvm/IR/DIBuilder.h +++ b/llvm/include/llvm/IR/DIBuilder.h @@ -38,6 +38,9 @@ namespace llvm { class Module; class Value; class DbgAssignIntrinsic; + class DbgRecord; + + using DbgInstPtr = PointerUnion; class DIBuilder { Module &M; @@ -90,13 +93,17 @@ namespace llvm { void trackIfUnresolved(MDNode *N); /// Internal helper for insertDeclare. - Instruction *insertDeclare(llvm::Value *Storage, DILocalVariable *VarInfo, - DIExpression *Expr, const DILocation *DL, - BasicBlock *InsertBB, Instruction *InsertBefore); + DbgInstPtr insertDeclare(llvm::Value *Storage, DILocalVariable *VarInfo, + DIExpression *Expr, const DILocation *DL, + BasicBlock *InsertBB, Instruction *InsertBefore); /// Internal helper for insertLabel. - Instruction *insertLabel(DILabel *LabelInfo, const DILocation *DL, - BasicBlock *InsertBB, Instruction *InsertBefore); + DbgInstPtr insertLabel(DILabel *LabelInfo, const DILocation *DL, + BasicBlock *InsertBB, Instruction *InsertBefore); + + /// Internal helper. Track metadata if untracked and insert \p DPV. + void insertDPValue(DPValue *DPV, BasicBlock *InsertBB, + Instruction *InsertBefore, bool InsertAtHead = false); /// Internal helper with common code used by insertDbg{Value,Addr}Intrinsic. Instruction *insertDbgIntrinsic(llvm::Function *Intrinsic, llvm::Value *Val, @@ -106,10 +113,11 @@ namespace llvm { Instruction *InsertBefore); /// Internal helper for insertDbgValueIntrinsic. - Instruction * - insertDbgValueIntrinsic(llvm::Value *Val, DILocalVariable *VarInfo, - DIExpression *Expr, const DILocation *DL, - BasicBlock *InsertBB, Instruction *InsertBefore); + DbgInstPtr insertDbgValueIntrinsic(llvm::Value *Val, + DILocalVariable *VarInfo, + DIExpression *Expr, const DILocation *DL, + BasicBlock *InsertBB, + Instruction *InsertBefore); public: /// Construct a builder for a module. @@ -921,9 +929,9 @@ namespace llvm { /// \param Expr A complex location expression. /// \param DL Debug info location. /// \param InsertAtEnd Location for the new intrinsic. - Instruction *insertDeclare(llvm::Value *Storage, DILocalVariable *VarInfo, - DIExpression *Expr, const DILocation *DL, - BasicBlock *InsertAtEnd); + DbgInstPtr insertDeclare(llvm::Value *Storage, DILocalVariable *VarInfo, + DIExpression *Expr, const DILocation *DL, + BasicBlock *InsertAtEnd); /// Insert a new llvm.dbg.assign intrinsic call. /// \param LinkedInstr Instruction with a DIAssignID to link with the new @@ -939,11 +947,10 @@ namespace llvm { /// \param DL Debug info location, usually: (line: 0, /// column: 0, scope: var-decl-scope). See /// getDebugValueLoc. - DbgAssignIntrinsic *insertDbgAssign(Instruction *LinkedInstr, Value *Val, - DILocalVariable *SrcVar, - DIExpression *ValExpr, Value *Addr, - DIExpression *AddrExpr, - const DILocation *DL); + DbgInstPtr insertDbgAssign(Instruction *LinkedInstr, Value *Val, + DILocalVariable *SrcVar, DIExpression *ValExpr, + Value *Addr, DIExpression *AddrExpr, + const DILocation *DL); /// Insert a new llvm.dbg.declare intrinsic call. /// \param Storage llvm::Value of the variable @@ -951,23 +958,23 @@ namespace llvm { /// \param Expr A complex location expression. /// \param DL Debug info location. /// \param InsertBefore Location for the new intrinsic. - Instruction *insertDeclare(llvm::Value *Storage, DILocalVariable *VarInfo, - DIExpression *Expr, const DILocation *DL, - Instruction *InsertBefore); + DbgInstPtr insertDeclare(llvm::Value *Storage, DILocalVariable *VarInfo, + DIExpression *Expr, const DILocation *DL, + Instruction *InsertBefore); /// Insert a new llvm.dbg.label intrinsic call. /// \param LabelInfo Label's debug info descriptor. /// \param DL Debug info location. /// \param InsertBefore Location for the new intrinsic. - Instruction *insertLabel(DILabel *LabelInfo, const DILocation *DL, - Instruction *InsertBefore); + DbgInstPtr insertLabel(DILabel *LabelInfo, const DILocation *DL, + Instruction *InsertBefore); /// Insert a new llvm.dbg.label intrinsic call. /// \param LabelInfo Label's debug info descriptor. /// \param DL Debug info location. /// \param InsertAtEnd Location for the new intrinsic. - Instruction *insertLabel(DILabel *LabelInfo, const DILocation *DL, - BasicBlock *InsertAtEnd); + DbgInstPtr insertLabel(DILabel *LabelInfo, const DILocation *DL, + BasicBlock *InsertAtEnd); /// Insert a new llvm.dbg.value intrinsic call. /// \param Val llvm::Value of the variable @@ -975,11 +982,10 @@ namespace llvm { /// \param Expr A complex location expression. /// \param DL Debug info location. /// \param InsertAtEnd Location for the new intrinsic. - Instruction *insertDbgValueIntrinsic(llvm::Value *Val, - DILocalVariable *VarInfo, - DIExpression *Expr, - const DILocation *DL, - BasicBlock *InsertAtEnd); + DbgInstPtr insertDbgValueIntrinsic(llvm::Value *Val, + DILocalVariable *VarInfo, + DIExpression *Expr, const DILocation *DL, + BasicBlock *InsertAtEnd); /// Insert a new llvm.dbg.value intrinsic call. /// \param Val llvm::Value of the variable @@ -987,11 +993,10 @@ namespace llvm { /// \param Expr A complex location expression. /// \param DL Debug info location. /// \param InsertBefore Location for the new intrinsic. - Instruction *insertDbgValueIntrinsic(llvm::Value *Val, - DILocalVariable *VarInfo, - DIExpression *Expr, - const DILocation *DL, - Instruction *InsertBefore); + DbgInstPtr insertDbgValueIntrinsic(llvm::Value *Val, + DILocalVariable *VarInfo, + DIExpression *Expr, const DILocation *DL, + Instruction *InsertBefore); /// Replace the vtable holder in the given type. /// diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp index c188d2f912d16b..673e2f68249cd6 100644 --- a/llvm/lib/IR/BasicBlock.cpp +++ b/llvm/lib/IR/BasicBlock.cpp @@ -754,8 +754,6 @@ void BasicBlock::spliceDebugInfoEmptyBlock(BasicBlock::iterator Dest, // occur when a block is optimised away and the terminator has been moved // somewhere else. if (Src->empty()) { - assert(Dest != end() && - "Transferring trailing DPValues to another trailing position"); DPMarker *SrcTrailingDPValues = Src->getTrailingDPValues(); if (!SrcTrailingDPValues) return; @@ -1040,15 +1038,10 @@ void BasicBlock::insertDPValueAfter(DbgRecord *DPV, Instruction *I) { void BasicBlock::insertDPValueBefore(DbgRecord *DPV, InstListType::iterator Where) { - // We should never directly insert at the end of the block, new DPValues - // shouldn't be generated at times when there's no terminator. - assert(Where != end()); - assert(Where->getParent() == this); - if (!Where->DbgMarker) - createMarker(Where); + assert(Where == end() || Where->getParent() == this); bool InsertAtHead = Where.getHeadBit(); - createMarker(&*Where); - Where->DbgMarker->insertDPValue(DPV, InsertAtHead); + DPMarker *M = createMarker(Where); + M->insertDPValue(DPV, InsertAtHead); } DPMarker *BasicBlock::getNextMarker(Instruction *I) { diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp index 62efaba025344b..c0643f63c9725b 100644 --- a/llvm/lib/IR/DIBuilder.cpp +++ b/llvm/lib/IR/DIBuilder.cpp @@ -925,35 +925,47 @@ DILexicalBlock *DIBuilder::createLexicalBlock(DIScope *Scope, DIFile *File, File, Line, Col); } -Instruction *DIBuilder::insertDeclare(Value *Storage, DILocalVariable *VarInfo, - DIExpression *Expr, const DILocation *DL, - Instruction *InsertBefore) { +DbgInstPtr DIBuilder::insertDeclare(Value *Storage, DILocalVariable *VarInfo, + DIExpression *Expr, const DILocation *DL, + Instruction *InsertBefore) { return insertDeclare(Storage, VarInfo, Expr, DL, InsertBefore->getParent(), InsertBefore); } -Instruction *DIBuilder::insertDeclare(Value *Storage, DILocalVariable *VarInfo, - DIExpression *Expr, const DILocation *DL, - BasicBlock *InsertAtEnd) { +DbgInstPtr DIBuilder::insertDeclare(Value *Storage, DILocalVariable *VarInfo, + DIExpression *Expr, const DILocation *DL, + BasicBlock *InsertAtEnd) { // If this block already has a terminator then insert this intrinsic before // the terminator. Otherwise, put it at the end of the block. Instruction *InsertBefore = InsertAtEnd->getTerminator(); return insertDeclare(Storage, VarInfo, Expr, DL, InsertAtEnd, InsertBefore); } -DbgAssignIntrinsic * -DIBuilder::insertDbgAssign(Instruction *LinkedInstr, Value *Val, - DILocalVariable *SrcVar, DIExpression *ValExpr, - Value *Addr, DIExpression *AddrExpr, - const DILocation *DL) { +DbgInstPtr DIBuilder::insertDbgAssign(Instruction *LinkedInstr, Value *Val, + DILocalVariable *SrcVar, + DIExpression *ValExpr, Value *Addr, + DIExpression *AddrExpr, + const DILocation *DL) { + auto *Link = cast_or_null( + LinkedInstr->getMetadata(LLVMContext::MD_DIAssignID)); + assert(Link && "Linked instruction must have DIAssign metadata attached"); + + if (M.IsNewDbgInfoFormat) { + DPValue *DPV = DPValue::createDPVAssign(Val, SrcVar, ValExpr, Link, Addr, + AddrExpr, DL); + BasicBlock *InsertBB = LinkedInstr->getParent(); + // Insert after LinkedInstr. + BasicBlock::iterator NextIt = std::next(LinkedInstr->getIterator()); + Instruction *InsertBefore = NextIt == InsertBB->end() ? nullptr : &*NextIt; + insertDPValue(DPV, InsertBB, InsertBefore, true); + return DPV; + } + LLVMContext &Ctx = LinkedInstr->getContext(); Module *M = LinkedInstr->getModule(); if (!AssignFn) AssignFn = Intrinsic::getDeclaration(M, Intrinsic::dbg_assign); - auto *Link = LinkedInstr->getMetadata(LLVMContext::MD_DIAssignID); - assert(Link && "Linked instruction must have DIAssign metadata attached"); - std::array Args = { MetadataAsValue::get(Ctx, ValueAsMetadata::get(Val)), MetadataAsValue::get(Ctx, SrcVar), @@ -971,35 +983,36 @@ DIBuilder::insertDbgAssign(Instruction *LinkedInstr, Value *Val, return DVI; } -Instruction *DIBuilder::insertLabel(DILabel *LabelInfo, const DILocation *DL, - Instruction *InsertBefore) { +DbgInstPtr DIBuilder::insertLabel(DILabel *LabelInfo, const DILocation *DL, + Instruction *InsertBefore) { return insertLabel(LabelInfo, DL, InsertBefore ? InsertBefore->getParent() : nullptr, InsertBefore); } -Instruction *DIBuilder::insertLabel(DILabel *LabelInfo, const DILocation *DL, - BasicBlock *InsertAtEnd) { +DbgInstPtr DIBuilder::insertLabel(DILabel *LabelInfo, const DILocation *DL, + BasicBlock *InsertAtEnd) { return insertLabel(LabelInfo, DL, InsertAtEnd, nullptr); } -Instruction *DIBuilder::insertDbgValueIntrinsic(Value *V, - DILocalVariable *VarInfo, - DIExpression *Expr, - const DILocation *DL, - Instruction *InsertBefore) { - Instruction *DVI = insertDbgValueIntrinsic( +DbgInstPtr DIBuilder::insertDbgValueIntrinsic(Value *V, + DILocalVariable *VarInfo, + DIExpression *Expr, + const DILocation *DL, + Instruction *InsertBefore) { + DbgInstPtr DVI = insertDbgValueIntrinsic( V, VarInfo, Expr, DL, InsertBefore ? InsertBefore->getParent() : nullptr, InsertBefore); - cast(DVI)->setTailCall(); + if (DVI.is()) + cast(DVI.get())->setTailCall(); return DVI; } -Instruction *DIBuilder::insertDbgValueIntrinsic(Value *V, - DILocalVariable *VarInfo, - DIExpression *Expr, - const DILocation *DL, - BasicBlock *InsertAtEnd) { +DbgInstPtr DIBuilder::insertDbgValueIntrinsic(Value *V, + DILocalVariable *VarInfo, + DIExpression *Expr, + const DILocation *DL, + BasicBlock *InsertAtEnd) { return insertDbgValueIntrinsic(V, VarInfo, Expr, DL, InsertAtEnd, nullptr); } @@ -1023,24 +1036,37 @@ static Function *getDeclareIntrin(Module &M) { return Intrinsic::getDeclaration(&M, Intrinsic::dbg_declare); } -Instruction *DIBuilder::insertDbgValueIntrinsic( +DbgInstPtr DIBuilder::insertDbgValueIntrinsic( llvm::Value *Val, DILocalVariable *VarInfo, DIExpression *Expr, const DILocation *DL, BasicBlock *InsertBB, Instruction *InsertBefore) { + if (M.IsNewDbgInfoFormat) { + DPValue *DPV = DPValue::createDPValue(Val, VarInfo, Expr, DL); + insertDPValue(DPV, InsertBB, InsertBefore); + return DPV; + } + if (!ValueFn) ValueFn = Intrinsic::getDeclaration(&M, Intrinsic::dbg_value); return insertDbgIntrinsic(ValueFn, Val, VarInfo, Expr, DL, InsertBB, InsertBefore); } -Instruction *DIBuilder::insertDeclare(Value *Storage, DILocalVariable *VarInfo, - DIExpression *Expr, const DILocation *DL, - BasicBlock *InsertBB, - Instruction *InsertBefore) { +DbgInstPtr DIBuilder::insertDeclare(Value *Storage, DILocalVariable *VarInfo, + DIExpression *Expr, const DILocation *DL, + BasicBlock *InsertBB, + Instruction *InsertBefore) { assert(VarInfo && "empty or invalid DILocalVariable* passed to dbg.declare"); assert(DL && "Expected debug loc"); assert(DL->getScope()->getSubprogram() == VarInfo->getScope()->getSubprogram() && "Expected matching subprograms"); + + if (M.IsNewDbgInfoFormat) { + DPValue *DPV = DPValue::createDPVDeclare(Storage, VarInfo, Expr, DL); + insertDPValue(DPV, InsertBB, InsertBefore); + return DPV; + } + if (!DeclareFn) DeclareFn = getDeclareIntrin(M); @@ -1055,6 +1081,23 @@ Instruction *DIBuilder::insertDeclare(Value *Storage, DILocalVariable *VarInfo, return B.CreateCall(DeclareFn, Args); } +void DIBuilder::insertDPValue(DPValue *DPV, BasicBlock *InsertBB, + Instruction *InsertBefore, bool InsertAtHead) { + assert(InsertBefore || InsertBB); + trackIfUnresolved(DPV->getVariable()); + trackIfUnresolved(DPV->getExpression()); + if (DPV->isDbgAssign()) + trackIfUnresolved(DPV->getAddressExpression()); + + BasicBlock::iterator InsertPt; + if (InsertBB && InsertBefore) + InsertPt = InsertBefore->getIterator(); + else if (InsertBB) + InsertPt = InsertBB->end(); + InsertPt.setHeadBit(InsertAtHead); + InsertBB->insertDPValueBefore(DPV, InsertPt); +} + Instruction *DIBuilder::insertDbgIntrinsic(llvm::Function *IntrinsicFn, Value *V, DILocalVariable *VarInfo, DIExpression *Expr, @@ -1081,18 +1124,28 @@ Instruction *DIBuilder::insertDbgIntrinsic(llvm::Function *IntrinsicFn, return B.CreateCall(IntrinsicFn, Args); } -Instruction *DIBuilder::insertLabel(DILabel *LabelInfo, const DILocation *DL, - BasicBlock *InsertBB, - Instruction *InsertBefore) { +DbgInstPtr DIBuilder::insertLabel(DILabel *LabelInfo, const DILocation *DL, + BasicBlock *InsertBB, + Instruction *InsertBefore) { assert(LabelInfo && "empty or invalid DILabel* passed to dbg.label"); assert(DL && "Expected debug loc"); assert(DL->getScope()->getSubprogram() == LabelInfo->getScope()->getSubprogram() && "Expected matching subprograms"); + + trackIfUnresolved(LabelInfo); + if (M.IsNewDbgInfoFormat) { + DPLabel *DPL = new DPLabel(LabelInfo, DL); + if (InsertBB && InsertBefore) + InsertBB->insertDPValueBefore(DPL, InsertBefore->getIterator()); + else if (InsertBB) + InsertBB->insertDPValueBefore(DPL, InsertBB->end()); + return DPL; + } + if (!LabelFn) LabelFn = Intrinsic::getDeclaration(&M, Intrinsic::dbg_label); - trackIfUnresolved(LabelInfo); Value *Args[] = {MetadataAsValue::get(VMContext, LabelInfo)}; IRBuilder<> B(DL->getContext()); diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp index 1f3ff2246a4453..68fd244e256974 100644 --- a/llvm/lib/IR/DebugInfo.cpp +++ b/llvm/lib/IR/DebugInfo.cpp @@ -1663,43 +1663,47 @@ LLVMValueRef LLVMDIBuilderInsertDeclareBefore(LLVMDIBuilderRef Builder, LLVMValueRef Storage, LLVMMetadataRef VarInfo, LLVMMetadataRef Expr, LLVMMetadataRef DL, LLVMValueRef Instr) { - return wrap(unwrap(Builder)->insertDeclare( - unwrap(Storage), unwrap(VarInfo), - unwrap(Expr), unwrap(DL), - unwrap(Instr))); -} - -LLVMValueRef LLVMDIBuilderInsertDeclareAtEnd( - LLVMDIBuilderRef Builder, LLVMValueRef Storage, LLVMMetadataRef VarInfo, - LLVMMetadataRef Expr, LLVMMetadataRef DL, LLVMBasicBlockRef Block) { - return wrap(unwrap(Builder)->insertDeclare( - unwrap(Storage), unwrap(VarInfo), - unwrap(Expr), unwrap(DL), - unwrap(Block))); -} - -LLVMValueRef LLVMDIBuilderInsertDbgValueBefore(LLVMDIBuilderRef Builder, - LLVMValueRef Val, - LLVMMetadataRef VarInfo, - LLVMMetadataRef Expr, - LLVMMetadataRef DebugLoc, - LLVMValueRef Instr) { - return wrap(unwrap(Builder)->insertDbgValueIntrinsic( - unwrap(Val), unwrap(VarInfo), - unwrap(Expr), unwrap(DebugLoc), - unwrap(Instr))); -} - -LLVMValueRef LLVMDIBuilderInsertDbgValueAtEnd(LLVMDIBuilderRef Builder, - LLVMValueRef Val, - LLVMMetadataRef VarInfo, - LLVMMetadataRef Expr, - LLVMMetadataRef DebugLoc, - LLVMBasicBlockRef Block) { - return wrap(unwrap(Builder)->insertDbgValueIntrinsic( - unwrap(Val), unwrap(VarInfo), - unwrap(Expr), unwrap(DebugLoc), - unwrap(Block))); + DbgInstPtr DbgInst = unwrap(Builder)->insertDeclare( + unwrap(Storage), unwrap(VarInfo), + unwrap(Expr), unwrap(DL), + unwrap(Instr)); + assert(isa(DbgInst) && + "Inserted a DbgRecord into function using old debug info mode"); + return wrap(cast(DbgInst)); +} + +LLVMValueRef +LLVMDIBuilderInsertDeclareAtEnd(LLVMDIBuilderRef Builder, LLVMValueRef Storage, + LLVMMetadataRef VarInfo, LLVMMetadataRef Expr, + LLVMMetadataRef DL, LLVMBasicBlockRef Block) { + DbgInstPtr DbgInst = unwrap(Builder)->insertDeclare( + unwrap(Storage), unwrap(VarInfo), + unwrap(Expr), unwrap(DL), unwrap(Block)); + assert(isa(DbgInst) && + "Inserted a DbgRecord into function using old debug info mode"); + return wrap(cast(DbgInst)); +} + +LLVMValueRef LLVMDIBuilderInsertDbgValueBefore( + LLVMDIBuilderRef Builder, LLVMValueRef Val, LLVMMetadataRef VarInfo, + LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMValueRef Instr) { + DbgInstPtr DbgInst = unwrap(Builder)->insertDbgValueIntrinsic( + unwrap(Val), unwrap(VarInfo), unwrap(Expr), + unwrap(DebugLoc), unwrap(Instr)); + assert(isa(DbgInst) && + "Inserted a DbgRecord into function using old debug info mode"); + return wrap(cast(DbgInst)); +} + +LLVMValueRef LLVMDIBuilderInsertDbgValueAtEnd( + LLVMDIBuilderRef Builder, LLVMValueRef Val, LLVMMetadataRef VarInfo, + LLVMMetadataRef Expr, LLVMMetadataRef DebugLoc, LLVMBasicBlockRef Block) { + DbgInstPtr DbgInst = unwrap(Builder)->insertDbgValueIntrinsic( + unwrap(Val), unwrap(VarInfo), unwrap(Expr), + unwrap(DebugLoc), unwrap(Block)); + assert(isa(DbgInst) && + "Inserted a DbgRecord into function using old debug info mode"); + return wrap(cast(DbgInst)); } LLVMMetadataRef LLVMDIBuilderCreateAutoVariable( @@ -2115,10 +2119,15 @@ static void emitDbgAssign(AssignmentInfo Info, Value *Val, Value *Dest, LLVM_DEBUG(if (Assign) errs() << " > INSERT: " << *Assign << "\n"); return; } - auto *Assign = DIB.insertDbgAssign(&StoreLikeInst, Val, VarRec.Var, Expr, - Dest, AddrExpr, VarRec.DL); + auto Assign = DIB.insertDbgAssign(&StoreLikeInst, Val, VarRec.Var, Expr, Dest, + AddrExpr, VarRec.DL); (void)Assign; - LLVM_DEBUG(if (Assign) errs() << " > INSERT: " << *Assign << "\n"); + LLVM_DEBUG(if (!Assign.isNull()) { + if (Assign.is()) + errs() << " > INSERT: " << *Assign.get() << "\n"; + else + errs() << " > INSERT: " << *Assign.get() << "\n"; + }); } #undef DEBUG_TYPE // Silence redefinition warning (from ConstantsContext.h). diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp index e863ef3eb8d6d7..6b8c6e0c85ed98 100644 --- a/llvm/lib/IR/Instruction.cpp +++ b/llvm/lib/IR/Instruction.cpp @@ -166,7 +166,8 @@ void Instruction::insertBefore(BasicBlock &BB, } // If we're inserting a terminator, check if we need to flush out - // TrailingDPValues. + // TrailingDPValues. Inserting instructions at the end of an incomplete + // block is handled by the code block above. if (isTerminator()) getParent()->flushTerminatorDbgValues(); } diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp index e11b984f13bbc4..190fee11618bf4 100644 --- a/llvm/lib/Transforms/Scalar/SROA.cpp +++ b/llvm/lib/Transforms/Scalar/SROA.cpp @@ -324,23 +324,16 @@ static DebugVariable getAggregateVariable(DPValue *DPV) { DPV->getDebugLoc().getInlinedAt()); } -static DPValue *createLinkedAssign(DPValue *, DIBuilder &DIB, - Instruction *LinkedInstr, Value *NewValue, - DILocalVariable *Variable, - DIExpression *Expression, Value *Address, - DIExpression *AddressExpression, - const DILocation *DI) { - (void)DIB; - return DPValue::createLinkedDPVAssign(LinkedInstr, NewValue, Variable, - Expression, Address, AddressExpression, - DI); +/// Helpers for handling new and old debug info modes in migrateDebugInfo. +/// These overloads unwrap a DbgInstPtr {Instruction* | DbgRecord*} union based +/// on the \p Unused parameter type. +DPValue *UnwrapDbgInstPtr(DbgInstPtr P, DPValue *Unused) { + (void)Unused; + return static_cast(cast(P)); } -static DbgAssignIntrinsic *createLinkedAssign( - DbgAssignIntrinsic *, DIBuilder &DIB, Instruction *LinkedInstr, - Value *NewValue, DILocalVariable *Variable, DIExpression *Expression, - Value *Address, DIExpression *AddressExpression, const DILocation *DI) { - return DIB.insertDbgAssign(LinkedInstr, NewValue, Variable, Expression, - Address, AddressExpression, DI); +DbgAssignIntrinsic *UnwrapDbgInstPtr(DbgInstPtr P, DbgAssignIntrinsic *Unused) { + (void)Unused; + return static_cast(cast(P)); } /// Find linked dbg.assign and generate a new one with the correct @@ -398,7 +391,7 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit, DIBuilder DIB(*OldInst->getModule(), /*AllowUnresolved*/ false); assert(OldAlloca->isStaticAlloca()); - auto MigrateDbgAssign = [&](auto DbgAssign) { + auto MigrateDbgAssign = [&](auto *DbgAssign) { LLVM_DEBUG(dbgs() << " existing dbg.assign is: " << *DbgAssign << "\n"); auto *Expr = DbgAssign->getExpression(); @@ -452,10 +445,12 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit, } ::Value *NewValue = Value ? Value : DbgAssign->getValue(); - auto *NewAssign = createLinkedAssign( - DbgAssign, DIB, Inst, NewValue, DbgAssign->getVariable(), Expr, Dest, - DIExpression::get(Expr->getContext(), std::nullopt), - DbgAssign->getDebugLoc()); + auto *NewAssign = UnwrapDbgInstPtr( + DIB.insertDbgAssign(Inst, NewValue, DbgAssign->getVariable(), Expr, + Dest, + DIExpression::get(Expr->getContext(), std::nullopt), + DbgAssign->getDebugLoc()), + DbgAssign); // If we've updated the value but the original dbg.assign has an arglist // then kill it now - we can't use the requested new value. @@ -5031,9 +5026,11 @@ static void insertNewDbgInst(DIBuilder &DIB, DbgAssignIntrinsic *Orig, NewAddr->setMetadata(LLVMContext::MD_DIAssignID, DIAssignID::getDistinct(NewAddr->getContext())); } - auto *NewAssign = DIB.insertDbgAssign( - NewAddr, Orig->getValue(), Orig->getVariable(), NewFragmentExpr, NewAddr, - Orig->getAddressExpression(), Orig->getDebugLoc()); + Instruction *NewAssign = + DIB.insertDbgAssign(NewAddr, Orig->getValue(), Orig->getVariable(), + NewFragmentExpr, NewAddr, + Orig->getAddressExpression(), Orig->getDebugLoc()) + .get(); LLVM_DEBUG(dbgs() << "Created new assign intrinsic: " << *NewAssign << "\n"); (void)NewAssign; } @@ -5052,7 +5049,7 @@ static void insertNewDbgInst(DIBuilder &DIB, DPValue *Orig, AllocaInst *NewAddr, NewAddr->setMetadata(LLVMContext::MD_DIAssignID, DIAssignID::getDistinct(NewAddr->getContext())); } - auto *NewAssign = DPValue::createLinkedDPVAssign( + DPValue *NewAssign = DPValue::createLinkedDPVAssign( NewAddr, Orig->getValue(), Orig->getVariable(), NewFragmentExpr, NewAddr, Orig->getAddressExpression(), Orig->getDebugLoc()); LLVM_DEBUG(dbgs() << "Created new DPVAssign: " << *NewAssign << "\n"); diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index d3bb89075015e9..a44536e34c9225 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -1649,9 +1649,9 @@ static void insertDbgValueOrDPValue(DIBuilder &Builder, Value *DV, const DebugLoc &NewLoc, BasicBlock::iterator Instr) { if (!UseNewDbgInfoFormat) { - auto *DbgVal = Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, - (Instruction *)nullptr); - DbgVal->insertBefore(Instr); + auto DbgVal = Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, + (Instruction *)nullptr); + DbgVal.get()->insertBefore(Instr); } else { // RemoveDIs: if we're using the new debug-info format, allocate a // DPValue directly instead of a dbg.value intrinsic. @@ -1667,9 +1667,9 @@ static void insertDbgValueOrDPValueAfter(DIBuilder &Builder, Value *DV, const DebugLoc &NewLoc, BasicBlock::iterator Instr) { if (!UseNewDbgInfoFormat) { - auto *DbgVal = Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, - (Instruction *)nullptr); - DbgVal->insertAfter(&*Instr); + auto DbgVal = Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, NewLoc, + (Instruction *)nullptr); + DbgVal.get()->insertAfter(&*Instr); } else { // RemoveDIs: if we're using the new debug-info format, allocate a // DPValue directly instead of a dbg.value intrinsic. diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp index 88b05aab8db4df..b462803bad38c3 100644 --- a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp +++ b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp @@ -101,21 +101,20 @@ bool llvm::isAllocaPromotable(const AllocaInst *AI) { namespace { -static DPValue *createDebugValue(DIBuilder &DIB, Value *NewValue, - DILocalVariable *Variable, - DIExpression *Expression, const DILocation *DI, - DPValue *InsertBefore) { +static void createDebugValue(DIBuilder &DIB, Value *NewValue, + DILocalVariable *Variable, + DIExpression *Expression, const DILocation *DI, + DPValue *InsertBefore) { + // FIXME: Merge these two functions now that DIBuilder supports DPValues. + // We neeed the API to accept DPValues as an insert point for that to work. (void)DIB; - return DPValue::createDPValue(NewValue, Variable, Expression, DI, - *InsertBefore); + DPValue::createDPValue(NewValue, Variable, Expression, DI, *InsertBefore); } -static DbgValueInst *createDebugValue(DIBuilder &DIB, Value *NewValue, - DILocalVariable *Variable, - DIExpression *Expression, - const DILocation *DI, - Instruction *InsertBefore) { - return static_cast(DIB.insertDbgValueIntrinsic( - NewValue, Variable, Expression, DI, InsertBefore)); +static void createDebugValue(DIBuilder &DIB, Value *NewValue, + DILocalVariable *Variable, + DIExpression *Expression, const DILocation *DI, + Instruction *InsertBefore) { + DIB.insertDbgValueIntrinsic(NewValue, Variable, Expression, DI, InsertBefore); } /// Helper for updating assignment tracking debug info when promoting allocas. diff --git a/llvm/unittests/IR/IRBuilderTest.cpp b/llvm/unittests/IR/IRBuilderTest.cpp index d15ff9dd51a4c4..cece65974c013c 100644 --- a/llvm/unittests/IR/IRBuilderTest.cpp +++ b/llvm/unittests/IR/IRBuilderTest.cpp @@ -871,25 +871,139 @@ TEST_F(IRBuilderTest, createFunction) { } TEST_F(IRBuilderTest, DIBuilder) { - IRBuilder<> Builder(BB); - DIBuilder DIB(*M); - auto File = DIB.createFile("F.CBL", "/"); - auto CU = DIB.createCompileUnit(dwarf::DW_LANG_Cobol74, - DIB.createFile("F.CBL", "/"), "llvm-cobol74", - true, "", 0); - auto Type = DIB.createSubroutineType(DIB.getOrCreateTypeArray(std::nullopt)); - auto SP = DIB.createFunction( - CU, "foo", "", File, 1, Type, 1, DINode::FlagZero, - DISubprogram::SPFlagDefinition | DISubprogram::SPFlagOptimized); - F->setSubprogram(SP); - AllocaInst *I = Builder.CreateAlloca(Builder.getInt8Ty()); - auto BarSP = DIB.createFunction( - CU, "bar", "", File, 1, Type, 1, DINode::FlagZero, - DISubprogram::SPFlagDefinition | DISubprogram::SPFlagOptimized); - auto BadScope = DIB.createLexicalBlockFile(BarSP, File, 0); - I->setDebugLoc(DILocation::get(Ctx, 2, 0, BadScope)); - DIB.finalize(); - EXPECT_TRUE(verifyModule(*M)); + auto GetLastDbgRecord = [](const Instruction *I) -> DbgRecord * { + if (I->getDbgValueRange().empty()) + return nullptr; + return &*std::prev(I->getDbgValueRange().end()); + }; + + auto ExpectOrder = [&](DbgInstPtr First, BasicBlock::iterator Second) { + if (M->IsNewDbgInfoFormat) { + EXPECT_TRUE(First.is()); + EXPECT_FALSE(Second->getDbgValueRange().empty()); + EXPECT_EQ(GetLastDbgRecord(&*Second), First.get()); + } else { + EXPECT_TRUE(First.is()); + EXPECT_EQ(&*std::prev(Second), First.get()); + } + }; + + auto RunTest = [&]() { + IRBuilder<> Builder(BB); + DIBuilder DIB(*M); + auto File = DIB.createFile("F.CBL", "/"); + auto CU = DIB.createCompileUnit(dwarf::DW_LANG_Cobol74, + DIB.createFile("F.CBL", "/"), + "llvm-cobol74", true, "", 0); + auto Type = + DIB.createSubroutineType(DIB.getOrCreateTypeArray(std::nullopt)); + auto SP = DIB.createFunction( + CU, "foo", "", File, 1, Type, 1, DINode::FlagZero, + DISubprogram::SPFlagDefinition | DISubprogram::SPFlagOptimized); + F->setSubprogram(SP); + AllocaInst *I = Builder.CreateAlloca(Builder.getInt8Ty()); + auto BarSP = DIB.createFunction( + CU, "bar", "", File, 1, Type, 1, DINode::FlagZero, + DISubprogram::SPFlagDefinition | DISubprogram::SPFlagOptimized); + auto BarScope = DIB.createLexicalBlockFile(BarSP, File, 0); + I->setDebugLoc(DILocation::get(Ctx, 2, 0, BarScope)); + + // Create another instruction so that there's one before the alloca we're + // inserting debug intrinsics before, to make end-checking easier. + I = Builder.CreateAlloca(Builder.getInt1Ty()); + + // Label metadata and records + // -------------------------- + DILocation *LabelLoc = DILocation::get(Ctx, 1, 0, BarScope); + DILabel *AlwaysPreserveLabel = DIB.createLabel( + BarScope, "meles_meles", File, 1, /*AlwaysPreserve*/ true); + DILabel *Label = + DIB.createLabel(BarScope, "badger", File, 1, /*AlwaysPreserve*/ false); + + { /* dbg.label | DPLabel */ + // Insert before I and check order. + ExpectOrder(DIB.insertLabel(Label, LabelLoc, I), I->getIterator()); + + // We should be able to insert at the end of the block, even if there's + // no terminator yet. Note that in RemoveDIs mode this record won't get + // inserted into the block untill another instruction is added. + DbgInstPtr LabelRecord = DIB.insertLabel(Label, LabelLoc, BB); + // Specifically do not insert a terminator, to check this works. `I` + // should have absorbed the DPLabel in the new debug info mode. + I = Builder.CreateAlloca(Builder.getInt32Ty()); + ExpectOrder(LabelRecord, I->getIterator()); + } + + // Variable metadata and records + // ----------------------------- + DILocation *VarLoc = DILocation::get(Ctx, 2, 0, BarScope); + auto *IntType = DIB.createBasicType("int", 32, dwarf::DW_ATE_signed); + DILocalVariable *VarX = + DIB.createAutoVariable(BarSP, "X", File, 2, IntType, true); + DILocalVariable *VarY = + DIB.createAutoVariable(BarSP, "Y", File, 2, IntType, true); + { /* dbg.value | DPValue::Value */ + ExpectOrder(DIB.insertDbgValueIntrinsic(I, VarX, DIB.createExpression(), + VarLoc, I), + I->getIterator()); + // Check inserting at end of the block works as with labels. + DbgInstPtr VarXValue = DIB.insertDbgValueIntrinsic( + I, VarX, DIB.createExpression(), VarLoc, BB); + I = Builder.CreateAlloca(Builder.getInt32Ty()); + ExpectOrder(VarXValue, I->getIterator()); + EXPECT_EQ(BB->getTrailingDPValues(), nullptr); + } + { /* dbg.declare | DPValue::Declare */ + ExpectOrder(DIB.insertDeclare(I, VarY, DIB.createExpression(), VarLoc, I), + I->getIterator()); + // Check inserting at end of the block works as with labels. + DbgInstPtr VarYDeclare = + DIB.insertDeclare(I, VarY, DIB.createExpression(), VarLoc, BB); + I = Builder.CreateAlloca(Builder.getInt32Ty()); + ExpectOrder(VarYDeclare, I->getIterator()); + EXPECT_EQ(BB->getTrailingDPValues(), nullptr); + } + { /* dbg.assign | DPValue::Assign */ + I = Builder.CreateAlloca(Builder.getInt32Ty()); + I->setMetadata(LLVMContext::MD_DIAssignID, DIAssignID::getDistinct(Ctx)); + // DbgAssign interface is slightly different - it always inserts after the + // linked instr. Check we can do this with no instruction to insert + // before. + DbgInstPtr VarXAssign = + DIB.insertDbgAssign(I, I, VarX, DIB.createExpression(), I, + DIB.createExpression(), VarLoc); + I = Builder.CreateAlloca(Builder.getInt32Ty()); + ExpectOrder(VarXAssign, I->getIterator()); + EXPECT_EQ(BB->getTrailingDPValues(), nullptr); + } + + Builder.CreateRet(nullptr); + DIB.finalize(); + // Check the labels are not/are added to Bar's retainedNodes array + // (AlwaysPreserve). + EXPECT_EQ(find(BarSP->getRetainedNodes(), Label), + BarSP->getRetainedNodes().end()); + EXPECT_NE(find(BarSP->getRetainedNodes(), AlwaysPreserveLabel), + BarSP->getRetainedNodes().end()); + EXPECT_NE(find(BarSP->getRetainedNodes(), VarX), + BarSP->getRetainedNodes().end()); + EXPECT_NE(find(BarSP->getRetainedNodes(), VarY), + BarSP->getRetainedNodes().end()); + EXPECT_TRUE(verifyModule(*M)); + }; + + // Test in old-debug mode. + EXPECT_FALSE(M->IsNewDbgInfoFormat); + RunTest(); + + // Test in new-debug mode. + // Reset the test then call convertToNewDbgValues to flip the flag + // on the test's Module, Function and BasicBlock. + TearDown(); + SetUp(); + M->convertToNewDbgValues(); + EXPECT_TRUE(M->IsNewDbgInfoFormat); + RunTest(); } TEST_F(IRBuilderTest, createArtificialSubprogram) {