[PATCH] D96938: [RFC] [Coroutine] [Debug] Insert dbg.declare to entry.resume to print alloca in the coroutine frame under O2

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:2219
+  else if (isa<Argument>(Storage))
+    DDI->moveAfter(F->getEntryBlock().getFirstNonPHI());
 }
----------------
After salvage the debug information, if the Storage is Argument (which means the FramePtr in resume function), we should insert DDI just in the entry block of the resume function. I am not clear about whether the design is suitable for swift and mlir. 


================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:666
+    auto *Storage = DDI->getAddress();
+    if (isa<Argument>(Storage)) {
+      auto &Entry = NewF->getEntryBlock();
----------------
aprantl wrote:
> I don't understand this change. Is this covered by your test? Are we even emitting dbg.declares when optimizations are turned on?
I moved this change to SalvageDebugInfo at CoroFrame.cpp.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:666
+    auto *Storage = DDI->getAddress();
+    if (isa<Argument>(Storage)) {
+      auto &Entry = NewF->getEntryBlock();
----------------
ChuanqiXu wrote:
> aprantl wrote:
> > I don't understand this change. Is this covered by your test? Are we even emitting dbg.declares when optimizations are turned on?
> I moved this change to SalvageDebugInfo at CoroFrame.cpp.
With the the optimization are turned on, the debug information for promise (which would be part of coroutine by the standard) is dbg.declare. I am not clear if this is just a miss match or by design. As a result, it makes it easy to print the promise. And I think it should be OK to emit dbg.declare for promise. Since it would be put in the coroutine frame, I think the chance to optimize it is rare.


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

https://reviews.llvm.org/D96938