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

[Xun Li via Phabricator via llvm-commits]

lxfind added inline comments.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:894
+                                FrameDataInfo &FrameData) {
+  if (!llvm::count_if(F.getParent()->debug_compile_units(), [](auto *Iter) {
+        return dwarf::isCPlusPlus(
----------------
Does this mean that if there is any C++ files, we don't build debug info?


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:907
+  assert(PromiseAlloca &&
+         "Coroutine with switch ABI should own Promise alloca");
+
----------------
Sorry I didn't mean to ask for this change, but more of curiosity, I see that in many places we check whether PromiseAlloca is nullptr under switch-lowering. So I wonder if there exists legit cases where PromiseAloca is null?


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:1474
   LLVMContext &C = CB->getContext();
   IRBuilder<> Builder(CB->getNextNode());
   StructType *FrameTy = Shape.FrameTy;
----------------
ChuanqiXu wrote:
> lxfind wrote:
> > Builder should no longer be initialized with CB->getNextNode, but just the context
> `IRBuilder<> Builder(CB->getNextNode())` tells where builder should insert instructions. I think it is not redundant.
If you look closely, this insert position is never used. In fact, it's confusing to set the insert position to the next instruction of CoroBegin after this patch, because now the next instruction of CoroBegin is the bitcast of the frame pointer. You don't want to insert anything before the frame pointer construction anyway.


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

https://reviews.llvm.org/D99179