ChuanqiXu9 wrote:
Oh, it looks like some bots are not happy. I send the PR at
https://github.com/llvm/llvm-project/pull/93167
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ChuanqiXu9 wrote:
I land it directly given it looks straightforward and trivial in
https://github.com/llvm/llvm-project/commit/31f1590e4fb324c43dc36199587c453e27b6f6fa
See the commit message if you're interested.
https://github.com/llvm/llvm-project/pull/89751
ChuanqiXu9 wrote:
It is pretty interesting that I can pass the test by:
```
$git diff -U10
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 450ea8234371..003bfbf7138a 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++
zmodem wrote:
Thanks, let me know if I can help.
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ChuanqiXu9 wrote:
I received some reports for this patch breaking some codes. I am reproducing
it. It looks like related to exceptions.
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/zmodem closed https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -206,14 +210,37 @@ static void lowerAwaitSuspend(IRBuilder<> ,
CoroAwaitSuspendInst *CB) {
llvm_unreachable("Unexpected coro_await_suspend invocation method");
}
+ if (CB->getCalledFunction()->getIntrinsicID() ==
+ Intrinsic::coro_await_suspend_handle) {
+
@@ -206,14 +210,37 @@ static void lowerAwaitSuspend(IRBuilder<> ,
CoroAwaitSuspendInst *CB) {
llvm_unreachable("Unexpected coro_await_suspend invocation method");
}
+ if (CB->getCalledFunction()->getIntrinsicID() ==
+ Intrinsic::coro_await_suspend_handle) {
+
@@ -206,14 +210,37 @@ static void lowerAwaitSuspend(IRBuilder<> ,
CoroAwaitSuspendInst *CB) {
llvm_unreachable("Unexpected coro_await_suspend invocation method");
}
+ if (CB->getCalledFunction()->getIntrinsicID() ==
+ Intrinsic::coro_await_suspend_handle) {
+
@@ -206,14 +210,37 @@ static void lowerAwaitSuspend(IRBuilder<> ,
CoroAwaitSuspendInst *CB) {
llvm_unreachable("Unexpected coro_await_suspend invocation method");
}
+ if (CB->getCalledFunction()->getIntrinsicID() ==
+ Intrinsic::coro_await_suspend_handle) {
+
https://github.com/ChuanqiXu9 approved this pull request.
LGTM with a comment.
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/mtrofin approved this pull request.
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mtrofin wrote:
> I think my patch is a significant improvement, both in terms of simplicity
> and reliability of the codegen for symmetric transfer, and would like to move
> forward. @ChuanqiXu9 @mtrofin do you have any further comments?
Nothing on my side, lgtm for my narrow concern. Please
zmodem wrote:
I think my patch is a significant improvement, both in terms of simplicity and
reliability of the codegen for symmetric transfer, and would like to move
forward. @ChuanqiXu9 @mtrofin do you have any further comments?
https://github.com/llvm/llvm-project/pull/89751
https://github.com/zmodem edited https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -40,10 +38,8 @@ exit:
; Verify that in the resume part resume call is marked with musttail.
; CHECK-LABEL: @f.resume(
-; CHECK: %[[addr2:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-; NOPGO-NEXT: musttail call fastcc void %[[addr2]](ptr null)
-; PGO: call void
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
zmodem wrote:
I've cleaned up the patch a bit and added some comments. Please take a look.
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -40,10 +38,8 @@ exit:
; Verify that in the resume part resume call is marked with musttail.
; CHECK-LABEL: @f.resume(
-; CHECK: %[[addr2:.+]] = call ptr @llvm.coro.subfn.addr(ptr null, i8 0)
-; NOPGO-NEXT: musttail call fastcc void %[[addr2]](ptr null)
-; PGO: call void
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
zmodem wrote:
With my patch, some of these tests become less interesting. The class of
problems arising from instructions between the resume and suspend doesn't exist
with the new lowering. We could probably drop coro-split-musttail{5,6,7}.ll?
zmodem wrote:
I wasn't able to figure out what this test does. There is a Clang level test
which was added at the same time:
llvm/test/Transforms/Coroutines/coro-preserve-final.ll presumable that's enough.
https://github.com/llvm/llvm-project/pull/89751
zmodem wrote:
This was testing that "the lifetime of the coroutine handle used to obtain
the address is contained within single basic block, and hence does not
live across suspension points"
Since we no longer separately obtain the handle and pass it to
@@ -1056,6 +1083,25 @@ void CoroCloner::create() {
// Set up the new entry block.
replaceEntryBlock();
+ // Turn symmetric transfers into musttail calls.
+ for (CallInst *ResumeCall : Shape.SymmetricTransfers) {
+ResumeCall = cast(VMap[ResumeCall]);
+
zmodem wrote:
> The higher level idea looks fine.
Thanks!
> A detail is that, in this patch, we must not mark
> `llvm.coro.await.suspend.handle` as nounwind. Previously,
> `llvm.coro.await.suspend.handle` may be marked as nounwind if the
> `await_suspend` is noexcept. But we can't do it in
@@ -1523,24 +1442,16 @@ struct SwitchCoroutineSplitter {
createResumeEntryBlock(F, Shape);
auto *ResumeClone =
-createClone(F, ".resume", Shape, CoroCloner::Kind::SwitchResume);
+createClone(F, ".resume", Shape, CoroCloner::Kind::SwitchResume, TTI);
https://github.com/zmodem updated
https://github.com/llvm/llvm-project/pull/89751
>From 33b07efe6d68cb4d17e96349b552ef5e5901d8c6 Mon Sep 17 00:00:00 2001
From: Hans Wennborg
Date: Tue, 26 Mar 2024 15:04:35 +0100
Subject: [PATCH 1/4] stuff
---
clang/lib/CodeGen/CGCoroutine.cpp |
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/ChuanqiXu9 commented:
The higher level idea looks fine.
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1523,24 +1442,16 @@ struct SwitchCoroutineSplitter {
createResumeEntryBlock(F, Shape);
auto *ResumeClone =
-createClone(F, ".resume", Shape, CoroCloner::Kind::SwitchResume);
+createClone(F, ".resume", Shape, CoroCloner::Kind::SwitchResume, TTI);
https://github.com/ChuanqiXu9 edited
https://github.com/llvm/llvm-project/pull/89751
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zmodem wrote:
> Does this address
> https://discourse.llvm.org/t/coro-pre-split-handling-of-the-suspend-edge/75043?
> Could you add a note there in that direction - a few folks were looking at
> going at the direction @jyknight was suggesting there, and it'd be good to
> have closure on the
mtrofin wrote:
Does this address
https://discourse.llvm.org/t/coro-pre-split-handling-of-the-suspend-edge/75043?
Could you add a note there in that direction - a few folks were looking at
going at the direction @jyknight was suggesting there, and it'd be good to have
closure on the topic.
https://github.com/zmodem updated
https://github.com/llvm/llvm-project/pull/89751
>From 33b07efe6d68cb4d17e96349b552ef5e5901d8c6 Mon Sep 17 00:00:00 2001
From: Hans Wennborg
Date: Tue, 26 Mar 2024 15:04:35 +0100
Subject: [PATCH 1/2] stuff
---
clang/lib/CodeGen/CGCoroutine.cpp |
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff c52b18d1e41107067b7557d8af3a06e6fe0beb0f
33b07efe6d68cb4d17e96349b552ef5e5901d8c6 --
zmodem wrote:
What do you think?
(The test updates are a bit hacky for now, but I wanted to get feedback before
spending more time on them. Also I think many of the tests become uninteresting
after this patch and could probably be deleted.)
https://github.com/llvm/llvm-project/pull/89751
llvmbot wrote:
@llvm/pr-subscribers-coroutines
Author: Hans (zmodem)
Changes
The C++ standard requires that symmetric transfer from one coroutine to another
is performed via a tail call. Failure to do so is a miscompile and often breaks
programs by quickly overflowing the stack.
Until
https://github.com/zmodem created
https://github.com/llvm/llvm-project/pull/89751
The C++ standard requires that symmetric transfer from one coroutine to another
is performed via a tail call. Failure to do so is a miscompile and often breaks
programs by quickly overflowing the stack.
Until
50 matches
Mail list logo
