[PATCH] D132240: [Coroutine][Debug] Add line and column number to suspension point id

[Adrian Vogelsgesang via Phabricator via llvm-commits]

avogelsgesang created this revision.
Herald added subscribers: ChuanqiXu, hiraditya.
Herald added a project: All.
avogelsgesang requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This commit is not ready to be merged! I post it for review mostly to
get early feedback on the overall approach. Test cases etc. are
currently still missing. Also, if we get consensus that we want to
follow up with this approach, I still need to update
https://clang.llvm.org/docs/DebuggingCoroutines.html

This commit improves the debug representation of the "suspension point
id" inside a coroutine frame. So far, the suspension point was presented
to the debugger as a simple integer. There was no simple way to map
this integer back to the corresponding line/column numbers of the
suspension point.

With this change, the suspension point is instead represented as an
enum where the individual enum values are name `line_*_column_*`.
Furthermore, this commit renames `__coro_index` into
`__suspension_point`. I think this name is better suited for the
debugger because it is more familiar to end users: As a C++ programmer I
usually think about suspension points instead of their indices.

When printing a coroutine frame we now get

  $1 = {__resume_fn = 0x555555555940 <test(int&)>, __destroy_fn = 0x555555555f10 <test(int&)>,
    __promise = {<No data fields>}, __suspension_point = __suspension_point::line_23_column_36,
    ...

instead of

  $1 = {__resume_fn = 0x555555555940 <test(int&)>, __destroy_fn = 0x555555555f10 <test(int&)>,
    __promise = {<No data fields>}, __coro_index = 1 '001',
    ...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132240

Files:
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp


Index: llvm/lib/Transforms/Coroutines/CoroFrame.cpp
===================================================================
--- llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -997,7 +997,7 @@
   DenseMap<unsigned, StringRef> NameCache;
   NameCache.insert({ResumeIndex, "__resume_fn"});
   NameCache.insert({DestroyIndex, "__destroy_fn"});
-  NameCache.insert({IndexIndex, "__coro_index"});
+  NameCache.insert({IndexIndex, "__suspension_point"});
 
   Type *ResumeFnTy = FrameTy->getElementType(ResumeIndex),
        *DestroyFnTy = FrameTy->getElementType(DestroyIndex),
@@ -1011,14 +1011,37 @@
       {DestroyIndex, DBuilder.createPointerType(
                          nullptr, Layout.getTypeSizeInBits(DestroyFnTy))});
 
-  /// FIXME: If we fill the field `SizeInBits` with the actual size of
-  /// __coro_index in bits, then __coro_index wouldn't show in the debugger.
-  TyCache.insert({IndexIndex, DBuilder.createBasicType(
-                                  "__coro_index",
-                                  (Layout.getTypeSizeInBits(IndexTy) < 8)
-                                      ? 8
-                                      : Layout.getTypeSizeInBits(IndexTy),
-                                  dwarf::DW_ATE_unsigned_char)});
+  /// Declare the suspension point index as an enum, where the names of
+  /// the enum values indicate the line numbers of the suspension point.
+  SmallVector<llvm::Metadata *, 16> IndexEnumerators;
+  for (uint64_t Idx = 0; Idx < Shape.CoroSuspends.size(); ++Idx) {
+    std::string Name;
+    // ABI final suspend
+    if (Shape.ABI == coro::ABI::Switch &&
+        Shape.SwitchLowering.HasFinalSuspend &&
+        Idx == Shape.CoroSuspends.size() - 1) {
+       Name = "final_suspend";
+    } else if (auto DebugLoc = Shape.CoroSuspends[Idx]->getDebugLoc()) {
+       Name = "line_";
+       Name += std::to_string(DebugLoc.getLine());
+       Name += "_column_";
+       Name += std::to_string(DebugLoc.getCol());
+    }
+    if (!Name.empty()) {
+      IndexEnumerators.push_back(DBuilder.createEnumerator(Name, Idx));
+    }
+  }
+  DIBasicType *IndexIntType =
+      DBuilder.createBasicType("__suspension_point_idx",
+                               (Layout.getTypeSizeInBits(IndexTy) < 8)
+                                   ? 8
+                                   : Layout.getTypeSizeInBits(IndexTy),
+                               dwarf::DW_ATE_unsigned);
+  DICompositeType *IndexEnumType = DBuilder.createEnumerationType(
+      PromiseDIScope, "__suspension_point", DFile, LineNum, /*Size*/ 0, /*Align*/ 0,
+      DBuilder.getOrCreateArray(IndexEnumerators), IndexIntType,
+      /*Identifier*/ "", /*isScoped*/ true);
+  TyCache.insert({IndexIndex, IndexEnumType});
 
   for (auto *V : FrameData.getAllDefs()) {
     if (DIVarCache.find(V) == DIVarCache.end())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D132240.454019.patch
Type: text/x-patch
Size: 2869 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220819/72a3a9ec/attachment.bin>