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

TBB Re-Configuration Fix, main branch (2024.10.30.) #759

Merged

Conversation

krasznaa
Copy link
Member

As discussed with @stephenswat earlier today, @paulgessinger's update in #682 revealed a deeper issue with our build.

What happened was that oneDPL would call find_package(TBB) to access TBB, when using the "dpcpp" backend. (The one we actually use.)

https://github.com/oneapi-src/oneDPL/blob/main/CMakeLists.txt#L163

This would:

  • Look for an external TBB, and also finding it as part of either oneAPI or the general setup that we would have for ROOT;
  • Set up the TBB_DIR cache variable, pointing at the external TBB version that was found.

This would work ~fine on the first CMake configuration. Even though at this point oneDPL would link against a different version of TBB than the one building together with the project. 😦 But on a re-configuration, we would now bump into:

https://github.com/oneapi-src/oneTBB/blob/v2021.7.0/CMakeLists.txt#L210

I.e. on this re-configuration we would completely abandon our own TBB version, and rather pick up the external version that was previously found by oneDPL.

To fix this, I had to do two things:

  • Upgrade to a newer version of TBB. One in which defining TBB_DIR by itself would not make it abandon setting up its own version of TBB. (In https://github.com/oneapi-src/oneTBB/blob/v2021.13.0/CMakeLists.txt#L236 the previous OR was changed to an AND.)
  • Make use of a new feature in FetchContent_Declare that makes any subsequent calls to find_package(TBB) be practically a no-op. This way ensuring that oneDPL would actually use the TBB version that we are building ourselves.

Yepp, some non-trivial CMake-ing going on here... 😛

P.S. I moved the setup of TBB before (Roc)Thrust, because those can also conditionally use TBB themselves. Even if in our current build they don't.

@krasznaa krasznaa added bug Something isn't working build This relates to the build system labels Oct 30, 2024
@krasznaa krasznaa force-pushed the TBBReconfigureFix-main-20241030 branch from 3745526 to 66694e6 Compare October 31, 2024 08:17
Copy link

sonarcloud bot commented Oct 31, 2024

@krasznaa
Copy link
Member Author

Let me also tag you Beomki, as fixing this would help my other developments quite a bit. 😉

Copy link
Member

@stephenswat stephenswat left a comment

Choose a reason for hiding this comment

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

Happy to approve this.

extern/tbb/CMakeLists.txt Show resolved Hide resolved
@stephenswat stephenswat merged commit 5337895 into acts-project:main Oct 31, 2024
26 checks passed
@krasznaa krasznaa deleted the TBBReconfigureFix-main-20241030 branch October 31, 2024 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build This relates to the build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants