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

Use TBB oneAPI #200

Merged
merged 14 commits into from
Oct 15, 2022
Merged

Use TBB oneAPI #200

merged 14 commits into from
Oct 15, 2022

Conversation

simogasp
Copy link
Member

@simogasp simogasp commented Oct 6, 2022

This PR is a rework of #178 to enable building with the new TBB oneAPI.
Contrary to the previous, this breaks the compatibility with previous versions of TBB, as the cmake now uses CONFIG.
Major changes:

  • update cmake for finding and linking with TBB oneAPI
  • code fixes to replace deprecated tbb mutex and locks, using those from the std library
  • update the docker configuration files
  • update the CI
  • update the doc

When merged we can make a patch release and submit a PR to vcpgk to bump the version as well. These changes are already taken into account in vcpkg by patches that do more or less the same thing.

@simogasp simogasp added this to the v1.0.3 milestone Oct 6, 2022
@simogasp
Copy link
Member Author

simogasp commented Oct 6, 2022

@p12tic there still 1 issue that I have to commit and it is related to this
Warlockbugs/cmangos-authserver@e09dcc9
Do you have any advice on the best way to do it? Is it better to add a

#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif

or is it better to add the definition in cmake with target_compile_definitions? Both work, I just don't know if there is a preferable way.

@p12tic
Copy link
Contributor

p12tic commented Oct 6, 2022

@simogasp I didn't see any problems with boost + aliceVision on macOS. What exact symptoms are you seeing?

@simogasp
Copy link
Member Author

simogasp commented Oct 6, 2022

I don't know if it depends on the version, but (on Silicon) I had a similar compiling error as boostorg/stacktrace#88
using boost 1.76

@simogasp
Copy link
Member Author

simogasp commented Oct 6, 2022

and they add this patch on vcpkg to the cctag port as well when they bumped the tbb version

@p12tic
Copy link
Contributor

p12tic commented Oct 6, 2022

Interesting, I'm also on Apple arm64 and boost 1.76. It could be that the reason why I'm not seeing problems is that currently I'm compiling macports environment which likely has different libraries.

@p12tic
Copy link
Contributor

p12tic commented Oct 6, 2022

Ah, I'm not actually compiling cctag component on macOS, sorry for the noise (facepalm).

Anyway, defining _GNU_SOURCE in this case shouldn't cause any problems.

@p12tic
Copy link
Contributor

p12tic commented Oct 6, 2022

@simogasp Appveyor build fails because of incorrect expected archive hash.

@simogasp
Copy link
Member Author

simogasp commented Oct 6, 2022

@simogasp Appveyor build fails because of incorrect expected archive hash.

yes it must be the cache, we should move to github actions like AV where we can keep control of vcpkg version, even though is less annoying here given the relatevelt fewer dependencies

@simogasp
Copy link
Member Author

simogasp commented Oct 6, 2022

ah yes this breaks the appveyor CI as oneAPI is not yet in the version of vcpkg shipped with the appveyor image.
We need to update vcpkg to a more recent version containing the oneAPI like it is explained here
https://www.appveyor.com/docs/lang/cpp/#vc-packaging-tool

Copy link
Member

@fabiencastan fabiencastan left a comment

Choose a reason for hiding this comment

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

Great PR. Just asking for one small change.

src/cctag/utils/Exceptions.hpp Show resolved Hide resolved
@simogasp simogasp merged commit 5e5cc5b into develop Oct 15, 2022
@simogasp simogasp deleted the dev/update_oneAPI branch October 15, 2022 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants