release/21.x: [KeyInstr] goto stmt atoms (#149101)

[llvmbot]

Backport 5c7c855

Requested by: @OCHyams

[llvmbot]

@jmorse What do you think about merging this PR to the release branch?

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport 5c7c855

Requested by: @OCHyams

---

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

2 Files Affected:

• (modified) clang/lib/CodeGen/CGStmt.cpp (+4)
• (added) clang/test/DebugInfo/KeyInstructions/goto.c (+30)

diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index e0650067b9547..26259a8894bd1 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -846,11 +846,13 @@ void CodeGenFunction::EmitGotoStmt(const GotoStmt &S) {
   if (HaveInsertPoint())
     EmitStopPoint(&S);
 
+  ApplyAtomGroup Grp(getDebugInfo());
   EmitBranchThroughCleanup(getJumpDestForLabel(S.getLabel()));
 }
 
 
 void CodeGenFunction::EmitIndirectGotoStmt(const IndirectGotoStmt &S) {
+  ApplyAtomGroup Grp(getDebugInfo());
   if (const LabelDecl *Target = S.getConstantTarget()) {
     EmitBranchThroughCleanup(getJumpDestForLabel(Target));
     return;
@@ -869,6 +871,8 @@ void CodeGenFunction::EmitIndirectGotoStmt(const IndirectGotoStmt &S) {
   cast<llvm::PHINode>(IndGotoBB->begin())->addIncoming(V, CurBB);
 
   EmitBranch(IndGotoBB);
+  if (CurBB && CurBB->getTerminator())
+    addInstToCurrentSourceAtom(CurBB->getTerminator(), nullptr);
 }
 
 void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
diff --git a/clang/test/DebugInfo/KeyInstructions/goto.c b/clang/test/DebugInfo/KeyInstructions/goto.c
new file mode 100644
index 0000000000000..ead92e600ca5b
--- /dev/null
+++ b/clang/test/DebugInfo/KeyInstructions/goto.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -gkey-instructions -x c++ -std=c++17 %s -debug-info-kind=line-tables-only -emit-llvm -o - -gno-column-info \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -gkey-instructions -x c %s -debug-info-kind=line-tables-only -emit-llvm -o - -gno-column-info \
+// RUN: | FileCheck %s
+
+// Check the goto branches get Key Instructions metadata.
+void ext();
+void test_goto(void) {
+// CHECK: br label %dst1, !dbg [[G1R1:!.*]]
+  goto dst1;
+dst1:
+  ext();
+
+  void *ptr = &&dst2;
+// CHECK: br label %indirectgoto, !dbg [[G3R1:!.*]]
+  goto *ptr;
+dst2:
+  ext();
+
+// CHECK: br label %dst3, !dbg [[G4R1:!.*]]
+  goto *&&dst3;
+dst3:
+  ext();
+
+  return;
+}
+
+// CHECK: [[G1R1]] = !DILocation(line: 10, scope: ![[#]], atomGroup: 1, atomRank: 1)
+// CHECK: [[G3R1]] = !DILocation(line: 16, scope: ![[#]], atomGroup: 3, atomRank: 1)
+// CHECK: [[G4R1]] = !DILocation(line: 21, scope: ![[#]], atomGroup: 4, atomRank: 1)

[llvmbot]

@llvm/pr-subscribers-clang-codegen

Author: None (llvmbot)

Changes

Backport 5c7c855

Requested by: @OCHyams

---

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

2 Files Affected:

• (modified) clang/lib/CodeGen/CGStmt.cpp (+4)
• (added) clang/test/DebugInfo/KeyInstructions/goto.c (+30)

diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index e0650067b9547..26259a8894bd1 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -846,11 +846,13 @@ void CodeGenFunction::EmitGotoStmt(const GotoStmt &S) {
   if (HaveInsertPoint())
     EmitStopPoint(&S);
 
+  ApplyAtomGroup Grp(getDebugInfo());
   EmitBranchThroughCleanup(getJumpDestForLabel(S.getLabel()));
 }
 
 
 void CodeGenFunction::EmitIndirectGotoStmt(const IndirectGotoStmt &S) {
+  ApplyAtomGroup Grp(getDebugInfo());
   if (const LabelDecl *Target = S.getConstantTarget()) {
     EmitBranchThroughCleanup(getJumpDestForLabel(Target));
     return;
@@ -869,6 +871,8 @@ void CodeGenFunction::EmitIndirectGotoStmt(const IndirectGotoStmt &S) {
   cast<llvm::PHINode>(IndGotoBB->begin())->addIncoming(V, CurBB);
 
   EmitBranch(IndGotoBB);
+  if (CurBB && CurBB->getTerminator())
+    addInstToCurrentSourceAtom(CurBB->getTerminator(), nullptr);
 }
 
 void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
diff --git a/clang/test/DebugInfo/KeyInstructions/goto.c b/clang/test/DebugInfo/KeyInstructions/goto.c
new file mode 100644
index 0000000000000..ead92e600ca5b
--- /dev/null
+++ b/clang/test/DebugInfo/KeyInstructions/goto.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -gkey-instructions -x c++ -std=c++17 %s -debug-info-kind=line-tables-only -emit-llvm -o - -gno-column-info \
+// RUN: | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -gkey-instructions -x c %s -debug-info-kind=line-tables-only -emit-llvm -o - -gno-column-info \
+// RUN: | FileCheck %s
+
+// Check the goto branches get Key Instructions metadata.
+void ext();
+void test_goto(void) {
+// CHECK: br label %dst1, !dbg [[G1R1:!.*]]
+  goto dst1;
+dst1:
+  ext();
+
+  void *ptr = &&dst2;
+// CHECK: br label %indirectgoto, !dbg [[G3R1:!.*]]
+  goto *ptr;
+dst2:
+  ext();
+
+// CHECK: br label %dst3, !dbg [[G4R1:!.*]]
+  goto *&&dst3;
+dst3:
+  ext();
+
+  return;
+}
+
+// CHECK: [[G1R1]] = !DILocation(line: 10, scope: ![[#]], atomGroup: 1, atomRank: 1)
+// CHECK: [[G3R1]] = !DILocation(line: 16, scope: ![[#]], atomGroup: 3, atomRank: 1)
+// CHECK: [[G4R1]] = !DILocation(line: 21, scope: ![[#]], atomGroup: 4, atomRank: 1)

[OCHyams]

It's not clear to me what the cutoff for backporting is so I opened this to get opinions. It's a low risk change that improves a new-this-release feature, but it's not critical to the feature overall, and not a crash-fix.

[jmorse]

LGTM, it's a minor change, and the feature won't be enabled by default on the release branch anyway right?

[OCHyams]

and the feature won't be enabled by default on the release branch anyway right?

That's right, feature support is enabled (so the option -gkey-instruction is available and works), but isn't enabled by default.

[github-actions]

@OCHyams (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.