[PATCH] D99179: [RFC] [Coroutines] Enable printing coroutine frame in debugger if program is compiled with -g

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

In D99179#2705552 <https://reviews.llvm.org/D99179#2705552>, @aprantl wrote:

> I'm mostly happy with this now. Do you think it makes sense to factor out all this code into a new file, so it doesn't get confused

Thanks for reviewing! I am a little confused about the sentence for 'factor out all this code into a new file'. Does it suggest me to move the code in this patch to outer of coroutine module as a basic infrastructure? Or does it suggest me to move these code into a new file in coroutine module?

If it means we need to move these codes as a basic infrastructure, it may not make sense due to all of us agree on we could only construct debug info in the special case coroutine.
If it means we need to move these codes to a new file in the coroutine module, my opinion is that it is more easy to read these codes if we keep them in the CoroFrame.cpp. Since the total lines of codes added in CoroFrame.cpp is about 370, I think it may be OK to remain these codes in the CoroFrame.cpp now. I have a plan to extract the analysis part of CoroFrame.cpp into Analysis module as an analysis pass. After that, the lines of codes in CoroFrame.cpp could be less than now. So it may not be so confusing.

In D99179#2705717 <https://reviews.llvm.org/D99179#2705717>, @dblaikie wrote:

> Ah, if the types and variables are always local to independent functions (please confirm that two of these entities can't appear in the same scope), that's fine by me.

Yes, the types (`__coro_frame_ty`) and variables (`__coro_frame`) are always local. Without inlining, there would be one and only coroutine frame in each coroutine. If inlining is turned on, there may be multiple coroutine frame in one function. And as far as I know, the debug info during inlining have been handled already. I mean, there wouldn't be two of `__coro_frame` appear in one DIScope.



================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:341
+    assert(Iter != FieldAlignMap.end());
+    return Iter->second;
+  }
----------------
aprantl wrote:
> What does this function add over `FieldAlignMap.lookup(V)`?
Compared with FieldAlignMap.lookup(V), this function add an assertion. I use this style to match the style in the context.


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

https://reviews.llvm.org/D99179