-
Notifications
You must be signed in to change notification settings - Fork 55
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
Add cmake support #15
Add cmake support #15
Conversation
also includes cmake -DBUILD_TESTING=ON <src_dir> is now required to compile tests. Tell me if you don't want it, I'll drop it. |
780058b
to
53a1c12
Compare
37 - async auto reset event tests-multi-threaded (SEGFAULT) This is because were running into sack overflow (on x64 only with this particular number of event), see my comment in this commit. |
Hi, thank you so much for your work. |
Ok |
53a1c12
to
e01e4e8
Compare
cmake/FindCoroutines.cmake
Outdated
@@ -115,6 +115,9 @@ check_cxx_compiler_flag(-fcoroutines _CXX_COROUTINES_SUPPORTS_CORO_FLAG) | |||
|
|||
if(_CXX_COROUTINES_SUPPORTS_MS_FLAG) | |||
set(_CXX_COROUTINES_EXTRA_FLAGS "/await") | |||
if(CMAKE_CL_64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there some documentation which versions of MSVC support the "/await:heapelide" flag? Are there old versions of MSVC which we could break by unconditionally setting the heapelide flag?
And do the new versions (16.8), which do not need the /await flag, need the heapelide flag?
I'd appreciate if we made sure we do not break anything by setting this flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot find any documentation about this hidden version of the /await flag, all I know is that MSVC 2019 does not complain about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we maybe, instead of checking for CMAKE_CL_64 (which is discouraged, according to CMake documentation), check for the existence of this flag using check_cxx_compiler_flag? That way we'd make sure nothing bad happens if some version of MSVC does not know it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know CMAKE_CL_64 was discouraged...
Yes it look like the better way to go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, however the /await:heapelide is also ok for 32bit target.
Do you know why have been added for 64bit targets only ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when using the heapelide version on 32bit, auto reset event test
also runs into SEGFAULT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, sorry it's ok, I was targetting wrong processor
I just tested an fixed install target:
|
- tests-main library changed to STATIC - MSVC dlls would require to do extra import/export effort
8c0a9a0
to
e0cd46a
Compare
Thanks a lot for your effort, we tested with MSVC 2019 Preview 16.8.5, works great! |
adds missing parts for MSVC support