[llvm] [DebugInfo] Handle followup loop metadata in updateLoopMetadataDebugLocations (PR #157557)
Björn Pettersson via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 25 04:02:42 PDT 2025
https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/157557
>From 7ef307b50bba4ec2f83e5ef508012f76a976a125 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Mon, 8 Sep 2025 14:39:27 +0200
Subject: [PATCH 1/6] [DebugInfo] Pre-commit test case for issue #141568. NFC
Adding test case showing that we do not update DILocations in
followup loop metadata when inlining.
---
.../Inline/dilocation-loop-metadata-update.ll | 81 +++++++++++++++++++
1 file changed, 81 insertions(+)
create mode 100644 llvm/test/Transforms/Inline/dilocation-loop-metadata-update.ll
diff --git a/llvm/test/Transforms/Inline/dilocation-loop-metadata-update.ll b/llvm/test/Transforms/Inline/dilocation-loop-metadata-update.ll
new file mode 100644
index 0000000000000..54a54a3986cd1
--- /dev/null
+++ b/llvm/test/Transforms/Inline/dilocation-loop-metadata-update.ll
@@ -0,0 +1,81 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=inline -S | FileCheck %s
+
+; When inlining we need to update DILocation recursively for the followup
+; metadata when updating llvm.loop metadata.
+
+define void @a() !dbg !3 {
+; CHECK-LABEL: define void @a(
+; CHECK-SAME: ) !dbg [[DBG3:![0-9]+]] {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: br label %[[FOR_BODY:.*]]
+; CHECK: [[FOR_BODY]]:
+; CHECK-NEXT: br label %[[FOR_BODY]], !llvm.loop [[LOOP6:![0-9]+]]
+;
+entry:
+ br label %for.body
+
+for.body: ; preds = %for.body, %entry
+ br label %for.body, !llvm.loop !6
+}
+
+define void @f() !dbg !17 {
+; CHECK-LABEL: define void @f(
+; CHECK-SAME: ) !dbg [[DBG17:![0-9]+]] {
+; CHECK-NEXT: [[ENTRY:.*:]]
+; CHECK-NEXT: br label %[[FOR_BODY_I:.*]]
+; CHECK: [[FOR_BODY_I]]:
+; CHECK-NEXT: br label %[[FOR_BODY_I]], !llvm.loop [[LOOP18:![0-9]+]]
+; CHECK: [[A_EXIT:.*:]]
+; CHECK-NEXT: ret void
+;
+entry:
+ call void @a(), !dbg !18
+ ret void
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: NoDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "foo.c", directory: "/")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = distinct !DISubprogram(name: "a", scope: !1, file: !1, line: 3, type: !4, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!4 = !DISubroutineType(types: !5)
+!5 = !{}
+!6 = distinct !{!6, !7, !8, !9, !10, !11}
+!7 = !DILocation(line: 6, column: 3, scope: !3)
+!8 = !DILocation(line: 7, column: 22, scope: !3)
+!9 = !{!"llvm.loop.mustprogress"}
+!10 = !{!"llvm.loop.distribute.enable", i1 true}
+!11 = !{!"llvm.loop.distribute.followup_all", !7, !8, !9, !12, !13, !14}
+!12 = !{!"llvm.loop.vectorize.width", i32 8}
+!13 = !{!"llvm.loop.vectorize.enable", i1 true}
+!14 = !{!"llvm.loop.vectorize.followup_all", !7, !8, !9, !15, !16}
+!15 = !{!"llvm.loop.isvectorized"}
+!16 = !{!"llvm.loop.unroll.count", i32 1}
+!17 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 9, type: !4, scopeLine: 9, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!18 = !DILocation(line: 9, column: 12, scope: !17)
+;.
+; CHECK: [[META0:![0-9]+]] = distinct !DICompileUnit(language: DW_LANG_C11, file: [[META1:![0-9]+]], producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: NoDebug, splitDebugInlining: false, nameTableKind: None)
+; CHECK: [[META1]] = !DIFile(filename: "{{.*}}foo.c", directory: {{.*}})
+; CHECK: [[DBG3]] = distinct !DISubprogram(name: "a", scope: [[META1]], file: [[META1]], line: 3, type: [[META4:![0-9]+]], scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: [[META0]])
+; CHECK: [[META4]] = !DISubroutineType(types: [[META5:![0-9]+]])
+; CHECK: [[META5]] = !{}
+; CHECK: [[LOOP6]] = distinct !{[[LOOP6]], [[META7:![0-9]+]], [[META8:![0-9]+]], [[META9:![0-9]+]], [[META10:![0-9]+]], [[META11:![0-9]+]]}
+; CHECK: [[META7]] = !DILocation(line: 6, column: 3, scope: [[DBG3]])
+; CHECK: [[META8]] = !DILocation(line: 7, column: 22, scope: [[DBG3]])
+; CHECK: [[META9]] = !{!"llvm.loop.mustprogress"}
+; CHECK: [[META10]] = !{!"llvm.loop.distribute.enable", i1 true}
+; CHECK: [[META11]] = !{!"llvm.loop.distribute.followup_all", [[META7]], [[META8]], [[META9]], [[META12:![0-9]+]], [[META13:![0-9]+]], [[META14:![0-9]+]]}
+; CHECK: [[META12]] = !{!"llvm.loop.vectorize.width", i32 8}
+; CHECK: [[META13]] = !{!"llvm.loop.vectorize.enable", i1 true}
+; CHECK: [[META14]] = !{!"llvm.loop.vectorize.followup_all", [[META7]], [[META8]], [[META9]], [[META15:![0-9]+]], [[META16:![0-9]+]]}
+; CHECK: [[META15]] = !{!"llvm.loop.isvectorized"}
+; CHECK: [[META16]] = !{!"llvm.loop.unroll.count", i32 1}
+; CHECK: [[DBG17]] = distinct !DISubprogram(name: "f", scope: [[META1]], file: [[META1]], line: 9, type: [[META4]], scopeLine: 9, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: [[META0]])
+; CHECK: [[LOOP18]] = distinct !{[[LOOP18]], [[META19:![0-9]+]], [[META21:![0-9]+]], [[META9]], [[META10]], [[META11]]}
+; CHECK: [[META19]] = !DILocation(line: 6, column: 3, scope: [[DBG3]], inlinedAt: [[META20:![0-9]+]])
+; CHECK: [[META20]] = distinct !DILocation(line: 9, column: 12, scope: [[DBG17]])
+; CHECK: [[META21]] = !DILocation(line: 7, column: 22, scope: [[DBG3]], inlinedAt: [[META20]])
+;.
>From 8587e1bc3c5f73f7893dbbc08bf4cb7b54bd631a Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Mon, 8 Sep 2025 14:46:37 +0200
Subject: [PATCH 2/6] [DebugInfo] Handle followup loop metadata in
updateLoopMetadataDebugLocations
Inliner/IROutliner/CodeExtractor all uses the
updateLoopMetadataDebugLocations helper in order to modify debug
location related to loop metadata. However, the helper has only
been updating DILocation nodes found as operands to the first level
of the MD_loop metadata. There could however be more DILocations
as part of the various kinds of followup metadata. A typical example
would be llvm.loop metadata like this
!6 = distinct !{!6, !7, !8, !9, !10, !11}
!7 = !DILocation(line: 6, column: 3, scope: !3)
!8 = !DILocation(line: 7, column: 22, scope: !3)
!11 = !{!"llvm.loop.distribute.followup_all", !7, !8, ..., !14}
!14 = !{!"llvm.loop.vectorize.followup_all", !7, !8, ...}
Instead of just updating !7 and !8 in !6, this patch make sure that
we now recursively update the DILocations in !11 and !14 as well.
Fixes #141568
---
llvm/lib/IR/DebugInfo.cpp | 32 +++++++++++++++++--
.../Inline/dilocation-loop-metadata-update.ll | 12 ++++---
2 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 166521a276643..2ce66d00b8ef9 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -375,6 +375,32 @@ bool DebugInfoFinder::addScope(DIScope *Scope) {
return true;
}
+// Recursively handle DILocations in followup metadata etc.
+static Metadata *updateLoopMetadataDebugLocationsRecursive(
+ Metadata *MetadataIn, function_ref<Metadata *(Metadata *)> Updater) {
+ const MDNode *M = dyn_cast_or_null<MDNode>(MetadataIn);
+ // The loop metadata options should start with a MDString.
+ if (!M || M->getNumOperands() < 1 || !isa<MDString>(M->getOperand(0)))
+ return nullptr;
+
+ bool Updated = false;
+ SmallVector<Metadata *, 4> MDs {M->getOperand(0)};
+ for (Metadata *MD : llvm::drop_begin(M->operands())) {
+ if (!MD) {
+ MDs.push_back(nullptr);
+ } else if (Metadata *NewMD = updateLoopMetadataDebugLocationsRecursive(
+ MD, Updater)) {
+ MDs.push_back(NewMD);
+ Updated = true;
+ } else if (Metadata *NewMD = Updater(MD)) {
+ MDs.push_back(NewMD);
+ Updated |= NewMD != MD;
+ }
+ }
+
+ return Updated ? MDNode::get(M->getContext(), MDs) : nullptr;
+}
+
static MDNode *updateLoopMetadataDebugLocationsImpl(
MDNode *OrigLoopID, function_ref<Metadata *(Metadata *)> Updater) {
assert(OrigLoopID && OrigLoopID->getNumOperands() > 0 &&
@@ -385,10 +411,12 @@ static MDNode *updateLoopMetadataDebugLocationsImpl(
// Save space for the self-referential LoopID.
SmallVector<Metadata *, 4> MDs = {nullptr};
- for (unsigned i = 1; i < OrigLoopID->getNumOperands(); ++i) {
- Metadata *MD = OrigLoopID->getOperand(i);
+ for (Metadata *MD : llvm::drop_begin(OrigLoopID->operands())) {
if (!MD)
MDs.push_back(nullptr);
+ else if (Metadata *NewMD = updateLoopMetadataDebugLocationsRecursive(
+ MD, Updater))
+ MDs.push_back(NewMD);
else if (Metadata *NewMD = Updater(MD))
MDs.push_back(NewMD);
}
diff --git a/llvm/test/Transforms/Inline/dilocation-loop-metadata-update.ll b/llvm/test/Transforms/Inline/dilocation-loop-metadata-update.ll
index 54a54a3986cd1..1bc132663331b 100644
--- a/llvm/test/Transforms/Inline/dilocation-loop-metadata-update.ll
+++ b/llvm/test/Transforms/Inline/dilocation-loop-metadata-update.ll
@@ -23,10 +23,10 @@ define void @f() !dbg !17 {
; CHECK-LABEL: define void @f(
; CHECK-SAME: ) !dbg [[DBG17:![0-9]+]] {
; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: br label %[[FOR_BODY_I:.*]]
-; CHECK: [[FOR_BODY_I]]:
-; CHECK-NEXT: br label %[[FOR_BODY_I]], !llvm.loop [[LOOP18:![0-9]+]]
-; CHECK: [[A_EXIT:.*:]]
+; CHECK-NEXT: br label %[[A_EXIT:.*]]
+; CHECK: [[A_EXIT]]:
+; CHECK-NEXT: br label %[[A_EXIT]], !llvm.loop [[LOOP18:![0-9]+]]
+; CHECK: [[A_EXIT1:.*:]]
; CHECK-NEXT: ret void
;
entry:
@@ -74,8 +74,10 @@ entry:
; CHECK: [[META15]] = !{!"llvm.loop.isvectorized"}
; CHECK: [[META16]] = !{!"llvm.loop.unroll.count", i32 1}
; CHECK: [[DBG17]] = distinct !DISubprogram(name: "f", scope: [[META1]], file: [[META1]], line: 9, type: [[META4]], scopeLine: 9, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: [[META0]])
-; CHECK: [[LOOP18]] = distinct !{[[LOOP18]], [[META19:![0-9]+]], [[META21:![0-9]+]], [[META9]], [[META10]], [[META11]]}
+; CHECK: [[LOOP18]] = distinct !{[[LOOP18]], [[META19:![0-9]+]], [[META21:![0-9]+]], [[META9]], [[META10]], [[META22:![0-9]+]]}
; CHECK: [[META19]] = !DILocation(line: 6, column: 3, scope: [[DBG3]], inlinedAt: [[META20:![0-9]+]])
; CHECK: [[META20]] = distinct !DILocation(line: 9, column: 12, scope: [[DBG17]])
; CHECK: [[META21]] = !DILocation(line: 7, column: 22, scope: [[DBG3]], inlinedAt: [[META20]])
+; CHECK: [[META22]] = !{!"llvm.loop.distribute.followup_all", [[META19]], [[META21]], [[META9]], [[META12]], [[META13]], [[META23:![0-9]+]]}
+; CHECK: [[META23]] = !{!"llvm.loop.vectorize.followup_all", [[META19]], [[META21]], [[META9]], [[META15]], [[META16]]}
;.
>From 8d56a63bee8a1fafd81471b7397aa83cc2066ac5 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Tue, 9 Sep 2025 00:03:04 +0200
Subject: [PATCH 3/6] Fix formatting (clang-format)
---
llvm/lib/IR/DebugInfo.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 2ce66d00b8ef9..c7e2bb35670a8 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -384,12 +384,12 @@ static Metadata *updateLoopMetadataDebugLocationsRecursive(
return nullptr;
bool Updated = false;
- SmallVector<Metadata *, 4> MDs {M->getOperand(0)};
+ SmallVector<Metadata *, 4> MDs{M->getOperand(0)};
for (Metadata *MD : llvm::drop_begin(M->operands())) {
if (!MD) {
MDs.push_back(nullptr);
- } else if (Metadata *NewMD = updateLoopMetadataDebugLocationsRecursive(
- MD, Updater)) {
+ } else if (Metadata *NewMD =
+ updateLoopMetadataDebugLocationsRecursive(MD, Updater)) {
MDs.push_back(NewMD);
Updated = true;
} else if (Metadata *NewMD = Updater(MD)) {
@@ -414,8 +414,8 @@ static MDNode *updateLoopMetadataDebugLocationsImpl(
for (Metadata *MD : llvm::drop_begin(OrigLoopID->operands())) {
if (!MD)
MDs.push_back(nullptr);
- else if (Metadata *NewMD = updateLoopMetadataDebugLocationsRecursive(
- MD, Updater))
+ else if (Metadata *NewMD =
+ updateLoopMetadataDebugLocationsRecursive(MD, Updater))
MDs.push_back(NewMD);
else if (Metadata *NewMD = Updater(MD))
MDs.push_back(NewMD);
>From 155ae8ab6de400504a58f685f7e0c6ceb501d9cb Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Wed, 24 Sep 2025 13:58:39 +0200
Subject: [PATCH 4/6] Fixups based on review feedback
---
llvm/lib/IR/DebugInfo.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index c7e2bb35670a8..852323e79868d 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -375,10 +375,10 @@ bool DebugInfoFinder::addScope(DIScope *Scope) {
return true;
}
-// Recursively handle DILocations in followup metadata etc.
+/// Recursively handle DILocations in followup metadata etc.
static Metadata *updateLoopMetadataDebugLocationsRecursive(
Metadata *MetadataIn, function_ref<Metadata *(Metadata *)> Updater) {
- const MDNode *M = dyn_cast_or_null<MDNode>(MetadataIn);
+ const MDTuple *M = dyn_cast_or_null<MDTuple>(MetadataIn);
// The loop metadata options should start with a MDString.
if (!M || M->getNumOperands() < 1 || !isa<MDString>(M->getOperand(0)))
return nullptr;
@@ -398,6 +398,7 @@ static Metadata *updateLoopMetadataDebugLocationsRecursive(
}
}
+ assert(!M->isDistinct() && "Assuming that M isn't distinct.");
return Updated ? MDNode::get(M->getContext(), MDs) : nullptr;
}
>From 8180708d49d1b1680b03f696ce915fa8095f0c65 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Wed, 24 Sep 2025 15:46:23 +0200
Subject: [PATCH 5/6] Refactor return value from
updateLoopMetadataDebugLocationsRecursive
Let updateLoopMetadataDebugLocationsRecursive return the input
node unless making changes.
---
llvm/lib/IR/DebugInfo.cpp | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 852323e79868d..9e2ff119f35fb 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -381,25 +381,24 @@ static Metadata *updateLoopMetadataDebugLocationsRecursive(
const MDTuple *M = dyn_cast_or_null<MDTuple>(MetadataIn);
// The loop metadata options should start with a MDString.
if (!M || M->getNumOperands() < 1 || !isa<MDString>(M->getOperand(0)))
- return nullptr;
+ return MetadataIn;
bool Updated = false;
SmallVector<Metadata *, 4> MDs{M->getOperand(0)};
for (Metadata *MD : llvm::drop_begin(M->operands())) {
- if (!MD) {
+ if (!MD)
MDs.push_back(nullptr);
- } else if (Metadata *NewMD =
- updateLoopMetadataDebugLocationsRecursive(MD, Updater)) {
- MDs.push_back(NewMD);
- Updated = true;
- } else if (Metadata *NewMD = Updater(MD)) {
- MDs.push_back(NewMD);
+ else {
+ Metadata *NewMD =
+ Updater(updateLoopMetadataDebugLocationsRecursive(MD, Updater));
+ if (NewMD)
+ MDs.push_back(NewMD);
Updated |= NewMD != MD;
}
}
- assert(!M->isDistinct() && "Assuming that M isn't distinct.");
- return Updated ? MDNode::get(M->getContext(), MDs) : nullptr;
+ assert(!M->isDistinct() && "M should not be distinct.");
+ return Updated ? MDNode::get(M->getContext(), MDs) : MetadataIn;
}
static MDNode *updateLoopMetadataDebugLocationsImpl(
@@ -415,10 +414,8 @@ static MDNode *updateLoopMetadataDebugLocationsImpl(
for (Metadata *MD : llvm::drop_begin(OrigLoopID->operands())) {
if (!MD)
MDs.push_back(nullptr);
- else if (Metadata *NewMD =
- updateLoopMetadataDebugLocationsRecursive(MD, Updater))
- MDs.push_back(NewMD);
- else if (Metadata *NewMD = Updater(MD))
+ else if (Metadata *NewMD = Updater(
+ updateLoopMetadataDebugLocationsRecursive(MD, Updater)))
MDs.push_back(NewMD);
}
>From c67f82ccd3e7df095c9d0cd4b45d95dfa15d607c Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Thu, 25 Sep 2025 13:02:12 +0200
Subject: [PATCH 6/6] More updates based on review feedback
---
llvm/lib/IR/DebugInfo.cpp | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index 9e2ff119f35fb..f9ded507f8328 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -376,6 +376,12 @@ bool DebugInfoFinder::addScope(DIScope *Scope) {
}
/// Recursively handle DILocations in followup metadata etc.
+///
+/// TODO: If for example a followup loop metadata would refence itself this
+/// function would go into infinite recursion. We do not expect such cycles in
+/// the loop metadata (except for the self-referencing first element
+/// "LoopID"). However, we could at least handle such situations more gracefully
+/// somehow (e.g. by keeping track of visited nodes and dropping metadata).
static Metadata *updateLoopMetadataDebugLocationsRecursive(
Metadata *MetadataIn, function_ref<Metadata *(Metadata *)> Updater) {
const MDTuple *M = dyn_cast_or_null<MDTuple>(MetadataIn);
@@ -386,15 +392,15 @@ static Metadata *updateLoopMetadataDebugLocationsRecursive(
bool Updated = false;
SmallVector<Metadata *, 4> MDs{M->getOperand(0)};
for (Metadata *MD : llvm::drop_begin(M->operands())) {
- if (!MD)
+ if (!MD) {
MDs.push_back(nullptr);
- else {
- Metadata *NewMD =
- Updater(updateLoopMetadataDebugLocationsRecursive(MD, Updater));
- if (NewMD)
- MDs.push_back(NewMD);
- Updated |= NewMD != MD;
+ continue;
}
+ Metadata *NewMD =
+ Updater(updateLoopMetadataDebugLocationsRecursive(MD, Updater));
+ if (NewMD)
+ MDs.push_back(NewMD);
+ Updated |= NewMD != MD;
}
assert(!M->isDistinct() && "M should not be distinct.");
More information about the llvm-commits
mailing list