Skip to content

[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

Merged
merged 2 commits into from
May 7, 2025

Conversation

OCHyams
Copy link
Contributor

@OCHyams OCHyams commented Mar 28, 2025

No description provided.

Copy link
Contributor Author

OCHyams commented Mar 28, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2025

@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-ir

Author: Orlando Cazalet-Hyams (OCHyams)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/133485.diff

3 Files Affected:

  • (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+7)
  • (modified) llvm/lib/Transforms/Utils/InlineFunction.cpp (+2-2)
  • (added) llvm/test/DebugInfo/KeyInstructions/Generic/inline-nodbg.ll (+43)
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)

Copy link
Contributor

@SLTozer SLTozer left a 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 {
Copy link
Contributor

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".

Copy link
Member

@jmorse jmorse left a 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();
Copy link
Member

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.

@OCHyams OCHyams force-pushed the users/OCHyams/ki-llvm-simplifycfg2 branch from e2d5b6e to a965e6e Compare May 6, 2025 15:52
Base automatically changed from users/OCHyams/ki-llvm-simplifycfg2 to main May 7, 2025 08:56
@OCHyams OCHyams force-pushed the users/OCHyams/ki-llvm-inline2 branch from 637b5b4 to d5d3676 Compare May 7, 2025 08:59
@OCHyams OCHyams merged commit 5be080e into main May 7, 2025
8 of 10 checks passed
@OCHyams OCHyams deleted the users/OCHyams/ki-llvm-inline2 branch May 7, 2025 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants