-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[KeyInstr][Inline] Don't propagate atoms to inlined nodebug instructions #133485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-llvm-ir Author: Orlando Cazalet-Hyams (OCHyams) ChangesFull diff: https://github.com/llvm/llvm-project/pull/133485.diff 3 Files Affected:
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index ede3bda5c68f4..243ea5e1236ad 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -2145,6 +2145,13 @@ class DILocation : public MDNode {
return 0;
}
+ const DILocation *getOrCloneWithoutAtom() const {
+ if (!getAtomGroup() && !getAtomRank())
+ return this;
+ return get(getContext(), getLine(), getColumn(), getScope(), getInlinedAt(),
+ isImplicitCode());
+ }
+
// Disallow replacing operands.
void replaceOperandWith(unsigned I, Metadata *New) = delete;
diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 9202451f5726d..fa4f21480b94d 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1821,9 +1821,9 @@ static DebugLoc inlineDebugLoc(DebugLoc OrigDL, DILocation *InlinedAt,
/// to encode location where these instructions are inlined.
static void fixupLineNumbers(Function *Fn, Function::iterator FI,
Instruction *TheCall, bool CalleeHasDebugInfo) {
- const DebugLoc &TheCallDL = TheCall->getDebugLoc();
- if (!TheCallDL)
+ if (!TheCall->getDebugLoc())
return;
+ DebugLoc TheCallDL = TheCall->getDebugLoc().get()->getOrCloneWithoutAtom();
auto &Ctx = Fn->getContext();
DILocation *InlinedAtNode = TheCallDL;
diff --git a/llvm/test/DebugInfo/KeyInstructions/Generic/inline-nodbg.ll b/llvm/test/DebugInfo/KeyInstructions/Generic/inline-nodbg.ll
new file mode 100644
index 0000000000000..33f7f673d91c3
--- /dev/null
+++ b/llvm/test/DebugInfo/KeyInstructions/Generic/inline-nodbg.ll
@@ -0,0 +1,43 @@
+; RUN: opt %s -passes=inline -S -o - | FileCheck %s
+
+;; $ cat test.cpp
+;; int g;
+;; [[clang::always_inline, gnu::nodebug]] void a() { g = 1; }
+;; void b() { a(); }
+;;
+;; Check the inlined instructions don't inherit the call's atom info.
+;; FIXME: Perhaps we want to do actually do that, to preserve existing
+;; behaviour? Unclear what's best.
+
+; CHECK: _Z1bv()
+; CHECK: store i32 1, ptr @g, align 4, !dbg [[DBG:!.*]]
+; CHECK: [[DBG]] = !DILocation(line: 3, scope: ![[#]])
+
+@g = hidden global i32 0, align 4
+
+define hidden void @_Z1av() {
+entry:
+ store i32 1, ptr @g, align 4
+ ret void
+}
+
+define hidden void @_Z1bv() !dbg !15 {
+entry:
+ call void @_Z1av(), !dbg !18
+ ret void, !dbg !19
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3}
+!llvm.ident = !{!10}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_17, file: !1, producer: "clang version 19.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", directory: "/")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{!"clang version 19.0.0"}
+!15 = distinct !DISubprogram(name: "b", scope: !1, file: !1, line: 3, type: !16, scopeLine: 3, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!16 = !DISubroutineType(types: !17)
+!17 = !{}
+!18 = !DILocation(line: 3, scope: !15, atomGroup: 1, atomRank: 1)
+!19 = !DILocation(line: 3, scope: !15, atomGroup: 2, atomRank: 1)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think conceptually there is some space for copying atom group/rank to the inlined instructions, giving the instruction(s) that produce the return value (if any) the highest precedence. This would be a separate feature however, and this behaviour seems fine to me as a first pass; minor inline comment.
@@ -2145,6 +2145,13 @@ class DILocation : public MDNode { | |||
return 0; | |||
} | |||
|
|||
const DILocation *getOrCloneWithoutAtom() const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could just be "getWithoutAtom", it's already implied with DIMetadata that "get" means "find me an existing metadata or create a new one".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nit
return; | ||
DebugLoc TheCallDL = TheCall->getDebugLoc().get()->getOrCloneWithoutAtom(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment on why this is necessary desired.
e2d5b6e
to
a965e6e
Compare
637b5b4
to
d5d3676
Compare
No description provided.