Index: llvm/include/llvm/IR/BasicBlock.h =================================================================== --- llvm/include/llvm/IR/BasicBlock.h +++ llvm/include/llvm/IR/BasicBlock.h @@ -63,6 +63,9 @@ /// scenarios, debug-info must be stored somewhere temporarily before the /// block returns to a cannonical form when the terminator is re-inserted. DPMarker TrailingDPValues; + /// Flag recording whether or not this block stores debug-info in the form + /// of intrinsic instructions (false) or non-instruction records (true). + bool IsNewDbgInfoFormat; private: friend class BlockAddress; @@ -71,6 +74,35 @@ InstListType InstList; Function *Parent; +public: + /// Attach a DPMarker to the given instruction. Enables the storage of any + /// debug-info at this position in the program. + DPMarker *createMarker(Instruction *I); + + /// Convert variable location debugging information stored in dbg.value + /// intrinsics into DPMarker / DPValue records. Deletes all dbg.values in + /// the process and sets IsNewDbgInfoFormat = true. Only takes effect if + /// the UseNewDbgInfoFormat LLVM command line option is given. + void convertToNewDbgValues(); + + /// Convert variable location debugging information stored in DPMarkers and + /// DPValues into the dbg.value intrinsic representation. Sets + /// IsNewDbgInfoFormat = false. + void convertFromNewDbgValues(); + + /// Set the value of the IsNewDbgInfoFormat flag. + void setIsNewDbgInfoFormat(bool NewFlag); + + /// Validate any DPMarkers / DPValues attached to instructions in this block, + /// and block-level stored data too (TrailingDPValues). + /// \p Assert Should this method fire an assertion if a problem is found? + /// \p Msg Should this method print a message to errs() if a problem is found? + /// \returns True if a problem is found. + bool validateDbgValues(bool Assert = true, bool Msg = false); + + void dumpDbgValues() const; + +private: void setParent(Function *parent); /// Constructor. Index: llvm/include/llvm/IR/Function.h =================================================================== --- llvm/include/llvm/IR/Function.h +++ llvm/include/llvm/IR/Function.h @@ -97,15 +97,27 @@ friend class SymbolTableListTraits; +public: + /// Is this function using intrinsics to record the position of debugging + /// information, or non-intrinsic records? + bool IsNewDbgInfoFormat; + /// hasLazyArguments/CheckLazyArguments - The argument list of a function is /// built on demand, so that the list isn't allocated until the first client /// needs it. The hasLazyArguments predicate returns true if the arg list /// hasn't been set up yet. -public: bool hasLazyArguments() const { return getSubclassDataFromValue() & (1<<0); } + /// \see BasicBlock::convertToNewDbgValues. + void convertToNewDbgValues(); + + /// \see BasicBlock::convertFromNewDbgValues. + void convertFromNewDbgValues(); + + void setIsNewDbgInfoFormat(bool NewVal); + private: void CheckLazyArguments() const { if (hasLazyArguments()) @@ -685,6 +697,7 @@ /// Insert \p BB in the basic block list at \p Position. \Returns an iterator /// to the newly inserted BB. Function::iterator insert(Function::iterator Position, BasicBlock *BB) { + BB->setIsNewDbgInfoFormat(IsNewDbgInfoFormat); return BasicBlocks.insert(Position, BB); } Index: llvm/include/llvm/IR/Module.h =================================================================== --- llvm/include/llvm/IR/Module.h +++ llvm/include/llvm/IR/Module.h @@ -213,6 +213,26 @@ /// @name Constructors /// @{ public: + /// Is this Module using intrinsics to record the position of debugging + /// information, or non-intrinsic records? + bool IsNewDbgInfoFormat; + + /// \see BasicBlock::convertToNewDbgValues. + void convertToNewDbgValues() { + for (auto &F : *this) { + F.convertToNewDbgValues(); + } + IsNewDbgInfoFormat = true; + } + + /// \see BasicBlock::convertFromNewDbgValues. + void convertFromNewDbgValues() { + for (auto &F : *this) { + F.convertFromNewDbgValues(); + } + IsNewDbgInfoFormat = false; + } + /// The Module constructor. Note that there is no default constructor. You /// must provide a name for the module upon construction. explicit Module(StringRef ModuleID, LLVMContext& C); Index: llvm/lib/IR/BasicBlock.cpp =================================================================== --- llvm/lib/IR/BasicBlock.cpp +++ llvm/lib/IR/BasicBlock.cpp @@ -20,12 +20,149 @@ #include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Type.h" +#include "llvm/Support/CommandLine.h" using namespace llvm; #define DEBUG_TYPE "ir" STATISTIC(NumInstrRenumberings, "Number of renumberings across all blocks"); +cl::opt UseNewDbgInfoFormat("experimental-debuginfo-iterators", + cl::desc("Enable communicating debuginfo positions through iterators, eliminating intrinsics"), + cl::init(false)); + +DPMarker *BasicBlock::createMarker(Instruction *I) { + assert(IsNewDbgInfoFormat && "Tried to create a marker in a non new debug-info block!"); + assert(I->DbgMarker == nullptr && + "Tried to create marker for instuction that already has one!"); + DPMarker *Marker = new DPMarker(); + Marker->MarkedInstr = I; + I->DbgMarker = Marker; + return Marker; +} + +void BasicBlock::convertToNewDbgValues() { + // Is the command line option set? + if (!UseNewDbgInfoFormat) + return; + + IsNewDbgInfoFormat = true; + + // Iterate over all instructions in the instruction list, collecting dbg.value + // instructions and converting them to DPValues. Once we find a "real" + // instruction, attach all those DPValues to a DPMarker in that instruction. + SmallVector DPVals; + for (Instruction &I : make_early_inc_range(InstList)) { + assert(!I.DbgMarker && "DbgMarker already set on old-format instrs?"); + if (DbgValueInst *DVI = dyn_cast(&I)) { + // Convert this dbg.value to a DPValue. + DPValue *Value = new DPValue(DVI); + DPVals.push_back(Value); + DVI->eraseFromParent(); + continue; + } + + // Create a marker to store DPValues in. Technically we don't need to store + // one marker per instruction, but that's a future optimisation. + createMarker(&I); + DPMarker *Marker = I.DbgMarker; + + for (DPValue *DPV : DPVals) + Marker->insertDPValue(DPV, false); + + DPVals.clear(); + } +} + +void BasicBlock::convertFromNewDbgValues() { + invalidateOrders(); + IsNewDbgInfoFormat = false; + + // Iterate over the block, finding instructions annotated with DPMarkers. + // Convert any attached DPValues to dbg.values and insert ahead of the + // instruction. + for (auto &Inst : *this) { + if (!Inst.DbgMarker) + continue; + + DPMarker &Marker = *Inst.DbgMarker; + for (DPValue &DPV : Marker.getDbgValueRange()) + InstList.insert(Inst.getIterator(), DPV.createDebugIntrinsic(getModule(), nullptr)); + + Marker.eraseFromParent(); + }; + + // Assume no trailing DPValues: we could technically create them at the end + // of the block, after a terminator, but this would be non-cannonical and + // indicates that something else is broken somewhere. + assert(TrailingDPValues.StoredDPValues.empty()); +} + +bool BasicBlock::validateDbgValues(bool Assert, bool Msg) { + bool RetVal = false; + + // Helper lambda for reporting failures: via assertion, printing, and return + // value. + auto TestFailure = [Assert, Msg, &RetVal](bool Val, const char *Text) { + // Did the test fail? + if (Val) + return; + + // If we're asserting, then fire off an assertion. + if (Assert) + llvm_unreachable(Text); + + if (Msg) + errs() << Text << "\n"; + RetVal = true; + }; + + // We should have the same debug-format as the parent function. + TestFailure(getParent()->IsNewDbgInfoFormat == IsNewDbgInfoFormat, + "Parent function doesn't have the same debug-info format"); + + // Only validate if we have a debug program. + if (!IsNewDbgInfoFormat) + return RetVal; + + // Match every DPMarker to every Instruction and vice versa, and + // verify that there are no invalid DPValues. + for (auto It = begin(); It != end(); ++It) { + if (!It->DbgMarker) + continue; + + // Validate DebugProgramMarkers. + DPMarker *CurrentDebugMarker = It->DbgMarker; + + // If this is a marker, it should match the instruction and vice versa. + TestFailure(CurrentDebugMarker->MarkedInstr == &*It, + "Debug Marker points to incorrect instruction?"); + + // Now validate any DPValues in the marker. + for (DPValue &DPV : CurrentDebugMarker->getDbgValueRange()) { + // Validate DebugProgramValues. + TestFailure(DPV.getMarker() == CurrentDebugMarker, + "Not pointing at correct next marker!"); + + // Verify that no DbgValues appear prior to PHIs. + TestFailure(!isa(It), + "DebugProgramValues must not appear before PHI nodes in a block!"); + } + } + + // Except transiently when removing + re-inserting the block terminator, there + // should be no trailing DPValues. + TestFailure(TrailingDPValues.StoredDPValues.empty(), "Trailing DPValues in block"); + return RetVal; +} + +void BasicBlock::setIsNewDbgInfoFormat(bool NewFlag) { + if (NewFlag && !IsNewDbgInfoFormat) + convertToNewDbgValues(); + else if (!NewFlag && IsNewDbgInfoFormat) + convertFromNewDbgValues(); +} + ValueSymbolTable *BasicBlock::getValueSymbolTable() { if (Function *F = getParent()) return F->getValueSymbolTable(); @@ -47,8 +184,9 @@ BasicBlock::BasicBlock(LLVMContext &C, const Twine &Name, Function *NewParent, BasicBlock *InsertBefore) - : Value(Type::getLabelTy(C), Value::BasicBlockVal), Parent(nullptr) { + : Value(Type::getLabelTy(C), Value::BasicBlockVal), Parent(nullptr) { + IsNewDbgInfoFormat = false; if (NewParent) insertInto(NewParent, InsertBefore); else @@ -56,12 +194,16 @@ "Cannot insert block before another block with no function!"); setName(Name); + if (NewParent) + setIsNewDbgInfoFormat(NewParent->IsNewDbgInfoFormat); } void BasicBlock::insertInto(Function *NewParent, BasicBlock *InsertBefore) { assert(NewParent && "Expected a parent"); assert(!Parent && "Already has a parent"); + setIsNewDbgInfoFormat(NewParent->IsNewDbgInfoFormat); + if (InsertBefore) NewParent->insert(InsertBefore->getIterator(), this); else @@ -91,6 +233,11 @@ assert(getParent() == nullptr && "BasicBlock still linked into the program!"); dropAllReferences(); + for (auto &Inst : *this) { + if (!Inst.DbgMarker) + continue; + Inst.DbgMarker->eraseFromParent(); + } InstList.clear(); } Index: llvm/lib/IR/Function.cpp =================================================================== --- llvm/lib/IR/Function.cpp +++ llvm/lib/IR/Function.cpp @@ -80,6 +80,27 @@ "non-global-value-max-name-size", cl::Hidden, cl::init(1024), cl::desc("Maximum size for the name of non-global values.")); +void Function::convertToNewDbgValues() { + IsNewDbgInfoFormat = true; + for (auto &BB : *this) { + BB.convertToNewDbgValues(); + } +} + +void Function::convertFromNewDbgValues() { + IsNewDbgInfoFormat = false; + for (auto &BB : *this) { + BB.convertFromNewDbgValues(); + } +} + +void Function::setIsNewDbgInfoFormat(bool NewFlag) { + if (NewFlag && !IsNewDbgInfoFormat) + convertToNewDbgValues(); + else if (!NewFlag && IsNewDbgInfoFormat) + convertFromNewDbgValues(); +} + //===----------------------------------------------------------------------===// // Argument Implementation //===----------------------------------------------------------------------===// @@ -401,7 +422,7 @@ : GlobalObject(Ty, Value::FunctionVal, OperandTraits::op_begin(this), 0, Linkage, name, computeAddrSpace(AddrSpace, ParentModule)), - NumArgs(Ty->getNumParams()) { + NumArgs(Ty->getNumParams()), IsNewDbgInfoFormat(false) { assert(FunctionType::isValidReturnType(getReturnType()) && "invalid return type"); setGlobalObjectSubClassData(0); Index: llvm/lib/IR/Module.cpp =================================================================== --- llvm/lib/IR/Module.cpp +++ llvm/lib/IR/Module.cpp @@ -71,7 +71,8 @@ Module::Module(StringRef MID, LLVMContext &C) : Context(C), ValSymTab(std::make_unique(-1)), - ModuleID(std::string(MID)), SourceFileName(std::string(MID)), DL("") { + ModuleID(std::string(MID)), SourceFileName(std::string(MID)), DL(""), + IsNewDbgInfoFormat(false) { Context.addModule(this); } Index: llvm/lib/IR/Verifier.cpp =================================================================== --- llvm/lib/IR/Verifier.cpp +++ llvm/lib/IR/Verifier.cpp @@ -2888,6 +2888,14 @@ { Check(I.getParent() == &BB, "Instruction has bogus parent pointer!"); } + + // Confirm that no issues arise from the debug program. + if (BB.IsNewDbgInfoFormat) { + // Configure the validate function to not fire assertions, instead print + // errors and return true if there's a problem. + bool RetVal = BB.validateDbgValues(false, true); + Check(!RetVal, "Invalid configuration of new-debug-info data found"); + } } void Verifier::visitTerminator(Instruction &I) { Index: llvm/lib/Linker/IRMover.cpp =================================================================== --- llvm/lib/Linker/IRMover.cpp +++ llvm/lib/Linker/IRMover.cpp @@ -1135,6 +1135,7 @@ Dst.setPrologueData(Src.getPrologueData()); if (Src.hasPersonalityFn()) Dst.setPersonalityFn(Src.getPersonalityFn()); + assert(Src.IsNewDbgInfoFormat == Dst.IsNewDbgInfoFormat); // Copy over the metadata attachments without remapping. Dst.copyMetadata(&Src, 0); Index: llvm/lib/Target/AMDGPU/AMDGPURewriteOutArguments.cpp =================================================================== --- llvm/lib/Target/AMDGPU/AMDGPURewriteOutArguments.cpp +++ llvm/lib/Target/AMDGPU/AMDGPURewriteOutArguments.cpp @@ -330,6 +330,8 @@ NewFunc->removeRetAttrs(RetAttrs); // TODO: How to preserve metadata? + NewFunc->setIsNewDbgInfoFormat(F.IsNewDbgInfoFormat); + // Move the body of the function into the new rewritten function, and replace // this function with a stub. NewFunc->splice(NewFunc->begin(), &F); Index: llvm/lib/Transforms/IPO/ArgumentPromotion.cpp =================================================================== --- llvm/lib/Transforms/IPO/ArgumentPromotion.cpp +++ llvm/lib/Transforms/IPO/ArgumentPromotion.cpp @@ -192,6 +192,7 @@ F->getName()); NF->copyAttributesFrom(F); NF->copyMetadata(F, 0); + NF->setIsNewDbgInfoFormat(F->IsNewDbgInfoFormat); // The new function will have the !dbg metadata copied from the original // function. The original function may not be deleted, and dbg metadata need Index: llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp =================================================================== --- llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp +++ llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp @@ -173,6 +173,7 @@ NF->setComdat(F.getComdat()); F.getParent()->getFunctionList().insert(F.getIterator(), NF); NF->takeName(&F); + NF->IsNewDbgInfoFormat = F.IsNewDbgInfoFormat; // Loop over all the callers of the function, transforming the call sites // to pass in a smaller number of arguments into the new function. @@ -876,6 +877,7 @@ // it again. F->getParent()->getFunctionList().insert(F->getIterator(), NF); NF->takeName(F); + NF->IsNewDbgInfoFormat = F->IsNewDbgInfoFormat; // Loop over all the callers of the function, transforming the call sites to // pass in a smaller number of arguments into the new function. Index: llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp =================================================================== --- llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp +++ llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp @@ -1775,6 +1775,7 @@ // sub-Scopes. assert(BB != PreEntryBlock && "Don't copy the preetntry block"); BasicBlock *NewBB = CloneBasicBlock(BB, VMap, ".nonchr", &F); + NewBB->setIsNewDbgInfoFormat(F.IsNewDbgInfoFormat); NewBlocks.push_back(NewBB); VMap[BB] = NewBB; } Index: llvm/lib/Transforms/Utils/CloneFunction.cpp =================================================================== --- llvm/lib/Transforms/Utils/CloneFunction.cpp +++ llvm/lib/Transforms/Utils/CloneFunction.cpp @@ -44,6 +44,7 @@ ClonedCodeInfo *CodeInfo, DebugInfoFinder *DIFinder) { BasicBlock *NewBB = BasicBlock::Create(BB->getContext(), "", F); + NewBB->IsNewDbgInfoFormat = BB->IsNewDbgInfoFormat; if (BB->hasName()) NewBB->setName(BB->getName() + NameSuffix); @@ -473,6 +474,7 @@ BBEntry = NewBB = BasicBlock::Create(BB->getContext()); if (BB->hasName()) NewBB->setName(BB->getName() + NameSuffix); + NewBB->IsNewDbgInfoFormat = BB->IsNewDbgInfoFormat; // It is only legal to clone a function if a block address within that // function is never referenced outside of the function. Given that, we Index: llvm/unittests/IR/DebugInfoTest.cpp =================================================================== --- llvm/unittests/IR/DebugInfoTest.cpp +++ llvm/unittests/IR/DebugInfoTest.cpp @@ -23,6 +23,8 @@ using namespace llvm; +extern cl::opt UseNewDbgInfoFormat; + static std::unique_ptr parseIR(LLVMContext &C, const char *IR) { SMDiagnostic Err; std::unique_ptr Mod = parseAssemblyString(IR, Err, C); @@ -853,4 +855,126 @@ } } +TEST(MetadataTest, DPValueConversionRoutines) { + LLVMContext C; + + // For the purpose of this test, set and un-set the command line option + // corresponding to UseNewDbgInfoFormat. + UseNewDbgInfoFormat = true; + + std::unique_ptr M = parseIR(C, R"( + define i16 @f(i16 %a) !dbg !6 { + call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11 + %b = add i16 %a, 1, !dbg !11 + call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11 + ret i16 0, !dbg !11 + + exit: + %c = add i16 %b, 1, !dbg !11 + ret i16 0, !dbg !11 + } + declare void @llvm.dbg.value(metadata, metadata, metadata) #0 + attributes #0 = { nounwind readnone speculatable willreturn } + + !llvm.dbg.cu = !{!0} + !llvm.module.flags = !{!5} + + !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2) + !1 = !DIFile(filename: "t.ll", directory: "/") + !2 = !{} + !5 = !{i32 2, !"Debug Info Version", i32 3} + !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8) + !7 = !DISubroutineType(types: !2) + !8 = !{!9} + !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10) + !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned) + !11 = !DILocation(line: 1, column: 1, scope: !6) +)"); + + // Check that the conversion routines and utilities between dbg.value + // debug-info format and DPValues works. + Function *F = M->getFunction("f"); + BasicBlock *BB1 = &F->getEntryBlock(); + // First instruction should be a dbg.value. + EXPECT_TRUE(isa(BB1->front())); + EXPECT_FALSE(BB1->IsNewDbgInfoFormat); + // Validating the block for DPValues / DPMarkers shouldn't fail -- there's + // no data stored right now. + EXPECT_FALSE(BB1->validateDbgValues(false, false)); + + // Function and module should be marked as not having the new format too. + EXPECT_FALSE(F->IsNewDbgInfoFormat); + EXPECT_FALSE(M->IsNewDbgInfoFormat); + + // Now convert. + M->convertToNewDbgValues(); + EXPECT_TRUE(M->IsNewDbgInfoFormat); + EXPECT_TRUE(F->IsNewDbgInfoFormat); + EXPECT_TRUE(BB1->IsNewDbgInfoFormat); + + // There should now be no dbg.value instructions! + // Ensure the first instruction exists, the test all of them. + EXPECT_FALSE(isa(BB1->front())); + for (auto &BB : *F) + for (auto &I : BB) + EXPECT_FALSE(isa(I)); + + // There should be a DPMarker on each of the two instructions in the entry + // block, each containing one DPValue. + EXPECT_EQ(BB1->size(), 2); + Instruction *FirstInst = &BB1->front(); + Instruction *SecondInst = FirstInst->getNextNode(); + ASSERT_TRUE(FirstInst->DbgMarker); + ASSERT_TRUE(SecondInst->DbgMarker); + EXPECT_NE(FirstInst->DbgMarker, SecondInst->DbgMarker); + EXPECT_EQ(FirstInst, FirstInst->DbgMarker->MarkedInstr); + EXPECT_EQ(SecondInst, SecondInst->DbgMarker->MarkedInstr); + + EXPECT_EQ(FirstInst->DbgMarker->StoredDPValues.size(), 1); + DPValue *DPV1 = &*FirstInst->DbgMarker->getDbgValueRange().begin(); + EXPECT_EQ(DPV1->getMarker(), FirstInst->DbgMarker); + // Should point at %a, an argument. + EXPECT_TRUE(isa(DPV1->getVariableLocationOp(0))); + + EXPECT_EQ(SecondInst->DbgMarker->StoredDPValues.size(), 1); + DPValue *DPV2 = &*SecondInst->DbgMarker->getDbgValueRange().begin(); + EXPECT_EQ(DPV2->getMarker(), SecondInst->DbgMarker); + // Should point at FirstInst. + EXPECT_EQ(DPV2->getVariableLocationOp(0), FirstInst); + + // There should be no DPValues / DPMarkers in the second block, but it should + // be marked as being in the new format. + BasicBlock *BB2 = BB1->getNextNode(); + EXPECT_TRUE(BB2->IsNewDbgInfoFormat); + for (auto &Inst : *BB2) + // Either there should be no marker, or it should be empty. + EXPECT_TRUE(!Inst.DbgMarker || Inst.DbgMarker->StoredDPValues.empty()); + + // Validating the first block should continue to not be a problem, + EXPECT_FALSE(BB1->validateDbgValues(false, false)); + // But if we were to break something, it should be able to fire. Don't attempt + // to comprehensively test the validator, it's a smoke-test rather than a + // "proper" verification pass. + DPV1->setMarker(nullptr); + // A marker pointing the wrong way should be an error. + EXPECT_TRUE(BB1->validateDbgValues(false, false)); + DPV1->setMarker(FirstInst->DbgMarker); + + // Convert everything back to the "old" format and ensure it's right. + M->convertFromNewDbgValues(); + EXPECT_FALSE(M->IsNewDbgInfoFormat); + EXPECT_FALSE(F->IsNewDbgInfoFormat); + EXPECT_FALSE(BB1->IsNewDbgInfoFormat); + + EXPECT_EQ(BB1->size(), 4); + ASSERT_TRUE(isa(BB1->front())); + DbgValueInst *DVI1 = cast(&BB1->front()); + // These dbg.values should still point at the same places. + EXPECT_TRUE(isa(DVI1->getVariableLocationOp(0))); + DbgValueInst *DVI2 = cast(DVI1->getNextNode()->getNextNode()); + EXPECT_EQ(DVI2->getVariableLocationOp(0), FirstInst); + + UseNewDbgInfoFormat = false; +} + } // end namespace