[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:345
+  void setAlign(Value *V, uint64_t Align) {
+    assert((LayoutIndexUpdateStarted || FieldAlignMap.count(V) == 0));
+    FieldAlignMap.insert({V, Align});
----------------
You will only be calling setAlign and setOffset once for each field, which is different from the index (index is set twice). So you should only be asserting count=0 here and also for setOffset.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:371-372
   DenseMap<Value *, uint32_t> FieldIndexMap;
+  DenseMap<Value *, uint64_t> FieldAlignMap;
+  DenseMap<Value *, uint64_t> FieldOffsetMap;
 };
----------------
Please add comment to the fields


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:890
+                                FrameDataInfo &FrameData) {
+  if (llvm::none_of(F.getParent()->debug_compile_units(), [](const auto &Iter) {
+        return dwarf::isCPlusPlus(
----------------
 How does `debug_compile_units` work? Could it contain a mix of C++ and other languages?


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:901
+
+  AllocaInst *PromiseAlloca = Shape.getPromiseAlloca();
+  if (!PromiseAlloca)
----------------
I am always curious, when would PromiseAlloca to not be present?


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:1474
   LLVMContext &C = CB->getContext();
   IRBuilder<> Builder(CB->getNextNode());
   StructType *FrameTy = Shape.FrameTy;
----------------
Builder should no longer be initialized with CB->getNextNode, but just the context


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:2640
+  createFramePtr(Shape);
+  // For now, this only works for C++ programs only.
+  buildFrameDebugInfo(F, Shape, FrameData);
----------------
redundant "only"


================
Comment at: llvm/lib/Transforms/Coroutines/CoroInternal.h:136-137
     unsigned IndexField;
+    unsigned IndexAlign;
+    unsigned IndexOffset;
     bool HasFinalSuspend;
----------------
Comment about the purpose


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

https://reviews.llvm.org/D99179