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

Chuanqi Xu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 26 00:32:50 PDT 2022


ChuanqiXu added a comment.

Looks like there are many unrelated changes, it'd better to remove them.



================
Comment at: llvm/lib/Transforms/Coroutines/CoroEarly.cpp:41
 };
-}
+} // namespace
 
----------------
Unrelated change.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroEarly.cpp:69-70
   const DataLayout &DL = TheModule.getDataLayout();
-  int64_t Offset = alignTo(
-      DL.getStructLayout(SampleStruct)->getElementOffset(2), Alignment);
+  int64_t Offset =
+      alignTo(DL.getStructLayout(SampleStruct)->getElementOffset(2), Alignment);
   if (Intrin->isFromPromise())
----------------
Unrelated change. 

I guess you're using clang-format in a different way? Here is my way to do clang-format:

```
git diff -U0 --no-color --relative HEAD^ | clang/tools/clang-format/clang-format-diff.py -p1 -I
```

And set the clang-format at the trunk as the default clang-format.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroEarly.cpp:106
+  Module &M = *NoopFn->getParent();
+  if (!M.debug_compile_units().empty()) {
+    DICompileUnit *CU = *M.debug_compile_units_begin();
----------------
We prefer short indentation.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroEarly.cpp:114
+    auto *SP = DB.createFunction(
+        CU, /*Name=*/Name, /*LinkageName=*/Name, /*File*/ nullptr,
+        /*LineNo=*/0, SubroutineType, /*ScopeLine=*/0, DINode::FlagArtificial,
----------------
Why do we set `File` as nullptr here? I feel like any DIFile is better here.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroEarly.cpp:158-163
-      default:
-        continue;
-      case Intrinsic::coro_free:
-        CoroFrees.push_back(cast<CoroFreeInst>(&I));
-        break;
-      case Intrinsic::coro_suspend:
----------------
Unrelated changes. If we want to do such changes, it'd better to do it in a NFC patch.


================
Comment at: llvm/test/Transforms/Coroutines/coro-noop.ll:24
+
+; CHECK: ![[RESUME]] = distinct !DISubprogram(name: "_NoopCoro_ResumeDestroy", linkageName: "_NoopCoro_ResumeDestroy"
+
----------------
I guess we need to update the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132580



More information about the llvm-commits mailing list