[PATCH] D108122: FunctionAttrs: do not mark coroutines with odd return path `noreturn`

[Tim Northover via Phabricator via llvm-commits]

t.p.northover created this revision.
t.p.northover added a reviewer: aschwaighofer.
Herald added subscribers: ormris, ChuanqiXu, lxfind, hiraditya, mcrosier.
t.p.northover requested review of this revision.
Herald added a project: LLVM.

Under one of the ABIs we support for coroutines, a call to `@llvm.coro.end` may actually (despite not being a BB terminator) return from the function. This is weird, but documented in https://llvm.org/docs/Coroutines.html#llvm-coro-end-intrinsic:

> In returned-continuation lowering, llvm.coro.end fully destroys the coroutine frame. If the second argument is false, it also returns from the coroutine with a null continuation pointer, and the next instruction will be unreachable.

In this case, because of the `unreachable` at the end we'd normally decide the block cannot return (and if it's the only return block in the function, that the function itself is `noreturn), but this logic needs revising.


https://reviews.llvm.org/D108122

Files:
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/test/Transforms/FunctionAttrs/noreturn.ll


Index: llvm/test/Transforms/FunctionAttrs/noreturn.ll
===================================================================
--- llvm/test/Transforms/FunctionAttrs/noreturn.ll
+++ llvm/test/Transforms/FunctionAttrs/noreturn.ll
@@ -71,3 +71,20 @@
   call i32 @f() noreturn
   ret void
 }
+
+; CHECK: Function Attrs: {{.*}}noreturn
+; CHECK-NEXT: @unreachable
+define void @unreachable() {
+  unreachable
+}
+
+; CHECK-NOT: Function Attrs: {{.*}}noreturn
+; CHECK: @coro
+define void @coro() {
+  call token @llvm.coro.id.retcon.once(i32 0, i32 0, i8* null, i8* bitcast(void() *@coro to i8*), i8* null, i8* null)
+  call i1 @llvm.coro.end(i8* null, i1 false)
+  unreachable
+}
+
+declare token @llvm.coro.id.retcon.once(i32 %size, i32 %align, i8* %buffer, i8* %prototype, i8* %alloc, i8* %free)
+declare i1 @llvm.coro.end(i8*, i1)
Index: llvm/lib/Transforms/IPO/FunctionAttrs.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -1404,9 +1404,25 @@
 // A basic block can only return if it terminates with a ReturnInst and does not
 // contain calls to noreturn functions.
 static bool basicBlockCanReturn(BasicBlock &BB) {
-  if (!isa<ReturnInst>(BB.getTerminator()))
+  if (!isa<ReturnInst>(BB.getTerminator()) &&
+      !isa<UnreachableInst>(BB.getTerminator()))
     return false;
-  return none_of(BB, instructionDoesNotReturn);
+
+  for (auto &I : BB) {
+    if (instructionDoesNotReturn(I))
+      return false;
+
+    // Calls to @llvm.coro.end(..., i1 false) are themselves a return operation
+    // if the coroutine is retcon, so conservatively assume all are.
+    if (auto *CB = dyn_cast<CallBase>(&I)) {
+      if (CB->getCalledFunction() &&
+          CB->getCalledFunction()->getIntrinsicID() == Intrinsic::coro_end &&
+          cast<ConstantInt>(CB->getOperand(1))->isZero())
+        return true;
+    }
+  }
+
+  return isa<ReturnInst>(BB.getTerminator());
 }
 
 // Set the noreturn function attribute if possible.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D108122.366593.patch
Type: text/x-patch
Size: 2043 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210816/44a7eebc/attachment.bin>