ChuanqiXu updated this revision to Diff 392669. ChuanqiXu added a comment. Address comments
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115219/new/ https://reviews.llvm.org/D115219 Files: llvm/docs/Coroutines.rst llvm/lib/Transforms/Coroutines/CoroSplit.cpp llvm/test/Transforms/Coroutines/coro-split-eh-00.ll llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
Index: llvm/test/Transforms/Coroutines/coro-split-eh-01.ll =================================================================== --- llvm/test/Transforms/Coroutines/coro-split-eh-01.ll +++ llvm/test/Transforms/Coroutines/coro-split-eh-01.ll @@ -61,6 +61,9 @@ ; CHECK: lpad: ; CHECK-NEXT: %tok = cleanuppad within none [] ; CHECK-NEXT: call void @print(i32 2) +; Checks that the coroutine would be marked as done if it exits in unwinding path. +; CHECK-NEXT: %[[RESUME_ADDR:.+]] = getelementptr inbounds %[[FRAME_TY:.+]], %[[FRAME_TY]]* %FramePtr, i32 0, i32 0 +; CHECK-NEXT: store void (%[[FRAME_TY]]*)* null, void (%[[FRAME_TY]]*)** %[[RESUME_ADDR]], align 8 ; CHECK-NEXT: cleanupret from %tok unwind to caller declare i8* @llvm.coro.free(token, i8*) Index: llvm/test/Transforms/Coroutines/coro-split-eh-00.ll =================================================================== --- llvm/test/Transforms/Coroutines/coro-split-eh-00.ll +++ llvm/test/Transforms/Coroutines/coro-split-eh-00.ll @@ -67,6 +67,9 @@ ; CHECK-NEXT: %lpval = landingpad { i8*, i32 } ; CHECK-NEXT: cleanup ; CHECK-NEXT: call void @print(i32 2) +; Checks that the coroutine would be marked as done if it exits in unwinding path. +; CHECK-NEXT: %[[RESUME_ADDR:.+]] = getelementptr inbounds %[[FRAME_TY:.+]], %[[FRAME_TY]]* %FramePtr, i32 0, i32 0 +; CHECK-NEXT: store void (%[[FRAME_TY]]*)* null, void (%[[FRAME_TY]]*)** %[[RESUME_ADDR]], align 8 ; CHECK-NEXT: resume { i8*, i32 } %lpval declare i8* @llvm.coro.free(token, i8*) Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp =================================================================== --- llvm/lib/Transforms/Coroutines/CoroSplit.cpp +++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp @@ -280,6 +280,27 @@ BB->getTerminator()->eraseFromParent(); } +// Mark a coroutine as done, which implies that the coroutine is finished and +// never get resumed. +// +// In resume-switched ABI, the done state is represented by storing zero in +// ResumeFnAddr. +// +// NOTE: We couldn't omit the argument `FramePtr`. It is necessary because the +// pointer to the frame in splitted function is not stored in `Shape`. +static void markCoroutineAsDone(IRBuilder<> &Builder, const coro::Shape &Shape, + Value *FramePtr) { + assert( + Shape.ABI == coro::ABI::Switch && + "markCoroutineAsDone is only supported for Switch-Resumed ABI for now."); + auto *GepIndex = Builder.CreateStructGEP( + Shape.FrameTy, FramePtr, coro::Shape::SwitchFieldIndex::Resume, + "ResumeFn.addr"); + auto *NullPtr = ConstantPointerNull::get(cast<PointerType>( + Shape.FrameTy->getTypeAtIndex(coro::Shape::SwitchFieldIndex::Resume))); + Builder.CreateStore(NullPtr, GepIndex); +} + /// Replace an unwind call to llvm.coro.end. static void replaceUnwindCoroEnd(AnyCoroEndInst *End, const coro::Shape &Shape, Value *FramePtr, bool InResume, @@ -288,10 +309,18 @@ switch (Shape.ABI) { // In switch-lowering, this does nothing in the main function. - case coro::ABI::Switch: + case coro::ABI::Switch: { + // In C++'s specification, the coroutine should be marked as done + // if promise.unhandled_exception() throws. The frontend will + // call coro.end(true) along this path. + // + // FIXME: We should refactor this once there is other language + // which uses Switch-Resumed style other than C++. + markCoroutineAsDone(Builder, Shape, FramePtr); if (!InResume) return; break; + } // In async lowering this does nothing. case coro::ABI::Async: break; @@ -364,13 +393,9 @@ auto *Save = S->getCoroSave(); Builder.SetInsertPoint(Save); if (S->isFinal()) { - // Final suspend point is represented by storing zero in ResumeFnAddr. - auto *GepIndex = Builder.CreateStructGEP(FrameTy, FramePtr, - coro::Shape::SwitchFieldIndex::Resume, - "ResumeFn.addr"); - auto *NullPtr = ConstantPointerNull::get(cast<PointerType>( - FrameTy->getTypeAtIndex(coro::Shape::SwitchFieldIndex::Resume))); - Builder.CreateStore(NullPtr, GepIndex); + // The coroutine should be marked done if it reaches the final suspend + // point. + markCoroutineAsDone(Builder, Shape, FramePtr); } else { auto *GepIndex = Builder.CreateStructGEP( FrameTy, FramePtr, Shape.getSwitchIndexField(), "index.addr"); Index: llvm/docs/Coroutines.rst =================================================================== --- llvm/docs/Coroutines.rst +++ llvm/docs/Coroutines.rst @@ -1377,17 +1377,22 @@ ``cleanupret from %tok unwind to caller`` before the `coro.end`_ intrinsic and will remove the rest of the block. +The `coro.end` intrinsic in the unwinding path would mark the coroutine as done. +The `coro.end` intrinsic in the normal path wouldn't do this since the coroutine +should already be marked as done by final suspend. + The following table summarizes the handling of `coro.end`_ intrinsic. -+--------------------------+-------------------+-------------------------------+ -| | In Start Function | In Resume/Destroy Functions | -+--------------------------+-------------------+-------------------------------+ -|unwind=false | nothing |``ret void`` | -+------------+-------------+-------------------+-------------------------------+ -| | WinEH | nothing |``cleanupret unwind to caller``| -|unwind=true +-------------+-------------------+-------------------------------+ -| | Landingpad | nothing | nothing | -+------------+-------------+-------------------+-------------------------------+ ++--------------------------+------------------------+-------------------------------+ +| | In Start Function | In Resume/Destroy Functions | ++--------------------------+------------------------+-------------------------------+ +|unwind=false | nothing |``ret void`` | ++------------+-------------+------------------------+-------------------------------+ +| | WinEH | mark coroutine as done |``cleanupret unwind to caller``| +| | | |mark coroutine done | +|unwind=true +-------------+------------------------+-------------------------------+ +| | Landingpad | mark coroutine as done | mark coroutine done | ++------------+-------------+------------------------+-------------------------------+ 'llvm.coro.end.async' Intrinsic
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits