Improve the debug info for coro-split .resume functions

[aprantl]

Summary

This patch updates the scope line to point to the suspend point. This makes the first address in the function point to the first source line in the resume function rather than the function declaration. Without this the line table "jumps" from the beginning of the function to the suspend point at the beginning.

rdar://73386346

[aprantl]

@dexonsmith I'm curious if you have any comment about the safety of updating a distinct DISubprogram right after cloning it. I looked into injecting the replacement code into CloneFunction/ValueMapper but that didn't look too appealing either.

[aprantl]

Strip out unnecessary change.

[ChuanqiXu]

Is this change only works for continuation and async ABIs? If so, what can we do for switch ABIs?

[junparser]

I'm wonder whether this also affects backtrace of coroutine?

[dongAxis1944]

@aprantl
hello, this patch might cause debug llvm crash, the backtrace is :
#0 0x00007ffff672f8af in raise () from /lib64/libc.so.6
#1 0x00007ffff67314aa in abort () from /lib64/libc.so.6
#2 0x00007ffff6727d37 in assert_fail_base () from /lib64/libc.so.6
#3 0x00007ffff6727de2 in assert_fail () from /lib64/libc.so.6
#4 0x00000000063af226 in llvm::DwarfUnit::applySubprogramDefinitionAttributes (this=0x13e3f980, SP=0x13fdd8f0, SPDie=...) at /disk1/yifeng.dongyifeng/my_code/llvm/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1169
#5 0x00000000063af368 in llvm::DwarfUnit::applySubprogramAttributes (this=0x13e3f980, SP=0x13fdd8f0, SPDie=..., SkipSPAttributes=false) at /disk1/yifeng.dongyifeng/my_code/llvm/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:1193
#6 0x0000000006435376 in llvm::DwarfCompileUnit::applySubprogramAttributesToDefinition (this=0x13e3f980, SP=0x13fdd8f0, SPDie=...) at /disk1/yifeng.dongyifeng/my_code/llvm/llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:1448

the following assert will be triggle if the link name dose not match decl name:

1169      assert(((LinkageName.empty() || DeclLinkageName.empty()) ||
1170              LinkageName == DeclLinkageName) &&
1171             "decl has a linkage name and it is different");

[aprantl]

Thanks @dongAxis1944 that makes sense. In C++ all resume functions share a common declaration which is supposed to hold the linkage name. Since this change was cosmetic only anyway, I just removed it from the patch.

[ChuanqiXu]

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

848

Since this code only works under continuation and async ABIs. So it maybe better to add comment or add explicit guard-if to tell the semantic.

[aprantl]

Improved comment as suggested.

[ChuanqiXu]

This patch LGTM.

llvm/include/llvm/IR/DebugInfoMetadata.h

public:
   }
 
 public:
   unsigned getLine() const { return Line; }
   unsigned getVirtuality() const { return getSPFlags() & SPFlagVirtuality; }
   unsigned getVirtualIndex() const { return VirtualIndex; }
   int getThisAdjustment() const { return ThisAdjustment; }
   unsigned getScopeLine() const { return ScopeLine; }
+  void setScopeLine(unsigned L) { assert(isDistinct()); ScopeLine = L; }
   DIFlags getFlags() const { return Flags; }
   DISPFlags getSPFlags() const { return SPFlags; }
   bool isLocalToUnit() const { return getSPFlags() & SPFlagLocalToUnit; }
   bool isDefinition() const { return getSPFlags() & SPFlagDefinition; }
   bool isOptimized() const { return getSPFlags() & SPFlagOptimized; }
   bool isMainSubprogram() const { return getSPFlags() & SPFlagMainSubprogram; }
 
   bool isArtificial() const { return getFlags() & FlagArtificial; }
public:
   //
   // Returns true if this subprogram is a thunk generated by the compiler.
   bool isThunk() const { return getFlags() & FlagThunk; }
 
   DIScope *getScope() const { return cast_or_null<DIScope>(getRawScope()); }
 
   StringRef getName() const { return getStringOperand(2); }
   StringRef getLinkageName() const { return getStringOperand(3); }
+  void replaceLinkageName(MDString *LN) { replaceOperandWith(3, LN); }
 
   DISubroutineType *getType() const {
     return cast_or_null<DISubroutineType>(getRawType());
   }
   DIType *getContainingType() const {
     return cast_or_null<DIType>(getRawContainingType());
   }
 

