[PATCH] D132580: [Coro][Debuginfo] Add debug info to `_NoopCoro_ResumeDestroy` function

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroEarly.cpp:120
         Function::Create(FnTy, GlobalValue::LinkageTypes::PrivateLinkage,
-                         "NoopCoro.ResumeDestroy", &M);
+                         "_NoopCoro_ResumeDestroy", &M);
     NoopFn->setCallingConv(CallingConv::Fast);
----------------
The double underscores look more like reserved.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroEarly.cpp:129
+    StringRef Producer = "LLVM Version " LLVM_VERSION_STRING;
+    auto *CU =
+        DB.createCompileUnit(dwarf::DW_LANG_Mips_Assembler, FileNode, Producer,
----------------
avogelsgesang wrote:
> aprantl wrote:
> > avogelsgesang wrote:
> > > ChuanqiXu wrote:
> > > > avogelsgesang wrote:
> > > > > aprantl wrote:
> > > > > > Is there a specific reason why you want to create a new CU here? I would just use the one from the function we're currently processing and mark the new DISubprogram as artificial.
> > > > > not really, it just felt the most natural to me. The user didn't write that function anywhere, hence I thought: the file name should read "<llvm-instrinsics>". Also, it seems there can be multiple compilation units attached to a single LLVM module, and I didn't know how to determine the one to which I should add this function
> > > > It looks better to split these logic to another smaller function. And we should generate debug information only if the debug option `-g` enabled. I think we can look at the start of `buildFrameDebugInfo` to get this. Then we can reuse the CU.
> > > `buildFrameDebugInfo` looks at the current function to figure out if debug information should be created. This works nicely for coroutine splitting. For `lowerCoroNoop`, I don't think this is a good idea. The generated `_NoopCoro_ResumeDestroy` is shared across potentially many functions, some of which could have debug info, some of which won't
> > > The user didn't write that function anywhere
> > 
> > The artificial flag on the subprogram and line 0 as a source location already signal that. But it should still part of the same translation/compile unit. That's how all other compiler-generated stubs / thunks / etc are also represented.
> I am reusing the existing CU now.
> 
> > we should generate debug information only if the debug option -g enabled
> 
> If no compilation unit debug info exists, we now don't add debug info for `_NoopCoro_ResumeDestroy`, either
I mean extract the if into a static function `buildDebugInfoForNoopCoroutine` or something else. I feel it would be better to read. The current one is slightly hard to read. Since the debug information may be noisy for people who want to visit how llvm handles coroutines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132580/new/

https://reviews.llvm.org/D132580