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

Detect C11 threads in CMake #5034

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

hyoklee
Copy link
Member

@hyoklee hyoklee commented Oct 29, 2024

v2.0.0 uses C11.

@jhendersonHDF
Copy link
Collaborator

May need input from @qkoziol

@derobins derobins added Priority - 1. High 🔼 These are important issues that should be resolved in the next release Type - Improvement Improvements that don't add a new feature or functionality labels Nov 4, 2024
@derobins
Copy link
Member

derobins commented Nov 4, 2024

The reason this is failing is that we're turning Pthreads on/off via the presence of the pthreads.h header, which is checked elsewhere in CMake. If we pick C11 threads here and then also have H5_HAVE_PTHREAD_H set, the ifdefs in H5TS are likely to be confused.

The Autotools are probably fine since it looks like configure.ac is smarter about checking for the Pthreads header, but ideally there should be an H5_USE_POSIX_THREADS symbol that is set independently of H5_HAVE_PTHREADS_H and used in the H5TS code (and the other thread options should be renamed to USE vs HAVE, IMHO). If you do that here, be sure to alert @qkoziol since he'll have to update his branches.

@qkoziol
Copy link
Contributor

qkoziol commented Nov 4, 2024

The reason this is failing is that we're turning Pthreads on/off via the presence of the pthreads.h header, which is checked elsewhere in CMake. If we pick C11 threads here and then also have H5_HAVE_PTHREAD_H set, the ifdefs in H5TS are likely to be confused.

The Autotools are probably fine since it looks like configure.ac is smarter about checking for the Pthreads header, but ideally there should be an H5_USE_POSIX_THREADS symbol that is set independently of H5_HAVE_PTHREADS_H and used in the H5TS code (and the other thread options should be renamed to USE vs HAVE, IMHO). If you do that here, be sure to alert @qkoziol since he'll have to update his branches.

I like this idea. I'm tied up with something else for a few days, but could tackle it later this week or early next, f someone else doesn't get to it first.

@derobins derobins changed the title Enable threads for Windows Detect C11 threads in CMake Nov 8, 2024
@seanm
Copy link
Contributor

seanm commented Nov 20, 2024

If I'm not mistaken, even the newest versions of macOS / Xcode do not support C11 threads.

@qkoziol
Copy link
Contributor

qkoziol commented Nov 26, 2024

If I'm not mistaken, even the newest versions of macOS / Xcode do not support C11 threads.

Agree

@qkoziol
Copy link
Contributor

qkoziol commented Nov 26, 2024

I gave a try at making this work on MacOS last week and ultimately failed. It may be possible, but it seems difficult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Build CMake, Autotools Priority - 1. High 🔼 These are important issues that should be resolved in the next release Type - Improvement Improvements that don't add a new feature or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants