[KeyInstrs] Disable key-instructions for coroutine scopes

[jmorse]

At this time (immediately prior to llvm21 branching) we haven't instrumented coroutine generation to identify the "key" instructions of things like co_return and similar. This will lead to worse stepping behaviours, as there won't be any key instruction for those lines.

This patch removes the key-instructions flag from the DISubprograms for coroutines, which will cause AsmPrinter to use the "old" / existing linetable stepping behaviour, avoiding a regression until we can instrument these constructs.

(I'm going to post on discourse about whether this is a good idea or not in a moment)

[llvmbot]

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

Changes

At this time (immediately prior to llvm21 branching) we haven't instrumented coroutine generation to identify the "key" instructions of things like co_return and similar. This will lead to worse stepping behaviours, as there won't be any key instruction for those lines.

This patch removes the key-instructions flag from the DISubprograms for coroutines, which will cause AsmPrinter to use the "old" / existing linetable stepping behaviour, avoiding a regression until we can instrument these constructs.

(I'm going to post on discourse about whether this is a good idea or not in a moment)

---

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

2 Files Affected:

• (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+5-1)
• (added) clang/test/DebugInfo/KeyInstructions/coro-dwarf-key-instrs.cpp (+81)

diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 8bf7d24d8551d..5240853199c11 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4629,6 +4629,7 @@ void CGDebugInfo::emitFunctionStart(GlobalDecl GD, SourceLocation Loc,
   llvm::DIFile *Unit = getOrCreateFile(Loc);
   llvm::DIScope *FDContext = Unit;
   llvm::DINodeArray TParamsArray;
+  bool KeyInstructions = CGM.getCodeGenOpts().DebugKeyInstructions;
   if (!HasDecl) {
     // Use llvm function name.
     LinkageName = Fn->getName();
@@ -4645,6 +4646,9 @@ void CGDebugInfo::emitFunctionStart(GlobalDecl GD, SourceLocation Loc,
     }
     collectFunctionDeclProps(GD, Unit, Name, LinkageName, FDContext,
                              TParamsArray, Flags);
+    // Disable KIs if this is a coroutine.
+    KeyInstructions =
+        KeyInstructions && !isa_and_present<CoroutineBodyStmt>(FD->getBody());
   } else if (const auto *OMD = dyn_cast<ObjCMethodDecl>(D)) {
     Name = getObjCMethodName(OMD);
     Flags |= llvm::DINode::FlagPrototyped;
@@ -4706,7 +4710,7 @@ void CGDebugInfo::emitFunctionStart(GlobalDecl GD, SourceLocation Loc,
   llvm::DISubprogram *SP = DBuilder.createFunction(
       FDContext, Name, LinkageName, Unit, LineNo, DIFnType, ScopeLine,
       FlagsForDef, SPFlagsForDef, TParamsArray.get(), Decl, nullptr,
-      Annotations, "", CGM.getCodeGenOpts().DebugKeyInstructions);
+      Annotations, "", KeyInstructions);
   Fn->setSubprogram(SP);
 
   // We might get here with a VarDecl in the case we're generating
diff --git a/clang/test/DebugInfo/KeyInstructions/coro-dwarf-key-instrs.cpp b/clang/test/DebugInfo/KeyInstructions/coro-dwarf-key-instrs.cpp
new file mode 100644
index 0000000000000..66537db88155d
--- /dev/null
+++ b/clang/test/DebugInfo/KeyInstructions/coro-dwarf-key-instrs.cpp
@@ -0,0 +1,81 @@
+// RUN: %clang_cc1 -disable-llvm-optzns -std=c++20 \
+// RUN:            -triple=x86_64 -dwarf-version=4 -debug-info-kind=limited \
+// RUN:            -emit-llvm -o - %s -gkey-instructions | \
+// RUN:            FileCheck %s
+
+// Check that for the coroutine below, we mark the created DISubprogram as
+// not having key instructions. This will prevent AsmPrinter from trying to
+// instrument the linetable with key-instructions for source-locations in
+// the coroutine scope.
+//
+// This is a temporary workaround for key instructions: we can instrument
+// coroutine code in the future, but it hasn't been done yet.
+//
+// File contents copied from coro-dwarf.cpp.
+
+namespace std {
+template <typename... T> struct coroutine_traits;
+
+template <class Promise = void> struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept;
+};
+template <> struct coroutine_handle<void> {
+  static coroutine_handle from_address(void *) noexcept;
+  coroutine_handle() = default;
+  template <class PromiseType>
+  coroutine_handle(coroutine_handle<PromiseType>) noexcept;
+};
+} // namespace std
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template <typename... Args> struct std::coroutine_traits<void, Args...> {
+  struct promise_type {
+    void get_return_object() noexcept;
+    suspend_always initial_suspend() noexcept;
+    suspend_always final_suspend() noexcept;
+    void return_void() noexcept;
+    promise_type();
+    ~promise_type() noexcept;
+    void unhandled_exception() noexcept;
+  };
+};
+
+// TODO: Not supported yet
+struct CopyOnly {
+  int val;
+  CopyOnly(const CopyOnly &) noexcept;
+  CopyOnly(CopyOnly &&) = delete;
+  ~CopyOnly();
+};
+
+struct MoveOnly {
+  int val;
+  MoveOnly(const MoveOnly &) = delete;
+  MoveOnly(MoveOnly &&) noexcept;
+  ~MoveOnly();
+};
+
+struct MoveAndCopy {
+  int val;
+  MoveAndCopy(const MoveAndCopy &) noexcept;
+  MoveAndCopy(MoveAndCopy &&) noexcept;
+  ~MoveAndCopy();
+};
+
+void consume(int, int, int) noexcept;
+
+void f_coro(int val, MoveOnly moParam, MoveAndCopy mcParam) {
+  consume(val, moParam.val, mcParam.val);
+  co_return;
+}
+
+// CHECK: ![[SP:[0-9]+]] = distinct !DISubprogram(name: "f_coro", linkageName: "_Z6f_coroi8MoveOnly11MoveAndCopy"
+// CHECK-NOT: keyInstructions:
+// CHECK: !DIFil
+

[SLTozer]

What's the reason for creating this as a new file rather than adding this as a new check in the existing file - is it just to keep the test confined to the KeyInstructions test dir?