[NATIVECPU] NativeCPU with optional oneTBB backend #2627
Conversation
| # release tbbbind | ||
| if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG") | ||
| target_compile_options(tbb PRIVATE -U_DEBUG) | ||
| endif() |
There was a problem hiding this comment.
The root cause for the complication here and before is including oneTBB with FetchContent, which processes it according to the current project's CMake rules, and then requires extra work when unified-runtime's CMake rules are no good for oneTBB. Is it possible to include it using ExternalProject_Add instead?
There was a problem hiding this comment.
Probably, but this may require some more rework and retesting, and the current FetchContent integration is working and passing the DPCPP CI so I'd recommend to revisit this in a follow-up PR if need be. Also, the latest Cmake "Using Dependencies Guide" doesn't seem to mention ExternalProject, so it's not clear how recommended it is.
There was a problem hiding this comment.
True, but the "Using Dependencies Guide" only gives instructions on how to either use pre-built packages, or how to include an external project's sources as this project's sources. Neither works for us and the result is that we need workarounds to include oneTBB's sources as this project's sources, except make it mostly, but not entirely, as if they are a different project's sources, because our project's configuration is not what we want for oneTBB.
But if we do not have an easy way of using an alternative, if it would be too much work to use ExternalProject_Add or some other alternatives, then sure, we can delay that until later.
There was a problem hiding this comment.
The comment looks like it does not match what the code does, this does not build oneTBB in release mode, this only builds oneTBB without assertions. In debug builds with no (or minimal?) optimizations, this would build oneTBB with no/minimal optimizations as well, whereas release mode would enable aggressive optimizations. Is that intended? If it is, that is fine but can you update the comment to match? If it is not intended, some more work would be needed to really build in release mode. From what I can find, setting CMAKE_BUILD_TYPE should be the most reliable way of doing that with single-config generators (e.g. Ninja). For multi-config generators (e.g. MSVC), I think we should still have access to CMAKE_CXX_FLAGS_RELEASE to add?
| #Date: Fri Jan 24 23:23:59 2025 +0200 | ||
| # Add explicit deduction guides for blocked_nd_range (#1525) | ||
| GIT_TAG 9d4578723827f31defd79389819a5fbf659577f7 | ||
| CMAKE_ARGS "-DTBB_TEST:BOOL=OFF -DTBB_EXAMPLES:BOOL=OFF -DTBB_BENCH:BOOL=OFF" |
There was a problem hiding this comment.
If we keep FetchContent: this line has no effect as far as I can see, CMake does not get called for oneTBB, so there is nothing that parses CMake args. This is why you needed to set TBB_TEST etc. again below despite including them here.
This is harmless though so I'm okay with leaving this as is unless other changes are needed as well.
Unified Runtime -> intel/llvm Repo Move NoticeInformationThe source code of Unified Runtime has been moved to intel/llvm under the unified-runtime top-level directory, The code will be mirrored to oneapi-src/unified-runtime and the specification will continue to be hosted at oneapi-src.github.io/unified-runtime. The contribution guide has been updated with new instructions for contributing to Unified Runtime. PR MigrationAll open PRs including this one will be labelled auto-close and shall be automatically closed after 30 days. Should you wish to continue with your PR you will need to migrate it to intel/llvm. This is an automated comment. |
Unified Runtime -> intel/llvm Repo Move NoticeFollowing on from the previous notice, we have now enabled workflows to automatically label and close PRs because the Unified Runtime source code has moved to intel/llvm. This PR has now been marked with the Please review the previous notice for more information, including assistance with migrating your PR to intel/llvm. Should there be a reason for this PR to remain open, manually remove the This is an automated comment. |
Initial PR to add option for Native CPU to use oneTBB as its backend.
Corresponding DPC++ PR: intel/llvm#16803