From 0e8296d5f34eedcf5aa7779687c4dc58aa6fe9de Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Wed, 27 Mar 2024 21:12:41 +0000 Subject: [PATCH 1/5] [RemoveDIs] Load into new debug format by default in LLVM --- llvm/include/llvm/AsmParser/LLParser.h | 1 - llvm/lib/AsmParser/LLParser.cpp | 34 ++++++------ llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 2 +- llvm/lib/IR/BasicBlock.cpp | 2 +- llvm/lib/IR/Function.cpp | 4 +- llvm/lib/IR/Module.cpp | 4 +- llvm/tools/llvm-as/llvm-as.cpp | 7 +-- llvm/tools/llvm-dis/llvm-dis.cpp | 2 +- llvm/tools/llvm-link/llvm-link.cpp | 8 +-- .../Analysis/IRSimilarityIdentifierTest.cpp | 32 +++++++++++ llvm/unittests/IR/BasicBlockDbgInfoTest.cpp | 17 ------ llvm/unittests/IR/DebugInfoTest.cpp | 15 ++--- llvm/unittests/IR/InstructionsTest.cpp | 5 ++ .../Transforms/Utils/CloningTest.cpp | 5 +- llvm/unittests/Transforms/Utils/LocalTest.cpp | 55 +++++++++++++++++++ 15 files changed, 132 insertions(+), 61 deletions(-) diff --git a/llvm/include/llvm/AsmParser/LLParser.h b/llvm/include/llvm/AsmParser/LLParser.h index b2dcdfad0a04b..e687254f6c4c7 100644 --- a/llvm/include/llvm/AsmParser/LLParser.h +++ b/llvm/include/llvm/AsmParser/LLParser.h @@ -337,7 +337,6 @@ namespace llvm { // Top-Level Entities bool parseTopLevelEntities(); - bool finalizeDebugInfoFormat(Module *M); void dropUnknownMetadataReferences(); bool validateEndOfModule(bool UpgradeDebugInfo); bool validateEndOfIndex(); diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 2902bd9fe17c4..34053a5ca9c8e 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -74,23 +74,6 @@ static std::string getTypeString(Type *T) { return Tmp.str(); } -// Whatever debug info format we parsed, we should convert to the expected debug -// info format immediately afterwards. -bool LLParser::finalizeDebugInfoFormat(Module *M) { - // We should have already returned an error if we observed both intrinsics and - // records in this IR. - assert(!(SeenNewDbgInfoFormat && SeenOldDbgInfoFormat) && - "Mixed debug intrinsics/records seen without a parsing error?"); - if (PreserveInputDbgFormat == cl::boolOrDefault::BOU_TRUE) { - UseNewDbgInfoFormat = SeenNewDbgInfoFormat; - WriteNewDbgInfoFormatToBitcode = SeenNewDbgInfoFormat; - WriteNewDbgInfoFormat = SeenNewDbgInfoFormat; - } else if (M) { - M->setIsNewDbgInfoFormat(false); - } - return false; -} - /// Run: module ::= toplevelentity* bool LLParser::Run(bool UpgradeDebugInfo, DataLayoutCallbackTy DataLayoutCallback) { @@ -108,7 +91,7 @@ bool LLParser::Run(bool UpgradeDebugInfo, } return parseTopLevelEntities() || validateEndOfModule(UpgradeDebugInfo) || - validateEndOfIndex() || finalizeDebugInfoFormat(M); + validateEndOfIndex(); } bool LLParser::parseStandaloneConstantValue(Constant *&C, @@ -207,6 +190,18 @@ void LLParser::dropUnknownMetadataReferences() { bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) { if (!M) return false; + + // We should have already returned an error if we observed both intrinsics and + // records in this IR. + assert(!(SeenNewDbgInfoFormat && SeenOldDbgInfoFormat) && + "Mixed debug intrinsics/records seen without a parsing error?"); + if (PreserveInputDbgFormat == cl::boolOrDefault::BOU_TRUE) { + UseNewDbgInfoFormat = SeenNewDbgInfoFormat; + WriteNewDbgInfoFormatToBitcode = SeenNewDbgInfoFormat; + WriteNewDbgInfoFormat = SeenNewDbgInfoFormat; + M->setNewDbgInfoFormatFlag(SeenNewDbgInfoFormat); + } + // Handle any function attribute group forward references. for (const auto &RAG : ForwardRefAttrGroups) { Value *V = RAG.first; @@ -439,6 +434,9 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) { UpgradeModuleFlags(*M); UpgradeSectionAttributes(*M); + if (PreserveInputDbgFormat != cl::boolOrDefault::BOU_TRUE) + M->setIsNewDbgInfoFormat(UseNewDbgInfoFormat); + if (!Slots) return false; // Initialize the slot mapping. diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index 0b7fcd8841889..73fe63b5b8f6f 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -4319,7 +4319,7 @@ Error BitcodeReader::parseModule(uint64_t ResumeBit, if (PreserveInputDbgFormat != cl::boolOrDefault::BOU_TRUE) { TheModule->IsNewDbgInfoFormat = UseNewDbgInfoFormat && - LoadBitcodeIntoNewDbgInfoFormat == cl::boolOrDefault::BOU_TRUE; + LoadBitcodeIntoNewDbgInfoFormat != cl::boolOrDefault::BOU_FALSE; } this->ValueTypeCallback = std::move(Callbacks.ValueType); diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp index 6e62767c99e2a..73595dbe280be 100644 --- a/llvm/lib/IR/BasicBlock.cpp +++ b/llvm/lib/IR/BasicBlock.cpp @@ -181,7 +181,7 @@ template class llvm::SymbolTableListTraits NonGlobalValueMaxNameSize( "non-global-value-max-name-size", cl::Hidden, cl::init(1024), cl::desc("Maximum size for the name of non-global values.")); +extern cl::opt UseNewDbgInfoFormat; + void Function::convertToNewDbgValues() { IsNewDbgInfoFormat = true; for (auto &BB : *this) { @@ -438,7 +440,7 @@ Function::Function(FunctionType *Ty, LinkageTypes Linkage, unsigned AddrSpace, : GlobalObject(Ty, Value::FunctionVal, OperandTraits::op_begin(this), 0, Linkage, name, computeAddrSpace(AddrSpace, ParentModule)), - NumArgs(Ty->getNumParams()), IsNewDbgInfoFormat(false) { + NumArgs(Ty->getNumParams()), IsNewDbgInfoFormat(UseNewDbgInfoFormat) { assert(FunctionType::isValidReturnType(getReturnType()) && "invalid return type"); setGlobalObjectSubClassData(0); diff --git a/llvm/lib/IR/Module.cpp b/llvm/lib/IR/Module.cpp index a8696ed9e3ce5..915fa5097383c 100644 --- a/llvm/lib/IR/Module.cpp +++ b/llvm/lib/IR/Module.cpp @@ -54,6 +54,8 @@ using namespace llvm; +extern cl::opt UseNewDbgInfoFormat; + //===----------------------------------------------------------------------===// // Methods to implement the globals and functions lists. // @@ -72,7 +74,7 @@ template class llvm::SymbolTableListTraits; Module::Module(StringRef MID, LLVMContext &C) : Context(C), ValSymTab(std::make_unique(-1)), ModuleID(std::string(MID)), SourceFileName(std::string(MID)), DL(""), - IsNewDbgInfoFormat(false) { + IsNewDbgInfoFormat(UseNewDbgInfoFormat) { Context.addModule(this); } diff --git a/llvm/tools/llvm-as/llvm-as.cpp b/llvm/tools/llvm-as/llvm-as.cpp index e48e3f4d22c12..0958e16c2197a 100644 --- a/llvm/tools/llvm-as/llvm-as.cpp +++ b/llvm/tools/llvm-as/llvm-as.cpp @@ -142,11 +142,10 @@ int main(int argc, char **argv) { } // Convert to new debug format if requested. - assert(!M->IsNewDbgInfoFormat && "Unexpectedly in new debug mode"); - if (UseNewDbgInfoFormat && WriteNewDbgInfoFormatToBitcode) { - M->convertToNewDbgValues(); + M->setIsNewDbgInfoFormat(UseNewDbgInfoFormat && + WriteNewDbgInfoFormatToBitcode); + if (M->IsNewDbgInfoFormat) M->removeDebugIntrinsicDeclarations(); - } std::unique_ptr Index = std::move(ModuleAndIndex.Index); diff --git a/llvm/tools/llvm-dis/llvm-dis.cpp b/llvm/tools/llvm-dis/llvm-dis.cpp index fbbb5506e43e0..d28af85bc739e 100644 --- a/llvm/tools/llvm-dis/llvm-dis.cpp +++ b/llvm/tools/llvm-dis/llvm-dis.cpp @@ -258,7 +258,7 @@ int main(int argc, char **argv) { // All that llvm-dis does is write the assembly to a file. if (!DontPrint) { if (M) { - ScopedDbgInfoFormatSetter FormatSetter(*M, WriteNewDbgInfoFormat); + M->setIsNewDbgInfoFormat(WriteNewDbgInfoFormat); if (WriteNewDbgInfoFormat) M->removeDebugIntrinsicDeclarations(); M->print(Out->os(), Annotator.get(), PreserveAssemblyUseListOrder); diff --git a/llvm/tools/llvm-link/llvm-link.cpp b/llvm/tools/llvm-link/llvm-link.cpp index 7794f2d81ed06..b84469d1c757f 100644 --- a/llvm/tools/llvm-link/llvm-link.cpp +++ b/llvm/tools/llvm-link/llvm-link.cpp @@ -489,12 +489,6 @@ int main(int argc, char **argv) { if (LoadBitcodeIntoNewDbgInfoFormat == cl::boolOrDefault::BOU_UNSET) LoadBitcodeIntoNewDbgInfoFormat = cl::boolOrDefault::BOU_TRUE; - // RemoveDIs debug-info transition: tests may request that we /try/ to use the - // new debug-info format. - if (TryUseNewDbgInfoFormat) { - // Turn the new debug-info format on. - UseNewDbgInfoFormat = true; - } // Since llvm-link collects multiple IR modules together, for simplicity's // sake we disable the "PreserveInputDbgFormat" flag to enforce a single // debug info format. @@ -556,7 +550,7 @@ int main(int argc, char **argv) { SetFormat(WriteNewDbgInfoFormat); Composite->print(Out.os(), nullptr, PreserveAssemblyUseListOrder); } else if (Force || !CheckBitcodeOutputToConsole(Out.os())) { - SetFormat(WriteNewDbgInfoFormatToBitcode); + SetFormat(UseNewDbgInfoFormat && WriteNewDbgInfoFormatToBitcode); WriteBitcodeToFile(*Composite, Out.os(), PreserveBitcodeUseListOrder); } diff --git a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp index f6a053792f852..69ea590945668 100644 --- a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp +++ b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "llvm/Analysis/IRSimilarityIdentifier.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/AsmParser/Parser.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Module.h" @@ -22,6 +23,27 @@ using namespace llvm; using namespace IRSimilarity; +extern llvm::cl::opt UseNewDbgInfoFormat; +extern cl::opt PreserveInputDbgFormat; +extern bool WriteNewDbgInfoFormatToBitcode; +extern cl::opt WriteNewDbgInfoFormat; + +// Backup all of the existing settings that may be modified when +// PreserveInputDbgFormat=true, so that when the test is finished we return them +// (and the "preserve" setting) to their original values. +auto TempSettingChange() { + return make_scope_exit( + [OldPreserveInputDbgFormat = PreserveInputDbgFormat.getValue(), + OldUseNewDbgInfoFormat = UseNewDbgInfoFormat.getValue(), + OldWriteNewDbgInfoFormatToBitcode = WriteNewDbgInfoFormatToBitcode, + OldWriteNewDbgInfoFormat = WriteNewDbgInfoFormat.getValue()] { + PreserveInputDbgFormat = OldPreserveInputDbgFormat; + UseNewDbgInfoFormat = OldUseNewDbgInfoFormat; + WriteNewDbgInfoFormatToBitcode = OldWriteNewDbgInfoFormatToBitcode; + WriteNewDbgInfoFormat = OldWriteNewDbgInfoFormat; + }); +} + static std::unique_ptr makeLLVMModule(LLVMContext &Context, StringRef ModuleStr) { SMDiagnostic Err; @@ -1308,6 +1330,9 @@ TEST(IRInstructionMapper, CallBrInstIllegal) { // Checks that an debuginfo intrinsics are mapped to be invisible. Since they // do not semantically change the program, they can be recognized as similar. +// FIXME: PreserveInputDbgFormat is set to true because this test contains +// malformed debug info that cannot be converted to the new debug info format; +// this test should be updated later to use valid debug info. TEST(IRInstructionMapper, DebugInfoInvisible) { StringRef ModuleString = R"( define i32 @f(i32 %a, i32 %b) { @@ -1320,6 +1345,8 @@ TEST(IRInstructionMapper, DebugInfoInvisible) { declare void @llvm.dbg.value(metadata) !0 = distinct !{!"test\00", i32 10})"; + auto SettingGuard = TempSettingChange(); + PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; LLVMContext Context; std::unique_ptr M = makeLLVMModule(Context, ModuleString); @@ -1916,6 +1943,9 @@ TEST(IRSimilarityCandidate, CheckRegionsDifferentTypes) { // Check that debug instructions do not impact similarity. They are marked as // invisible. +// FIXME: PreserveInputDbgFormat is set to true because this test contains +// malformed debug info that cannot be converted to the new debug info format; +// this test should be updated later to use valid debug info. TEST(IRSimilarityCandidate, IdenticalWithDebug) { StringRef ModuleString = R"( define i32 @f(i32 %a, i32 %b) { @@ -1938,6 +1968,8 @@ TEST(IRSimilarityCandidate, IdenticalWithDebug) { declare void @llvm.dbg.value(metadata) !0 = distinct !{!"test\00", i32 10} !1 = distinct !{!"test\00", i32 11})"; + auto SettingGuard = TempSettingChange(); + PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; LLVMContext Context; std::unique_ptr M = makeLLVMModule(Context, ModuleString); diff --git a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp index 905928819dda8..cea0e6fd852a3 100644 --- a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp +++ b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp @@ -72,8 +72,6 @@ TEST(BasicBlockDbgInfoTest, InsertAfterSelf) { !11 = !DILocation(line: 1, column: 1, scope: !6) )"); - // Convert the module to "new" form debug-info. - M->convertToNewDbgValues(); // Fetch the entry block. BasicBlock &BB = M->getFunction("f")->getEntryBlock(); @@ -104,8 +102,6 @@ TEST(BasicBlockDbgInfoTest, InsertAfterSelf) { auto Range2 = RetInst->getDbgRecordRange(); EXPECT_EQ(std::distance(Range2.begin(), Range2.end()), 1u); - M->convertFromNewDbgValues(); - UseNewDbgInfoFormat = false; } @@ -140,8 +136,6 @@ TEST(BasicBlockDbgInfoTest, MarkerOperations) { // Fetch the entry block, BasicBlock &BB = M->getFunction("f")->getEntryBlock(); - // Convert the module to "new" form debug-info. - M->convertToNewDbgValues(); EXPECT_EQ(BB.size(), 2u); // Fetch out our two markers, @@ -276,8 +270,6 @@ TEST(BasicBlockDbgInfoTest, HeadBitOperations) { // Test that the movement of debug-data when using moveBefore etc and // insertBefore etc are governed by the "head" bit of iterators. BasicBlock &BB = M->getFunction("f")->getEntryBlock(); - // Convert the module to "new" form debug-info. - M->convertToNewDbgValues(); // Test that the head bit behaves as expected: it should be set when the // code wants the _start_ of the block, but not otherwise. @@ -385,8 +377,6 @@ TEST(BasicBlockDbgInfoTest, InstrDbgAccess) { // Check that DbgVariableRecords can be accessed from Instructions without // digging into the depths of DbgMarkers. BasicBlock &BB = M->getFunction("f")->getEntryBlock(); - // Convert the module to "new" form debug-info. - M->convertToNewDbgValues(); Instruction *BInst = &*BB.begin(); Instruction *CInst = BInst->getNextNode(); @@ -523,7 +513,6 @@ class DbgSpliceTest : public ::testing::Test { void SetUp() override { UseNewDbgInfoFormat = true; M = parseIR(C, SpliceTestIR.c_str()); - M->convertToNewDbgValues(); BBEntry = &M->getFunction("f")->getEntryBlock(); BBExit = BBEntry->getNextNode(); @@ -1163,7 +1152,6 @@ TEST(BasicBlockDbgInfoTest, DbgSpliceTrailing) { BasicBlock &Entry = M->getFunction("f")->getEntryBlock(); BasicBlock &Exit = *Entry.getNextNode(); - M->convertToNewDbgValues(); // Begin by forcing entry block to have dangling DbgVariableRecord. Entry.getTerminator()->eraseFromParent(); @@ -1217,7 +1205,6 @@ TEST(BasicBlockDbgInfoTest, RemoveInstAndReinsert) { )"); BasicBlock &Entry = M->getFunction("f")->getEntryBlock(); - M->convertToNewDbgValues(); // Fetch the relevant instructions from the converted function. Instruction *SubInst = &*Entry.begin(); @@ -1296,7 +1283,6 @@ TEST(BasicBlockDbgInfoTest, RemoveInstAndReinsertForOneDbgVariableRecord) { )"); BasicBlock &Entry = M->getFunction("f")->getEntryBlock(); - M->convertToNewDbgValues(); // Fetch the relevant instructions from the converted function. Instruction *SubInst = &*Entry.begin(); @@ -1380,7 +1366,6 @@ TEST(BasicBlockDbgInfoTest, DbgSpliceToEmpty1) { Function &F = *M->getFunction("f"); BasicBlock &Entry = F.getEntryBlock(); BasicBlock &Exit = *Entry.getNextNode(); - M->convertToNewDbgValues(); // Begin by forcing entry block to have dangling DbgVariableRecord. Entry.getTerminator()->eraseFromParent(); @@ -1450,7 +1435,6 @@ TEST(BasicBlockDbgInfoTest, DbgSpliceToEmpty2) { Function &F = *M->getFunction("f"); BasicBlock &Entry = F.getEntryBlock(); BasicBlock &Exit = *Entry.getNextNode(); - M->convertToNewDbgValues(); // Begin by forcing entry block to have dangling DbgVariableRecord. Entry.getTerminator()->eraseFromParent(); @@ -1520,7 +1504,6 @@ TEST(BasicBlockDbgInfoTest, DbgMoveToEnd) { Function &F = *M->getFunction("f"); BasicBlock &Entry = F.getEntryBlock(); BasicBlock &Exit = *Entry.getNextNode(); - M->convertToNewDbgValues(); // Move the return to the end of the entry block. Instruction *Br = Entry.getTerminator(); diff --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp index d06b979bf4a1c..2e75384c67b06 100644 --- a/llvm/unittests/IR/DebugInfoTest.cpp +++ b/llvm/unittests/IR/DebugInfoTest.cpp @@ -237,6 +237,9 @@ TEST(DbgVariableIntrinsic, EmptyMDIsKillLocation) { // Duplicate of above test, but in DbgVariableRecord representation. TEST(MetadataTest, DeleteInstUsedByDbgVariableRecord) { LLVMContext C; + bool OldDbgValueMode = UseNewDbgInfoFormat; + UseNewDbgInfoFormat = true; + std::unique_ptr M = parseIR(C, R"( define i16 @f(i16 %a) !dbg !6 { %b = add i16 %a, 1, !dbg !11 @@ -262,10 +265,7 @@ TEST(MetadataTest, DeleteInstUsedByDbgVariableRecord) { !11 = !DILocation(line: 1, column: 1, scope: !6) )"); - bool OldDbgValueMode = UseNewDbgInfoFormat; - UseNewDbgInfoFormat = true; Instruction &I = *M->getFunction("f")->getEntryBlock().getFirstNonPHI(); - M->convertToNewDbgValues(); // Find the DbgVariableRecords using %b. SmallVector DVIs; @@ -1044,10 +1044,6 @@ TEST(MetadataTest, ConvertDbgToDbgVariableRecord) { TEST(MetadataTest, DbgVariableRecordConversionRoutines) { 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 @@ -1077,6 +1073,11 @@ TEST(MetadataTest, DbgVariableRecordConversionRoutines) { !11 = !DILocation(line: 1, column: 1, scope: !6) )"); + // For the purpose of this test, set and un-set the command line option + // corresponding to UseNewDbgInfoFormat, but only after parsing, to ensure + // that the IR starts off in the old format. + UseNewDbgInfoFormat = true; + // Check that the conversion routines and utilities between dbg.value // debug-info format and DbgVariableRecords works. Function *F = M->getFunction("f"); diff --git a/llvm/unittests/IR/InstructionsTest.cpp b/llvm/unittests/IR/InstructionsTest.cpp index b47c73f0b329a..6c4debf4dbec8 100644 --- a/llvm/unittests/IR/InstructionsTest.cpp +++ b/llvm/unittests/IR/InstructionsTest.cpp @@ -31,6 +31,8 @@ #include "gtest/gtest.h" #include +extern llvm::cl::opt PreserveInputDbgFormat; + namespace llvm { namespace { @@ -1460,6 +1462,8 @@ TEST(InstructionsTest, GetSplat) { TEST(InstructionsTest, SkipDebug) { LLVMContext C; + cl::boolOrDefault OldDbgFormat = PreserveInputDbgFormat; + PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; std::unique_ptr M = parseIR(C, R"( declare void @llvm.dbg.value(metadata, metadata, metadata) @@ -1495,6 +1499,7 @@ TEST(InstructionsTest, SkipDebug) { // After the terminator, there are no non-debug instructions. EXPECT_EQ(nullptr, Term->getNextNonDebugInstruction()); + PreserveInputDbgFormat = OldDbgFormat; } TEST(InstructionsTest, PhiMightNotBeFPMathOperator) { diff --git a/llvm/unittests/Transforms/Utils/CloningTest.cpp b/llvm/unittests/Transforms/Utils/CloningTest.cpp index 025771f07ce5d..6f4e860d60468 100644 --- a/llvm/unittests/Transforms/Utils/CloningTest.cpp +++ b/llvm/unittests/Transforms/Utils/CloningTest.cpp @@ -844,8 +844,9 @@ TEST(CloneFunction, CloneFunctionWithInlinedSubprograms) { EXPECT_FALSE(verifyModule(*ImplModule, &errs())); // Check that DILexicalBlock of inlined function was not cloned. - auto DbgDeclareI = Func->begin()->begin(); - auto ClonedDbgDeclareI = ClonedFunc->begin()->begin(); + auto DbgDeclareI = Func->begin()->begin()->getDbgRecordRange().begin(); + auto ClonedDbgDeclareI = + ClonedFunc->begin()->begin()->getDbgRecordRange().begin(); const DebugLoc &DbgLoc = DbgDeclareI->getDebugLoc(); const DebugLoc &ClonedDbgLoc = ClonedDbgDeclareI->getDebugLoc(); EXPECT_NE(DbgLoc.get(), ClonedDbgLoc.get()); diff --git a/llvm/unittests/Transforms/Utils/LocalTest.cpp b/llvm/unittests/Transforms/Utils/LocalTest.cpp index d7d0ea2c6a6e7..6c350ebb9ba2e 100644 --- a/llvm/unittests/Transforms/Utils/LocalTest.cpp +++ b/llvm/unittests/Transforms/Utils/LocalTest.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "llvm/Transforms/Utils/Local.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/Analysis/DomTreeUpdater.h" #include "llvm/Analysis/InstructionSimplify.h" #include "llvm/Analysis/PostDominators.h" @@ -26,6 +27,27 @@ using namespace llvm; +extern llvm::cl::opt UseNewDbgInfoFormat; +extern cl::opt PreserveInputDbgFormat; +extern bool WriteNewDbgInfoFormatToBitcode; +extern cl::opt WriteNewDbgInfoFormat; + +// Backup all of the existing settings that may be modified when +// PreserveInputDbgFormat=true, so that when the test is finished we return them +// (and the "preserve" setting) to their original values. +auto TempSettingChange() { + return make_scope_exit( + [OldPreserveInputDbgFormat = PreserveInputDbgFormat.getValue(), + OldUseNewDbgInfoFormat = UseNewDbgInfoFormat.getValue(), + OldWriteNewDbgInfoFormatToBitcode = WriteNewDbgInfoFormatToBitcode, + OldWriteNewDbgInfoFormat = WriteNewDbgInfoFormat.getValue()] { + PreserveInputDbgFormat = OldPreserveInputDbgFormat; + UseNewDbgInfoFormat = OldUseNewDbgInfoFormat; + WriteNewDbgInfoFormatToBitcode = OldWriteNewDbgInfoFormatToBitcode; + WriteNewDbgInfoFormat = OldWriteNewDbgInfoFormat; + }); +} + TEST(Local, RecursivelyDeleteDeadPHINodes) { LLVMContext C; @@ -116,6 +138,11 @@ static std::unique_ptr parseIR(LLVMContext &C, const char *IR) { TEST(Local, ReplaceDbgDeclare) { LLVMContext C; + // FIXME: PreserveInputDbgFormat is set to true because this test has + // been written to expect debug intrinsics rather than debug records; use the + // intrinsic format until we update the test checks. + auto SettingGuard = TempSettingChange(); + PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; // Original C source to get debug info for a local variable: // void f() { int x; } @@ -493,6 +520,14 @@ struct SalvageDebugInfoTest : ::testing::Test { Function *F = nullptr; void SetUp() override { + // FIXME: PreserveInputDbgFormat is set to true because this test has + // been written to expect debug intrinsics rather than debug records; use + // the intrinsic format until we update the test checks. Note that the + // temporary setting of this flag only needs to cover the parsing step, not + // the test body itself. + auto SettingGuard = TempSettingChange(); + PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; + M = parseIR(C, R"( define void @f() !dbg !8 { @@ -591,6 +626,11 @@ TEST_F(SalvageDebugInfoTest, RecursiveBlockSimplification) { TEST(Local, wouldInstructionBeTriviallyDead) { LLVMContext Ctx; + // FIXME: PreserveInputDbgFormat is set to true because this test has + // been written to expect debug intrinsics rather than debug records; use the + // intrinsic format until we update the test checks. + auto SettingGuard = TempSettingChange(); + PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; std::unique_ptr M = parseIR(Ctx, R"( define dso_local void @fun() local_unnamed_addr #0 !dbg !9 { @@ -680,6 +720,11 @@ TEST(Local, ChangeToUnreachable) { TEST(Local, FindDbgUsers) { LLVMContext Ctx; + // FIXME: PreserveInputDbgFormat is set to true because this test has + // been written to expect debug intrinsics rather than debug records; use the + // intrinsic format until we update the test checks. + auto SettingGuard = TempSettingChange(); + PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; std::unique_ptr M = parseIR(Ctx, R"( define dso_local void @fun(ptr %a) #0 !dbg !11 { @@ -736,6 +781,11 @@ TEST(Local, ReplaceAllDbgUsesWith) { using namespace llvm::dwarf; LLVMContext Ctx; + // FIXME: PreserveInputDbgFormat is set to true because this test has + // been written to expect debug intrinsics rather than debug records; use the + // intrinsic format until we update the test checks. + auto SettingGuard = TempSettingChange(); + PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; // Note: The datalayout simulates Darwin/x86_64. std::unique_ptr M = parseIR(Ctx, @@ -1284,6 +1334,11 @@ TEST(Local, ExpressionForConstant) { TEST(Local, ReplaceDbgVariableRecord) { LLVMContext C; + // FIXME: PreserveInputDbgFormat is set to true because this test has + // been written to expect debug intrinsics rather than debug records; use the + // intrinsic format until we update the test checks. + auto SettingGuard = TempSettingChange(); + PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; // Test that RAUW also replaces the operands of DbgVariableRecord objects, // i.e. non-instruction stored debugging information. From 651925c7835ec2a241b98c19898b22d2118ddc0a Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Tue, 30 Apr 2024 09:51:57 +0100 Subject: [PATCH 2/5] Address review comments --- llvm/docs/ReleaseNotes.rst | 6 ++++++ llvm/unittests/IR/DebugInfoTest.cpp | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index 580dc512d9690..592c1dae48e98 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -161,6 +161,12 @@ Changes to the Metadata Info Changes to the Debug Info --------------------------------- +* LLVM has switched from using debug intrinsics internally to using debug + records. This should happen transparently when using the DIBuilder to + construct debug variable information, but will require changes for any code + that interacts with debug intrinsics directly; for more information, see + https://llvm.org/docs/RemoveDIsDebugInfo.html. + Changes to the LLVM tools --------------------------------- * llvm-nm and llvm-objdump can now print symbol information from linked diff --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp index 2e75384c67b06..ef6aa7fc10df2 100644 --- a/llvm/unittests/IR/DebugInfoTest.cpp +++ b/llvm/unittests/IR/DebugInfoTest.cpp @@ -1044,6 +1044,9 @@ TEST(MetadataTest, ConvertDbgToDbgVariableRecord) { TEST(MetadataTest, DbgVariableRecordConversionRoutines) { LLVMContext C; + bool OldDbgValueMode = UseNewDbgInfoFormat; + UseNewDbgInfoFormat = false; + 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 @@ -1182,7 +1185,7 @@ TEST(MetadataTest, DbgVariableRecordConversionRoutines) { EXPECT_EQ(DVI2->getVariable(), DLV2); EXPECT_EQ(DVI2->getExpression(), Expr2); - UseNewDbgInfoFormat = false; + UseNewDbgInfoFormat = OldDbgValueMode; } } // end namespace From c82cf46642dab13636cd9ede8890815ebd4ef5e4 Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Tue, 30 Apr 2024 09:58:05 +0100 Subject: [PATCH 3/5] Small change to docs link style --- llvm/docs/ReleaseNotes.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index 592c1dae48e98..7c32e65c3f4db 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -164,8 +164,8 @@ Changes to the Debug Info * LLVM has switched from using debug intrinsics internally to using debug records. This should happen transparently when using the DIBuilder to construct debug variable information, but will require changes for any code - that interacts with debug intrinsics directly; for more information, see - https://llvm.org/docs/RemoveDIsDebugInfo.html. + that interacts with debug intrinsics directly; for more information, see the + `migration docs `_. Changes to the LLVM tools --------------------------------- From 9819b44ef1c549f71ab67743be8bf5d7972f3a40 Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Tue, 30 Apr 2024 12:24:32 +0100 Subject: [PATCH 4/5] Address remaining outstanding comments --- .../Analysis/IRSimilarityIdentifierTest.cpp | 6 +++--- llvm/unittests/Transforms/Utils/LocalTest.cpp | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp index 69ea590945668..0a08ca3cb99db 100644 --- a/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp +++ b/llvm/unittests/Analysis/IRSimilarityIdentifierTest.cpp @@ -31,7 +31,7 @@ extern cl::opt WriteNewDbgInfoFormat; // Backup all of the existing settings that may be modified when // PreserveInputDbgFormat=true, so that when the test is finished we return them // (and the "preserve" setting) to their original values. -auto TempSettingChange() { +static auto SaveDbgInfoFormat() { return make_scope_exit( [OldPreserveInputDbgFormat = PreserveInputDbgFormat.getValue(), OldUseNewDbgInfoFormat = UseNewDbgInfoFormat.getValue(), @@ -1345,7 +1345,7 @@ TEST(IRInstructionMapper, DebugInfoInvisible) { declare void @llvm.dbg.value(metadata) !0 = distinct !{!"test\00", i32 10})"; - auto SettingGuard = TempSettingChange(); + auto SettingGuard = SaveDbgInfoFormat(); PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; LLVMContext Context; std::unique_ptr M = makeLLVMModule(Context, ModuleString); @@ -1968,7 +1968,7 @@ TEST(IRSimilarityCandidate, IdenticalWithDebug) { declare void @llvm.dbg.value(metadata) !0 = distinct !{!"test\00", i32 10} !1 = distinct !{!"test\00", i32 11})"; - auto SettingGuard = TempSettingChange(); + auto SettingGuard = SaveDbgInfoFormat(); PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; LLVMContext Context; std::unique_ptr M = makeLLVMModule(Context, ModuleString); diff --git a/llvm/unittests/Transforms/Utils/LocalTest.cpp b/llvm/unittests/Transforms/Utils/LocalTest.cpp index 6c350ebb9ba2e..7bb65d0aee7c3 100644 --- a/llvm/unittests/Transforms/Utils/LocalTest.cpp +++ b/llvm/unittests/Transforms/Utils/LocalTest.cpp @@ -35,7 +35,7 @@ extern cl::opt WriteNewDbgInfoFormat; // Backup all of the existing settings that may be modified when // PreserveInputDbgFormat=true, so that when the test is finished we return them // (and the "preserve" setting) to their original values. -auto TempSettingChange() { +static auto SaveDbgInfoFormat() { return make_scope_exit( [OldPreserveInputDbgFormat = PreserveInputDbgFormat.getValue(), OldUseNewDbgInfoFormat = UseNewDbgInfoFormat.getValue(), @@ -141,7 +141,7 @@ TEST(Local, ReplaceDbgDeclare) { // FIXME: PreserveInputDbgFormat is set to true because this test has // been written to expect debug intrinsics rather than debug records; use the // intrinsic format until we update the test checks. - auto SettingGuard = TempSettingChange(); + auto SettingGuard = SaveDbgInfoFormat(); PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; // Original C source to get debug info for a local variable: @@ -525,7 +525,7 @@ struct SalvageDebugInfoTest : ::testing::Test { // the intrinsic format until we update the test checks. Note that the // temporary setting of this flag only needs to cover the parsing step, not // the test body itself. - auto SettingGuard = TempSettingChange(); + auto SettingGuard = SaveDbgInfoFormat(); PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; M = parseIR(C, @@ -627,9 +627,10 @@ TEST_F(SalvageDebugInfoTest, RecursiveBlockSimplification) { TEST(Local, wouldInstructionBeTriviallyDead) { LLVMContext Ctx; // FIXME: PreserveInputDbgFormat is set to true because this test has - // been written to expect debug intrinsics rather than debug records; use the - // intrinsic format until we update the test checks. - auto SettingGuard = TempSettingChange(); + // been written to expect debug intrinsics rather than debug records. + // TODO: This test doesn't have a DbgRecord equivalent form so delete + // it when debug intrinsics are removed. + auto SettingGuard = SaveDbgInfoFormat(); PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; std::unique_ptr M = parseIR(Ctx, R"( @@ -723,7 +724,7 @@ TEST(Local, FindDbgUsers) { // FIXME: PreserveInputDbgFormat is set to true because this test has // been written to expect debug intrinsics rather than debug records; use the // intrinsic format until we update the test checks. - auto SettingGuard = TempSettingChange(); + auto SettingGuard = SaveDbgInfoFormat(); PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; std::unique_ptr M = parseIR(Ctx, R"( @@ -784,7 +785,7 @@ TEST(Local, ReplaceAllDbgUsesWith) { // FIXME: PreserveInputDbgFormat is set to true because this test has // been written to expect debug intrinsics rather than debug records; use the // intrinsic format until we update the test checks. - auto SettingGuard = TempSettingChange(); + auto SettingGuard = SaveDbgInfoFormat(); PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; // Note: The datalayout simulates Darwin/x86_64. @@ -1337,7 +1338,7 @@ TEST(Local, ReplaceDbgVariableRecord) { // FIXME: PreserveInputDbgFormat is set to true because this test has // been written to expect debug intrinsics rather than debug records; use the // intrinsic format until we update the test checks. - auto SettingGuard = TempSettingChange(); + auto SettingGuard = SaveDbgInfoFormat(); PreserveInputDbgFormat = cl::boolOrDefault::BOU_TRUE; // Test that RAUW also replaces the operands of DbgVariableRecord objects, From 1e1705194b541995d285dd8e8e7e93480b73b710 Mon Sep 17 00:00:00 2001 From: Stephen Tozer Date: Tue, 30 Apr 2024 13:55:27 +0100 Subject: [PATCH 5/5] Update release notes --- llvm/docs/ReleaseNotes.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index 7c32e65c3f4db..891d4b4453ebf 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -162,10 +162,11 @@ Changes to the Debug Info --------------------------------- * LLVM has switched from using debug intrinsics internally to using debug - records. This should happen transparently when using the DIBuilder to - construct debug variable information, but will require changes for any code - that interacts with debug intrinsics directly; for more information, see the - `migration docs `_. + records by default. This should happen transparently when using the DIBuilder + to construct debug variable information, but will require changes for any code + that interacts with debug intrinsics directly. Debug intrinsics will only be + supported on a best-effort basis from here onwards; for more information, see + the `migration docs `_. Changes to the LLVM tools ---------------------------------