[PATCH] D23586: [Coroutines] Part 8: Coroutine Frame Building algorithm

[David Majnemer via llvm-commits]

majnemer accepted this revision.
majnemer added a comment.
This revision is now accepted and ready to land.

LGTM with nits addressed.


================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:95-98
@@ +94,6 @@
+
+#ifndef NDEBUG
+  void dump() const;
+  void dump(StringRef Label, BitVector const &BV) const;
+#endif
+
----------------
I wouldn't bother with these, they'll get removed by the linker in a release build anyway.

================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:137-145
@@ +136,11 @@
+#ifndef NDEBUG
+void SuspendCrossingInfo::dump(StringRef Label, BitVector const &BV) const {
+  dbgs() << Label << ":";
+  for (size_t I = 0, N = BV.size(); I < N; ++I)
+    if (BV[I])
+      dbgs() << " " << Mapping.indexToBlock(I)->getName();
+  dbgs() << "\n";
+}
+
+void SuspendCrossingInfo::dump() const {
+  for (size_t I = 0, N = Block.size(); I < N; ++I) {
----------------
I'd mark these `LLVM_DUMP_METHOD`.

================
Comment at: lib/Transforms/Coroutines/CoroFrame.cpp:626-638
@@ +625,15 @@
+
+  for (Instruction &I : instructions(F)) {
+    // token returned by CoroSave is an artifact of how we build save/suspend
+    // pairs and should not be part of the Coroutine Frame
+    if (isa<CoroSaveInst>(&I))
+      continue;
+    // CoroBeginInst returns a handle to a coroutine which is passed as a sole
+    // parameter to .resume and .cleanup parts and should not go into coroutine
+    // frame.
+    if (isa<CoroBeginInst>(&I))
+      continue;
+
+    for (User *U : I.users())
+      if (Checker.isDefinitionAcrossSuspend(I, U)) {
+        assert(!materializable(I) &&
----------------
Can an EHPad be across a suspend?


https://reviews.llvm.org/D23586