llvm/include/llvm/Transforms/Utils/Cloning.h

 namespace llvm {
 
 class AAResults;
 class AllocaInst;
 class BasicBlock;
 class BlockFrequencyInfo;
 class CallInst;
 class CallGraph;
+class DISubprogram;
 class DebugInfoFinder;
 class DominatorTree;
 class Function;
 class Instruction;
 class InvokeInst;
 class Loop;
 class LoopInfo;
 class Module;

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

void CoroCloner::create() {
   auto savedUnnamedAddr = NewF->getUnnamedAddr();
   auto savedDLLStorageClass = NewF->getDLLStorageClass();
 
   // NewF's linkage (which CloneFunctionInto does *not* change) might not
   // be compatible with the visibility of OrigF (which it *does* change),
   // so protect against that.
   auto savedLinkage = NewF->getLinkage();
   NewF->setLinkage(llvm::GlobalValue::ExternalLinkage);
+  auto &Context = NewF->getContext();
 
   CloneFunctionInto(NewF, &OrigF, VMap,
                     CloneFunctionChangeType::LocalChangesOnly, Returns);
+  // Adjust the scope line of the clone to the line number of the suspend point.
+  if (DISubprogram *SP = NewF->getSubprogram()) {
+    assert(SP != OrigF.getSubprogram() && SP->isDistinct());
+    if (ActiveSuspend)
+      if (auto DL = ActiveSuspend->getDebugLoc())
+        SP->setScopeLine(DL->getLine());
+    SP->replaceLinkageName(MDString::get(Context, NewF->getName()));
+  }
 
   NewF->setLinkage(savedLinkage);
   NewF->setVisibility(savedVisibility);
   NewF->setUnnamedAddr(savedUnnamedAddr);
   NewF->setDLLStorageClass(savedDLLStorageClass);
 
-  auto &Context = NewF->getContext();
-
   // Replace the attributes of the new function:
   auto OrigAttrs = NewF->getAttributes();
   auto NewAttrs = AttributeList();
 
   switch (Shape.ABI) {
   case coro::ABI::Switch:
     // Bootstrap attributes by copying function attributes from the
     // original function.  This should include optimization settings and so on.

llvm/test/Transforms/Coroutines/coro-async.ll

 define i8* @resume_context_projection(i8* %ctxt) {
 entry:
   %resume_ctxt_addr = bitcast i8* %ctxt to i8**
   %resume_ctxt = load i8*, i8** %resume_ctxt_addr, align 8
   ret i8* %resume_ctxt
 }
 
 
-define swiftcc void @my_async_function(i8* %async.ctxt, %async.task* %task, %async.actor* %actor)  {
+define swiftcc void @my_async_function(i8* %async.ctxt, %async.task* %task, %async.actor* %actor) !dbg !1 {
 entry:
   %tmp = alloca { i64, i64 }, align 8
   %proj.1 = getelementptr inbounds { i64, i64 }, { i64, i64 }* %tmp, i64 0, i32 0
   %proj.2 = getelementptr inbounds { i64, i64 }, { i64, i64 }* %tmp, i64 0, i32 1
 
   %id = call token @llvm.coro.id.async(i32 128, i32 16, i32 0,
           i8* bitcast (<{i32, i32}>* @my_async_function_fp to i8*))
   %hdl = call i8* @llvm.coro.begin(token %id, i8* null)
entry:
   %callee_context.caller_context.addr = getelementptr inbounds %async.ctxt, %async.ctxt* %callee_context.0, i32 0, i32 0
   store i8* %async.ctxt, i8** %callee_context.caller_context.addr
   %resume_proj_fun = bitcast i8*(i8*)* @resume_context_projection to i8*
   %callee = bitcast void(i8*, %async.task*, %async.actor*)* @asyncSuspend to i8*
   %res = call {i8*, i8*, i8*} (i8*, i8*, ...) @llvm.coro.suspend.async(
                                                   i8* %resume.func_ptr,
                                                   i8* %resume_proj_fun,
                                                   void (i8*, i8*, %async.task*, %async.actor*)* @my_async_function.my_other_async_function_fp.apply,
-                                                  i8* %callee, i8* %callee_context, %async.task* %task, %async.actor *%actor)
+                                                  i8* %callee, i8* %callee_context, %async.task* %task, %async.actor *%actor), !dbg !5
 
   call void @llvm.coro.async.context.dealloc(i8* %callee_context)
   %continuation_task_arg = extractvalue {i8*, i8*, i8*} %res, 1
   %task.2 =  bitcast i8* %continuation_task_arg to %async.task*
   %val = load i64, i64* %proj.1
   call void @some_user(i64 %val)
   %val.2 = load i64, i64* %proj.2
   call void @some_user(i64 %val.2)
 
   tail call swiftcc void @asyncReturn(i8* %async.ctxt, %async.task* %task.2, %async.actor* %actor)
   call i1 (i8*, i1, ...) @llvm.coro.end.async(i8* %hdl, i1 0)
   unreachable
 }
 
 ; Make sure we update the async function pointer
 ; CHECK: @my_async_function_fp = constant <{ i32, i32 }> <{ {{.*}}, i32 176 }
 ; CHECK: @my_async_function2_fp = constant <{ i32, i32 }> <{ {{.*}}, i32 176 }
 
-; CHECK-LABEL: define swiftcc void @my_async_function(i8* %async.ctxt, %async.task* %task, %async.actor* %actor) {
+; CHECK-LABEL: define swiftcc void @my_async_function(i8* %async.ctxt, %async.task* %task, %async.actor* %actor)
+; CHECK-SAME: !dbg ![[SP1:[0-9]+]] {
 ; CHECK: entry:
 ; CHECK:   [[FRAMEPTR:%.*]] = getelementptr inbounds i8, i8* %async.ctxt, i64 128
 ; CHECK:   [[ACTOR_SPILL_ADDR:%.*]] = getelementptr inbounds i8, i8* %async.ctxt, i64 152
 ; CHECK:   [[CAST1:%.*]] = bitcast i8* [[ACTOR_SPILL_ADDR]] to %async.actor**
 ; CHECK:   store %async.actor* %actor, %async.actor** [[CAST1]]
 ; CHECK:   [[ADDR1:%.*]]  = getelementptr inbounds i8, i8* %async.ctxt, i64 144
 ; CHECK:   [[ASYNC_CTXT_SPILL_ADDR:%.*]] = bitcast i8* [[ADDR1]] to i8**
 ; CHECK:   store i8* %async.ctxt, i8** [[ASYNC_CTXT_SPILL_ADDR]]

 ; CHECK:   [[RETURN_TO_CALLER_ADDR:%.*]] = bitcast i8* [[TYPED_RETURN_TO_CALLER_ADDR]] to i8**
 ; CHECK:   store i8* bitcast (void (i8*, i8*, i8*)* @my_async_function.resume.0 to i8*), i8** [[RETURN_TO_CALLER_ADDR]]
 ; CHECK:   [[CALLER_CONTEXT_ADDR:%.*]] = bitcast i8* [[CALLEE_CTXT]] to i8**
 ; CHECK:   store i8* %async.ctxt, i8** [[CALLER_CONTEXT_ADDR]]
 ; CHECK:   tail call swiftcc void @asyncSuspend(i8* [[CALLEE_CTXT]], %async.task* %task, %async.actor* %actor)
 ; CHECK:   ret void
 ; CHECK: }
 
-; CHECK-LABEL: define internal swiftcc void @my_async_function.resume.0(i8* nocapture readonly %0, i8* %1, i8* nocapture readnone %2) {
+; CHECK-LABEL: define internal swiftcc void @my_async_function.resume.0(i8* nocapture readonly %0, i8* %1, i8* nocapture readnone %2)
+; CHECK-SAME: !dbg ![[SP2:[0-9]+]] {
 ; CHECK: entryresume.0:
 ; CHECK:   [[CALLER_CONTEXT_ADDR:%.*]] = bitcast i8* %0 to i8**
 ; CHECK:   [[CALLER_CONTEXT:%.*]] = load i8*, i8** [[CALLER_CONTEXT_ADDR]]
 ; CHECK:   [[FRAME_PTR:%.*]] = getelementptr inbounds i8, i8* [[CALLER_CONTEXT]], i64 128
 ; CHECK:   [[CALLEE_CTXT_SPILL_ADDR:%.*]] = getelementptr inbounds i8, i8* [[CALLER_CONTEXT]], i64 160
 ; CHECK:   [[CAST1:%.*]] = bitcast i8* [[CALLEE_CTXT_SPILL_ADDR]] to i8**
 ; CHECK:   [[CALLEE_CTXT_RELOAD:%.*]] = load i8*, i8** [[CAST1]]
 ; CHECK:   [[ACTOR_RELOAD_ADDR:%.*]] = getelementptr inbounds i8, i8* [[CALLER_CONTEXT]], i64 152

 declare i1 @llvm.coro.end.async(i8*, i1, ...)
 declare i1 @llvm.coro.end(i8*, i1)
 declare {i8*, i8*, i8*} @llvm.coro.suspend.async(i8*, i8*, ...)
 declare i8* @llvm.coro.async.context.alloc(i8*, i8*)
 declare void @llvm.coro.async.context.dealloc(i8*)
 declare swiftcc void @asyncReturn(i8*, %async.task*, %async.actor*)
 declare swiftcc void @asyncSuspend(i8*, %async.task*, %async.actor*)
 declare i8* @llvm.coro.async.resume()
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+
+; CHECK: ![[SP1]] = distinct !DISubprogram(name: "my_async_function",
+; CHECK-SAME:                              linkageName: "my_async_function",
+; CHECK-SAME:                              scopeLine: 1
+!1 = distinct !DISubprogram(name: "my_async_function", linkageName: "my_async_function", scope:; CHECK: ![[SP2]] = distinct !DISubprogram(name: "my_async_function",
+; CHECK-SAME:                              linkageName: "my_async_function.resume.0",
+; CHECK-SAME:                              scopeLine: 2
+ !2, file: !3, line: 1, type: !4, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !2)
+!2 = distinct !DICompileUnit(language: DW_LANG_Swift, file: !3, emissionKind: FullDebug)
+!3 = !DIFile(filename: "/tmp/1.swift", directory: "/")
+!4 = !DISubroutineType(types: !{})
+!5 = !DILocation(line: 2, column: 0, scope: !1)

llvm/test/Transforms/Coroutines/coro-debug.ll

 ; CHECK-NOT: alloca %struct.test*
 ; CHECK: call void @llvm.dbg.declare(metadata i32 0, metadata ![[RESUME_CONST:[0-9]+]], metadata !DIExpression())
 ; CHECK: call void @coro.devirt.trigger(i8* null)
 ; CHECK: define internal fastcc void @f.destroy(%f.Frame* noalias nonnull align 8 dereferenceable(32) %FramePtr) #0 !dbg ![[DESTROY:[0-9]+]]
 ; CHECK: define internal fastcc void @f.cleanup(%f.Frame* noalias nonnull align 8 dereferenceable(32) %FramePtr) #0 !dbg ![[CLEANUP:[0-9]+]]
 
 ; CHECK: ![[ORIG]] = distinct !DISubprogram(name: "f", linkageName: "flink"
 
-; CHECK: ![[RESUME]] = distinct !DISubprogram(name: "f", linkageName: "flink"
+; CHECK: ![[RESUME]] = distinct !DISubprogram(name: "f", linkageName: "f.resume"
 ; CHECK: ![[RESUME_COROHDL]] = !DILocalVariable(name: "coro_hdl", scope: ![[RESUME]]
 ; CHECK: ![[RESUME_X]] = !DILocalVariable(name: "x", arg: 1, scope: ![[RESUME]]
 ; CHECK: ![[RESUME_DIRECT]] = !DILocalVariable(name: "direct_mem", scope: ![[RESUME]]
 ; CHECK: ![[RESUME_CONST]] = !DILocalVariable(name: "direct_const", scope: ![[RESUME]]
 
-; CHECK: ![[DESTROY]] = distinct !DISubprogram(name: "f", linkageName: "flink"
+; CHECK: ![[DESTROY]] = distinct !DISubprogram(name: "f", linkageName: "f.destroy"
 
-; CHECK: ![[CLEANUP]] = distinct !DISubprogram(name: "f", linkageName: "flink"
+; CHECK: ![[CLEANUP]] = distinct !DISubprogram(name: "f", linkageName: "f.cleanup"