[PATCH] D23844: [Coroutines] Part 9: Add cleanup subfunction.

[Gor Nishanov via llvm-commits]

GorNishanov marked 8 inline comments as done.

================
Comment at: docs/Coroutines.rst:934-935
@@ -929,2 +933,4 @@
 
-The third argument is `null` before coroutine is split, and later is replaced 
+The third argument is `null` coming out of the frontend. The CoroEarly pass sets
+this argument to point to the function this coro.id belongs to. 
+
----------------
GorNishanov wrote:
> majnemer wrote:
> > This is a little strange...
> It was one of the ways I thought of how we can detect whether a coro.id we are looking at describes the enclosing function or it describes a different function due to inline coroutine start function into its caller.
> 
> This used to be important in the design where I was stroring "elide-or-not" boolean in the coroutine frame. Since now we switched to separate f$destroy and f$cleanup, identifying whether coro.id corresponds to the current function or not is no longer important. 
> 
> I can take that parameter out altogether.
I'll keep if for now. We may run into another case where we will need it. 

================
Comment at: lib/Transforms/Coroutines/CoroElide.cpp:150-158
@@ +149,11 @@
+// leaves the function in some way.
+static bool returnsOrUnwindsToCaller(TerminatorInst *T) {
+  if (isa<ReturnInst>(T) || isa<ResumeInst>(T))
+    return true;
+
+  if (auto *CR = dyn_cast<CleanupReturnInst>(T))
+    return CR->unwindsToCaller();
+  if (auto *CS = dyn_cast<CatchSwitchInst>(T))
+    return CS->unwindsToCaller();
+
+  return false;
----------------
GorNishanov wrote:
> majnemer wrote:
> > This looks like a duplicated version of `Instruction::mayThrow`
> Cool. I can inline this function into shouldElide as:
> 
> isa<ReturnInst> || mayThrow(T)
This code was simplified and no longer require these.


https://reviews.llvm.org/D23844