[llvm] a253a0b - Avoid removing useful loop metadata when stripping debug info
Adrian Prantl via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 25 10:05:36 PST 2023
Author: She Dongrui
Date: 2023-01-25T10:05:27-08:00
New Revision: a253a0bbe5ece7795633f8abeba7786e98a3e875
URL: https://github.com/llvm/llvm-project/commit/a253a0bbe5ece7795633f8abeba7786e98a3e875
DIFF: https://github.com/llvm/llvm-project/commit/a253a0bbe5ece7795633f8abeba7786e98a3e875.diff
LOG: Avoid removing useful loop metadata when stripping debug info
The stripDebugLocFromLoopID() may mistakenly remove useful metadata nodes
when they are operands of a child node, which also has DILocation operands.
It can be reproduced by the output of clang for code similar to the following:
for(int i = 0; i < n; i++)
x[i] = 10;
-strip-debug removes the child node of llvm.loop.vectorize.followup_all,
which contains llvm.loop.isvectorized and llvm.loop.unroll.count.
This patch fixes by checking all children nodes and only remove a metadata
node if all its children are DILocation.
Differential Revision: https://reviews.llvm.org/D141909
Added:
llvm/test/Verifier/llvm.loop-cu-strip-followup.ll
Modified:
llvm/lib/IR/DebugInfo.cpp
llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll
Removed:
################################################################################
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 9198179674bda..be18d2b5eda03 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -410,16 +410,80 @@ static bool isDILocationReachable(SmallPtrSetImpl<Metadata *> &Visited,
for (auto &OpIt : N->operands()) {
Metadata *Op = OpIt.get();
if (isDILocationReachable(Visited, Reachable, Op)) {
+ // Don't return just yet as we want to visit all MD's children to
+ // initialize DILocationReachable in stripDebugLocFromLoopID
Reachable.insert(N);
- return true;
}
}
- return false;
+ return Reachable.count(N);
+}
+
+static bool isAllDILocation(SmallPtrSetImpl<Metadata *> &Visited,
+ SmallPtrSetImpl<Metadata *> &AllDILocation,
+ const SmallPtrSetImpl<Metadata *> &DIReachable,
+ Metadata *MD) {
+ MDNode *N = dyn_cast_or_null<MDNode>(MD);
+ if (!N)
+ return false;
+ if (isa<DILocation>(N) || AllDILocation.count(N))
+ return true;
+ if (!DIReachable.count(N))
+ return false;
+ if (!Visited.insert(N).second)
+ return false;
+ for (auto &OpIt : N->operands()) {
+ Metadata *Op = OpIt.get();
+ if (Op == MD)
+ continue;
+ if (!isAllDILocation(Visited, AllDILocation, DIReachable, Op)) {
+ return false;
+ }
+ }
+ AllDILocation.insert(N);
+ return true;
+}
+
+static Metadata *
+stripLoopMDLoc(const SmallPtrSetImpl<Metadata *> &AllDILocation,
+ const SmallPtrSetImpl<Metadata *> &DIReachable, Metadata *MD) {
+ if (isa<DILocation>(MD) || AllDILocation.count(MD))
+ return nullptr;
+
+ if (!DIReachable.count(MD))
+ return MD;
+
+ MDNode *N = dyn_cast_or_null<MDNode>(MD);
+ if (!N)
+ return MD;
+
+ SmallVector<Metadata *, 4> Args;
+ bool HasSelfRef = false;
+ for (unsigned i = 0; i < N->getNumOperands(); ++i) {
+ Metadata *A = N->getOperand(i);
+ if (!A) {
+ Args.push_back(nullptr);
+ } else if (A == MD) {
+ assert(i == 0 && "expected i==0 for self-reference");
+ HasSelfRef = true;
+ Args.push_back(nullptr);
+ } else if (Metadata *NewArg =
+ stripLoopMDLoc(AllDILocation, DIReachable, A)) {
+ Args.push_back(NewArg);
+ }
+ }
+ if (Args.empty() || (HasSelfRef && Args.size() == 1))
+ return nullptr;
+
+ MDNode *NewMD = N->isDistinct() ? MDNode::getDistinct(N->getContext(), Args)
+ : MDNode::get(N->getContext(), Args);
+ if (HasSelfRef)
+ NewMD->replaceOperandWith(0, NewMD);
+ return NewMD;
}
static MDNode *stripDebugLocFromLoopID(MDNode *N) {
assert(!N->operands().empty() && "Missing self reference?");
- SmallPtrSet<Metadata *, 8> Visited, DILocationReachable;
+ SmallPtrSet<Metadata *, 8> Visited, DILocationReachable, AllDILocation;
// If we already visited N, there is nothing to do.
if (!Visited.insert(N).second)
return N;
@@ -428,27 +492,27 @@ static MDNode *stripDebugLocFromLoopID(MDNode *N) {
// 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(),
+ if (!llvm::count_if(llvm::drop_begin(N->operands()),
[&Visited, &DILocationReachable](const MDOperand &Op) {
return isDILocationReachable(
Visited, DILocationReachable, Op.get());
}))
return N;
+ Visited.clear();
// If there is only the debug location without any actual loop metadata, we
// can remove the metadata.
if (llvm::all_of(llvm::drop_begin(N->operands()),
- [&Visited, &DILocationReachable](const MDOperand &Op) {
- return isDILocationReachable(Visited, DILocationReachable,
- Op.get());
+ [&Visited, &AllDILocation,
+ &DILocationReachable](const MDOperand &Op) {
+ return isAllDILocation(Visited, AllDILocation,
+ DILocationReachable, Op.get());
}))
return nullptr;
return updateLoopMetadataDebugLocationsImpl(
- N, [&DILocationReachable](Metadata *MD) -> Metadata * {
- if (isa<DILocation>(MD) || DILocationReachable.count(MD))
- return nullptr;
- return MD;
+ N, [&AllDILocation, &DILocationReachable](Metadata *MD) -> Metadata * {
+ return stripLoopMDLoc(AllDILocation, DILocationReachable, MD);
});
}
diff --git a/llvm/test/Verifier/llvm.loop-cu-strip-followup.ll b/llvm/test/Verifier/llvm.loop-cu-strip-followup.ll
new file mode 100644
index 0000000000000..25a09b15ca652
--- /dev/null
+++ b/llvm/test/Verifier/llvm.loop-cu-strip-followup.ll
@@ -0,0 +1,47 @@
+; RUN: llvm-as < %s -o - | llvm-dis - | FileCheck %s
+
+; The loop metadata in this test is similar to the output from clang for
+; code like the this:
+;
+; #pragma clang loop unroll_count(4)
+; #pragma clang loop vectorize(assume_safety)
+; for(int i = 0; i < n; i++)
+; ...
+;
+; strip debug should not remove the useful metadata in the followup part.
+
+declare hidden void @g() local_unnamed_addr align 2
+
+define hidden void @f() {
+; CHECK: tail call {{.*}} !llvm.loop [[LID:![0-9]+]]
+ tail call void @g(), !llvm.loop !2
+ ret void
+}
+
+!llvm.module.flags = !{!0, !1}
+
+; CHECK-NOT: DILocation
+; CHECK: [[LID]] = distinct !{[[LID]], [[VE:![0-9]+]], [[VF:![0-9]+]]}
+; CHECK-NOT: DILocation
+; CHECK: [[VE]] = !{!"llvm.loop.vectorize.enable", i1 true}
+; CHECK: [[VF]] = !{!"llvm.loop.vectorize.followup_all", [[FU:![0-9]+]]}
+; CHECK: [[FU]] = distinct !{[[FU]], [[VD:![0-9]+]], [[UC:![0-9]+]]}
+; CHECK: [[VD]] = !{!"llvm.loop.isvectorized"}
+; CHECK: [[UC]] = !{!"llvm.loop.unroll.count", i32 4}
+
+!0 = !{i32 2, !"Dwarf Version", i32 4}
+!1 = !{i32 2, !"Debug Info Version", i32 3}
+!2 = distinct !{!2, !3, !10, !11, !12}
+!3 = !DILocation(line: 5, column: 12, scope: !4)
+!4 = distinct !DILexicalBlock(scope: !6, file: !5, line: 105, column: 3)
+!5 = !DIFile(filename: "/", directory: "f.cpp")
+!6 = distinct !DISubprogram(name: "f", scope: !5, file: !5, line: 1324, type: !7, scopeLine: 1324, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !9)
+!7 = !DISubroutineType(types: !8)
+!8 = !{}
+!9 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !5, isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
+!10 = !DILocation(line: 6, column: 1, scope: !4)
+!11 = !{!"llvm.loop.vectorize.enable", i1 true}
+!12 = !{!"llvm.loop.vectorize.followup_all", !13}
+!13 = distinct !{!13, !3, !10, !14, !15}
+!14 = !{!"llvm.loop.isvectorized"}
+!15 = !{!"llvm.loop.unroll.count", i32 4}
diff --git a/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll b/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll
index 61d2d9a4470f3..d724cf7de97e0 100644
--- a/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll
+++ b/llvm/test/Verifier/llvm.loop-cu-strip-indirect.ll
@@ -12,7 +12,8 @@ define hidden void @f() {
!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"}
+; CHECK-STRIP: ![[MD:.*]] = distinct !{![[MD]], !{{[0-9]+}}, !{{[0-9]+}}, !"fake loop metadata"}
+; CHECK-STRIP-NOT: DILocation
!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)
More information about the llvm-commits
mailing list