From f3869a5c32b78bc70e5051efbc2594f772b0176e Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Wed, 26 May 2021 18:23:02 -0700 Subject: [PATCH] Support stripping indirectly referenced DILocations from !llvm.loop metadata in stripDebugInfo(). This patch fixes an oversight in https://reviews.llvm.org/D96181 and also takes into account loop metadata pointing to other MDNodes that point into the debug info. rdar://78487175 Differential Revision: https://reviews.llvm.org/D103220 --- llvm/include/llvm/IR/DebugInfo.h | 6 +- llvm/lib/IR/DebugInfo.cpp | 82 ++++++++++++++----- llvm/lib/Transforms/Utils/CodeExtractor.cpp | 9 +- llvm/lib/Transforms/Utils/InlineFunction.cpp | 8 +- .../Verifier/llvm.loop-cu-strip-indirect.ll | 25 ++++++ 5 files changed, 98 insertions(+), 32 deletions(-) create mode 100644 llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h index 05c82939cb3d..eba422a9fde6 100644 --- a/llvm/include/llvm/IR/DebugInfo.h +++ b/llvm/include/llvm/IR/DebugInfo.h @@ -70,11 +70,11 @@ bool stripDebugInfo(Function &F); bool stripNonLineTableDebugInfo(Module &M); /// Update the debug locations contained within the MD_loop metadata attached -/// to the instruction \p I, if one exists. \p Updater is applied to each debug -/// location in the MD_loop metadata: the returned value is included in the +/// to the instruction \p I, if one exists. \p Updater is applied to Metadata +/// operand in the MD_loop metadata: the returned value is included in the /// updated loop metadata node if it is non-null. void updateLoopMetadataDebugLocations( - Instruction &I, function_ref Updater); + Instruction &I, function_ref Updater); /// Return Debug Info Metadata Version by checking module flags. unsigned getDebugMetadataVersionFromModule(const Module &M); diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp index b8579c2d8457..331bf8a1ecc8 100644 --- a/llvm/lib/IR/DebugInfo.cpp +++ b/llvm/lib/IR/DebugInfo.cpp @@ -348,8 +348,7 @@ bool DebugInfoFinder::addScope(DIScope *Scope) { } static MDNode *updateLoopMetadataDebugLocationsImpl( - MDNode *OrigLoopID, - function_ref Updater) { + MDNode *OrigLoopID, function_ref Updater) { assert(OrigLoopID && OrigLoopID->getNumOperands() > 0 && "Loop ID needs at least one operand"); assert(OrigLoopID && OrigLoopID->getOperand(0).get() == OrigLoopID && @@ -360,11 +359,10 @@ static MDNode *updateLoopMetadataDebugLocationsImpl( for (unsigned i = 1; i < OrigLoopID->getNumOperands(); ++i) { Metadata *MD = OrigLoopID->getOperand(i); - if (DILocation *DL = dyn_cast(MD)) { - if (DILocation *NewDL = Updater(*DL)) - MDs.push_back(NewDL); - } else - MDs.push_back(MD); + if (!MD) + MDs.push_back(nullptr); + else if (Metadata *NewMD = Updater(MD)) + MDs.push_back(NewMD); } MDNode *NewLoopID = MDNode::getDistinct(OrigLoopID->getContext(), MDs); @@ -374,7 +372,7 @@ static MDNode *updateLoopMetadataDebugLocationsImpl( } void llvm::updateLoopMetadataDebugLocations( - Instruction &I, function_ref Updater) { + Instruction &I, function_ref Updater) { MDNode *OrigLoopID = I.getMetadata(LLVMContext::MD_loop); if (!OrigLoopID) return; @@ -382,26 +380,64 @@ void llvm::updateLoopMetadataDebugLocations( I.setMetadata(LLVMContext::MD_loop, NewLoopID); } +/// Return true if a node is a DILocation or if a DILocation is +/// indirectly referenced by one of the node's children. +static bool isDILocationReachable(SmallPtrSetImpl &Visited, + SmallPtrSetImpl &Reachable, + Metadata *MD) { + MDNode *N = dyn_cast_or_null(MD); + if (!N) + return false; + if (Reachable.count(N) || isa(N)) + return true; + if (!Visited.insert(N).second) + return false; + for (auto &OpIt : N->operands()) { + Metadata *Op = OpIt.get(); + if (isDILocationReachable(Visited, Reachable, Op)) { + Reachable.insert(N); + return true; + } + } + return false; +} + static MDNode *stripDebugLocFromLoopID(MDNode *N) { assert(!N->operands().empty() && "Missing self reference?"); + SmallPtrSet Visited, DILocationReachable; + // If we already visited N, there is nothing to do. + if (!Visited.insert(N).second) + return N; - // if there is no debug location, we do not have to rewrite this MDNode. - if (std::none_of(N->op_begin() + 1, N->op_end(), [](const MDOperand &Op) { - return isa(Op.get()); - })) + // If there is no debug location, we do not have to rewrite this + // MDNode. This loop also initializes DILocationReachable, later + // needed by updateLoopMetadataDebugLocationsImpl; the use of + // count_if avoids an early exit. + if (!std::count_if(N->op_begin() + 1, N->op_end(), + [&Visited, &DILocationReachable](const MDOperand &Op) { + return isa(Op.get()) || + isDILocationReachable( + Visited, DILocationReachable, Op.get()); + })) return N; // If there is only the debug location without any actual loop metadata, we // can remove the metadata. - if (std::none_of(N->op_begin() + 1, N->op_end(), [](const MDOperand &Op) { - return !isa(Op.get()); - })) + if (std::all_of( + N->op_begin() + 1, N->op_end(), + [&Visited, &DILocationReachable](const MDOperand &Op) { + return isa(Op.get()) || + isDILocationReachable(Visited, DILocationReachable, + Op.get()); + })) return nullptr; - auto dropDebugLoc = [](const DILocation &) -> DILocation * { - return nullptr; - }; - return updateLoopMetadataDebugLocationsImpl(N, dropDebugLoc); + return updateLoopMetadataDebugLocationsImpl( + N, [&DILocationReachable](Metadata *MD) -> Metadata * { + if (isa(MD) || DILocationReachable.count(MD)) + return nullptr; + return MD; + }); } bool llvm::stripDebugInfo(Function &F) { @@ -411,7 +447,7 @@ bool llvm::stripDebugInfo(Function &F) { F.setSubprogram(nullptr); } - DenseMap LoopIDsMap; + DenseMap LoopIDsMap; for (BasicBlock &BB : F) { for (auto II = BB.begin(), End = BB.end(); II != End;) { Instruction &I = *II++; // We may delete the instruction, increment now. @@ -740,8 +776,10 @@ bool llvm::stripNonLineTableDebugInfo(Module &M) { I.setDebugLoc(remapDebugLoc(I.getDebugLoc())); // Remap DILocations in llvm.loop attachments. - updateLoopMetadataDebugLocations(I, [&](const DILocation &Loc) { - return remapDebugLoc(&Loc).get(); + updateLoopMetadataDebugLocations(I, [&](Metadata *MD) -> Metadata * { + if (auto *Loc = dyn_cast_or_null(MD)) + return remapDebugLoc(Loc).get(); + return MD; }); // Strip heapallocsite attachments, they point into the DIType system. diff --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp index 189e87e180cb..3d631651fa7d 100644 --- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp +++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp @@ -1552,10 +1552,11 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc, I.setDebugLoc(DILocation::get(Ctx, DL.getLine(), DL.getCol(), NewSP)); // Loop info metadata may contain line locations. Fix them up. - auto updateLoopInfoLoc = [&Ctx, - NewSP](const DILocation &Loc) -> DILocation * { - return DILocation::get(Ctx, Loc.getLine(), Loc.getColumn(), NewSP, - nullptr); + auto updateLoopInfoLoc = [&Ctx, NewSP](Metadata *MD) -> Metadata * { + if (auto *Loc = dyn_cast_or_null(MD)) + return DILocation::get(Ctx, Loc->getLine(), Loc->getColumn(), NewSP, + nullptr); + return MD; }; updateLoopMetadataDebugLocations(I, updateLoopInfoLoc); } diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp index d848b1fe193c..bd9bd77e7ccc 100644 --- a/llvm/lib/Transforms/Utils/InlineFunction.cpp +++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp @@ -1519,9 +1519,11 @@ static void fixupLineNumbers(Function *Fn, Function::iterator FI, BI != BE; ++BI) { // Loop metadata needs to be updated so that the start and end locs // reference inlined-at locations. - auto updateLoopInfoLoc = [&Ctx, &InlinedAtNode, &IANodes]( - const DILocation &Loc) -> DILocation * { - return inlineDebugLoc(&Loc, InlinedAtNode, Ctx, IANodes).get(); + auto updateLoopInfoLoc = [&Ctx, &InlinedAtNode, + &IANodes](Metadata *MD) -> Metadata * { + if (auto *Loc = dyn_cast_or_null(MD)) + return inlineDebugLoc(Loc, InlinedAtNode, Ctx, IANodes).get(); + return MD; }; updateLoopMetadataDebugLocations(*BI, updateLoopInfoLoc); diff --git a/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll b/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll new file mode 100644 index 000000000000..61d2d9a4470f --- /dev/null +++ b/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll @@ -0,0 +1,25 @@ +; RUN: llvm-as -disable-output < %s -o /dev/null 2>&1 | FileCheck %s +; RUN: llvm-as < %s -o - | llvm-dis - | FileCheck %s --check-prefix=CHECK-STRIP +; CHECK: DICompileUnit not listed in llvm.dbg.cu +; CHECK: ignoring invalid debug info in +; CHECK-NOT: DICompileUnit not listed in llvm.dbg.cu +declare hidden void @g() local_unnamed_addr #1 align 2 +define hidden void @f() { + tail call void @g() #2, !llvm.loop !5 + ret void +} +!llvm.module.flags = !{!0, !1} +!0 = !{i32 2, !"Dwarf Version", i32 4} +!1 = !{i32 2, !"Debug Info Version", i32 3} +!5 = distinct !{!5, !14, !15, !"fake loop metadata"} +; CHECK-STRIP: ![[MD:.*]] = distinct !{![[MD]], !{{[0-9]+}}, !"fake loop metadata"} +!7 = distinct !DISubprogram(name: "f", scope: !8, file: !8, line: 1324, type: !9, scopeLine: 1324, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !11) +!8 = !DIFile(filename: "/", directory: "f.cpp") +!9 = !DISubroutineType(types: !10) +!10 = !{} +!11 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !8) +!14 = !{!"metadata 1", i1 true} +!15 = !{!"metadata 1", !16} +!16 = distinct !{!16, !17} +!17 = !DILocation(line: 105, column: 3, scope: !18) +!18 = distinct !DILexicalBlock(scope: !7, file: !8, line: 105, column: 3)