Clean up stdexec-only paths and update affected tests #7123
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -623,11 +623,17 @@ hpx_option( | |
|
|
||
| hpx_option( | ||
| HPX_WITH_STDEXEC BOOL | ||
| "Use STDEXEC executors instead of native HPX.(default: ON)" ON | ||
|
guptapratykshh marked this conversation as resolved.
Outdated
|
||
| CATEGORY "Executor" | ||
| ADVANCED | ||
| ) | ||
|
|
||
| if(NOT HPX_WITH_STDEXEC) | ||
| hpx_error( | ||
| "HPX_WITH_STDEXEC=OFF is no longer supported. HPX now requires stdexec." | ||
| ) | ||
| endif() | ||
|
There was a problem hiding this comment. The option help text still suggests there are two supported modes (stdexec vs native), but the configuration now hard-errors when
Contributor
There was a problem hiding this comment. I think we can remove the option altogether. |
||
|
|
||
| hpx_option( | ||
| HPX_WITH_FETCH_STDEXEC BOOL "Use FetchContent to fetch STDEXEC.(default: ON)" | ||
| ON | ||
|
|
@@ -2065,6 +2071,13 @@ if(WIN32) | |
| # Make sure that __cplusplus is properly defined | ||
| hpx_add_target_compile_option(-Zc:__cplusplus PUBLIC) | ||
|
|
||
| # stdexec requires the conforming MSVC preprocessor. | ||
| if(NOT (${CMAKE_CXX_COMPILER_ID} STREQUAL "Clang" | ||
| AND "x${CMAKE_CXX_SIMULATE_ID}" STREQUAL "xMSVC") | ||
| ) | ||
| hpx_add_target_compile_option(-Zc:preprocessor PUBLIC) | ||
| endif() | ||
|
|
||
| # Silence C++17 deprecation warnings | ||
| hpx_add_config_cond_define(_SILENCE_ALL_CXX17_DEPRECATION_WARNINGS) | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,83 +10,156 @@ if(STDEXEC_ROOT AND NOT Stdexec_ROOT) | |
| unset(STDEXEC_ROOT CACHE) | ||
| endif() | ||
|
|
||
| if(Stdexec_ROOT AND HPX_WITH_FETCH_STDEXEC) | ||
| hpx_warn( | ||
| "Both Stdexec_ROOT and HPX_WITH_FETCH_STDEXEC are provided. HPX_WITH_FETCH_STDEXEC will take precedence." | ||
| ) | ||
| endif() | ||
|
|
||
| function(_hpx_patch_stdexec_header header) | ||
| file(READ "${header}" _content) | ||
| set(_patched_content "${_content}") | ||
|
|
||
| # clang-scan-deps used for C++ modules currently rejects apostrophe-separated | ||
| # pp-number literals in stdexec. Remove digit separators in the fetched copy | ||
| # so dependency scanning succeeds. | ||
| set(_previous_content "") | ||
| while(NOT _patched_content STREQUAL _previous_content) | ||
| set(_previous_content "${_patched_content}") | ||
| string(REGEX REPLACE "([0-9])'([0-9])" "\\1\\2" _patched_content | ||
| "${_patched_content}" | ||
| ) | ||
| endwhile() | ||
|
Contributor
There was a problem hiding this comment. This should be changed in the upstream version Please create a PR for them.
Contributor
Author
There was a problem hiding this comment. Fixed upstream in NVIDIA/stdexec PR #2035. |
||
|
|
||
| if(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") | ||
| # stdexec's stop_token header currently declares _mm_pause without the C | ||
| # linkage used by MSVC's intrinsic headers, which breaks module builds. | ||
| string(REPLACE "extern void _mm_pause();" "extern \"C\" void _mm_pause();" | ||
| _patched_content "${_patched_content}" | ||
|
Contributor
There was a problem hiding this comment. This should be fixed in the upstream version as well.
Contributor
There was a problem hiding this comment. Why did you resolve this comment (and similar comments below)?
Contributor
Author
There was a problem hiding this comment. Fixed upstream in NVIDIA/stdexec PR #2030. |
||
| ) | ||
| endif() | ||
|
|
||
| # C++20 module interface units can emit duplicate definitions for a few | ||
| # namespace-scope stdexec constants when both hpx_core and hpx_full consume | ||
| # the same headers. Mark them inline in the fetched copy so the module | ||
| # archives do not export strong duplicate symbols. | ||
| string(REPLACE "constexpr struct __catch_any_lvalue_t" | ||
| "inline constexpr struct __catch_any_lvalue_t" | ||
| _patched_content "${_patched_content}" | ||
| ) | ||
| string(REPLACE "} __catch_any_lvalue{};" "} inline __catch_any_lvalue{};" | ||
| _patched_content "${_patched_content}" | ||
| ) | ||
| string(REPLACE "constexpr __none_such __no_default{};" | ||
| "inline constexpr __none_such __no_default{};" | ||
| _patched_content "${_patched_content}" | ||
| ) | ||
| string(REPLACE "constexpr std::size_t __npos = ~0UL;" | ||
| "inline constexpr std::size_t __npos = ~0UL;" _patched_content | ||
| "${_patched_content}" | ||
| ) | ||
| string(REPLACE "constexpr char __type_name_prefix[] = \"__xyzzy<\";" | ||
| "inline constexpr char __type_name_prefix[] = \"__xyzzy<\";" | ||
| _patched_content "${_patched_content}" | ||
| ) | ||
| string(REPLACE "constexpr char __type_name_suffix[] = \">::__plugh\";" | ||
| "inline constexpr char __type_name_suffix[] = \">::__plugh\";" | ||
| _patched_content "${_patched_content}" | ||
| ) | ||
| string(REPLACE "constexpr auto __start_next_fn =" | ||
| "inline constexpr auto __start_next_fn =" _patched_content | ||
| "${_patched_content}" | ||
| ) | ||
| string(REPLACE "constexpr size_t __default_buffer_size =" | ||
| "inline constexpr size_t __default_buffer_size =" | ||
| _patched_content "${_patched_content}" | ||
| ) | ||
| string(REPLACE "constexpr char const *__pure_virt_msg =" | ||
| "inline constexpr char const *__pure_virt_msg =" | ||
| _patched_content "${_patched_content}" | ||
| ) | ||
|
Contributor
There was a problem hiding this comment. This 'patching' is very brittle and will silently break if the source change. I think we should a) make the upstream accept this change, and b) find a better way of working around the issue you're facing.
Contributor
Author
There was a problem hiding this comment. Fixed upstream in NVIDIA/stdexec PR #2030. |
||
|
|
||
| if(NOT _content STREQUAL _patched_content) | ||
| file(WRITE "${header}" "${_patched_content}") | ||
| endif() | ||
| endfunction() | ||
|
guptapratykshh marked this conversation as resolved.
Outdated
|
||
|
|
||
| function(_hpx_patch_stdexec_headers include_dir) | ||
| if(HPX_WITH_CXX_MODULES AND (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" | ||
| OR CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") | ||
| ) | ||
| hpx_info("Patching stdexec headers for C++ modules dependency scanning") | ||
| file(GLOB_RECURSE stdexec_headers "${include_dir}/*.hpp") | ||
| foreach(stdexec_header IN LISTS stdexec_headers) | ||
| _hpx_patch_stdexec_header("${stdexec_header}") | ||
| endforeach() | ||
| endif() | ||
| endfunction() | ||
|
|
||
| if(HPX_WITH_FETCH_STDEXEC) | ||
| hpx_info( | ||
| "HPX_WITH_FETCH_STDEXEC=${HPX_WITH_FETCH_STDEXEC}, Stdexec will be fetched using CMake's FetchContent and installed alongside HPX (HPX_WITH_STDEXEC_TAG=${HPX_WITH_STDEXEC_TAG})" | ||
| ) | ||
|
|
||
| include(FetchContent) | ||
|
guptapratykshh marked this conversation as resolved.
|
||
| fetchcontent_declare( | ||
| Stdexec | ||
| GIT_REPOSITORY https://github.com/NVIDIA/stdexec.git | ||
| GIT_TAG ${HPX_WITH_STDEXEC_TAG} | ||
| ) | ||
|
|
||
| fetchcontent_getproperties(Stdexec) | ||
| if(NOT Stdexec_POPULATED) | ||
| fetchcontent_populate(Stdexec) | ||
| endif() | ||
| set(Stdexec_ROOT ${stdexec_SOURCE_DIR}) | ||
| _hpx_patch_stdexec_headers("${stdexec_SOURCE_DIR}/include") | ||
|
|
||
| add_library(Stdexec INTERFACE) | ||
| target_include_directories( | ||
| Stdexec SYSTEM INTERFACE $<BUILD_INTERFACE:${stdexec_SOURCE_DIR}/include> | ||
| $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}> | ||
| ) | ||
|
|
||
| install( | ||
| TARGETS Stdexec | ||
| EXPORT HPXStdexecTarget | ||
| COMPONENT core | ||
| ) | ||
|
|
||
| install( | ||
| DIRECTORY ${Stdexec_ROOT}/include/ | ||
| DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} | ||
| COMPONENT core | ||
| FILES_MATCHING | ||
| PATTERN "*.hpp" | ||
| ) | ||
|
|
||
| export( | ||
| TARGETS Stdexec | ||
| NAMESPACE Stdexec:: | ||
| FILE "${CMAKE_CURRENT_BINARY_DIR}/lib/cmake/${HPX_PACKAGE_NAME}/HPXStdexecTarget.cmake" | ||
| ) | ||
|
|
||
| install( | ||
| EXPORT HPXStdexecTarget | ||
| NAMESPACE Stdexec:: | ||
| FILE HPXStdexecTarget.cmake | ||
| DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${HPX_PACKAGE_NAME} | ||
| COMPONENT cmake | ||
| ) | ||
|
|
||
| # TODO: Enforce a single spelling | ||
| add_library(Stdexec::Stdexec ALIAS Stdexec) | ||
| add_library(STDEXEC::stdexec ALIAS Stdexec) | ||
| else() | ||
| find_package(Stdexec) | ||
|
|
||
| if(NOT Stdexec_FOUND) | ||
| hpx_error( | ||
| "Stdexec could not be found, please specify Stdexec_ROOT to point to the correct location or enable HPX_WITH_FETCH_STDEXEC" | ||
| ) | ||
| elseif(Stdexec_INCLUDE_DIR) | ||
| _hpx_patch_stdexec_headers("${Stdexec_INCLUDE_DIR}") | ||
|
guptapratykshh marked this conversation as resolved.
Outdated
|
||
| endif() | ||
| endif() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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 believe this is a leftover file and should have been removed. If you want to exclude certain tests from the CI, please add those here: https://github.com/TheHPXProject/hpx/blob/master/.github/workflows/exclude.targets