Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable coroutine support for GCC 10.2 and earlier #466

Closed
wants to merge 1 commit into from

Conversation

ccotter
Copy link
Contributor

@ccotter ccotter commented Oct 5, 2022

GCC 10.2 coroutine support is not great. In particular, this version of GCC does not handle promise type constructors that accept the coroutine function parameters correctly. GCC passes a reference to the pre-copy object to the promise type constructor rather than a reference to the coroutine function argument after it's been copied to the coroutine frame. connect_awaitable relies on this working correctly, so we will require GCC 10.3 or newer to use coroutines with unifex.

Replaces #443

GCC 10.2 coroutine support is not great. In particular,
this version of GCC does not handle promise type constructors
that accept the coroutine function parameters correctly. GCC
passes a reference to the pre-copy object to the promise type
constructor rather than a reference to the coroutine function
argument after it's been copied to the coroutine frame.
connect_awaitable relies on this working correctly, so we will
require GCC 10.3 or newer to use coroutines with unifex.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 5, 2022

* GCC, 10.3 and later
* Clang, 10.x and later
* MSVC not currently supported
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Judging from unifex_flags.cmake, MSVC is not supported (explicitly disabled). I imagine MSVC these days does have better coroutine support. I don't use it/have access to MSVC myself.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lewissbaker may be able to answer the MSVC question.

I am OK with Clang, GCC targeting. Should we aim for versions supported by the compiler maintainers or current major and one previous major version?

As of today GCC lists 10.4 and Clang last released 10.x branch is 10.0.1 from Aug 2020.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We dropped older GCC support in #576, should be OK to close now.

coroutine support are required:

* GCC, 10.3 and later
* Clang, 10.x and later
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not sure what version of clang would be sufficient to be considered "stable" to use coroutines with libunifex, so I've left it at the minimum version of support for libunifex as stated above.

@ccotter
Copy link
Contributor Author

ccotter commented Nov 8, 2023

Closing per #576

@ccotter ccotter closed this Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants