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

COMP: add FindTBB.cmake from VTK #991

Closed

Conversation

dyollb
Copy link
Contributor

@dyollb dyollb commented Jun 4, 2019

  • this allows ITK to find TBB also on Windows (TBBConfig.cmake is not generated on Windows)
  • I tried to make it backwards compatible, i.e. use the current mechanism (find TBBConfig.cmake), and only if it fails use the FindTBB.cmake

PR Checklist

Refer to the ITK Software Guide for
further development details if necessary.

- this allows ITK to find TBB also on Windows (TBBConfig.cmake is not generated on Windows)
- I tried to make it backwards compatible, i.e. use the current mechanism (find TBBConfig.cmake), and only if it fails use the FindTBB.cmake
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

I will also test this on my computer.

set(COMPILER_PREFIX "vc11")
elseif(MSVC_VERSION EQUAL 1800)
set(COMPILER_PREFIX "vc12")
elseif(MSVC_VERSION EQUAL 1900)
Copy link
Member

Choose a reason for hiding this comment

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

VS2019 is missing:
1910-1919 = VS 15.0 (v141 toolset)

Copy link
Member

Choose a reason for hiding this comment

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

Missing 1920 (Visual Studio 2019 RTW (16.0)). Both 15.0 and 16.0 use vc14 folder.

Copy link
Member

Choose a reason for hiding this comment

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

@dyollb or @dzenanz can we please add these to get this in?

Copy link
Contributor Author

@dyollb dyollb Jun 21, 2019

Choose a reason for hiding this comment

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

@dyollb or @dzenanz can we please add these to get this in?

I could add lines like:

elseif(MSVC_VERSION GREATER 1900 AND MSVC_VERSION LESS 1920)
  set(COMPILER_PREFIX "vc15")
elseif(MSVC_VERSION EQUAL 1920 OR MSVC_VERSION EQUAL 1921)
  set(COMPILER_PREFIX "vc16")

But the most recent pre-compiled version of TBB does not come with those folders (yet?).

Copy link
Member

Choose a reason for hiding this comment

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

The newer versions all use vc14's compiled libraries, because they are binary compatible with it. So put something like:

elseif(MSVC_VERSION GREATER 1900 AND MSVC_VERSION LESS 1920)
  set(COMPILER_PREFIX "vc14")
elseif(MSVC_VERSION EQUAL 1920 OR MSVC_VERSION EQUAL 1921)
  set(COMPILER_PREFIX "vc14")

or:

elseif(MSVC_VERSION GREATER 1800 AND MSVC_VERSION LESS 1929) 
  set(COMPILER_PREFIX "vc14") # expect all compilers in 192X series to have same binary compatibility as 1920 and 1921.

@dzenanz
Copy link
Member

dzenanz commented Jun 6, 2019

I tried it on my computer, and it either worked or was not triggered due to the existence of TBBConfig.cmake.

@Hrach97
Copy link

Hrach97 commented Jun 14, 2019

Hi,

I am trying to build ITK with TBB.
In contrast of 4.13 version 5.0.0 gives TBB_DIR variable.
Do you have any idea which path I have to give to that variable?

Thank you,
Hach

@dzenanz
Copy link
Member

dzenanz commented Jun 14, 2019

On my computer, TBB_DIR is equal to C:/Libs/tbb2019_20190410oss/cmake. I am using Intel's binary distribution.

@dzenanz
Copy link
Member

dzenanz commented Jun 14, 2019

As reported on the forum, this patch does not work as expected.

@dyollb
Copy link
Contributor Author

dyollb commented Jun 21, 2019

As reported on the forum, this patch does not work as expected.

Commented in the forum. I don't understand what nm97 means by "run examples". I am happy to spend a bit more time to improve/fix the patch.

@dyollb
Copy link
Contributor Author

dyollb commented Jun 21, 2019

I tried it on my computer, and it either worked or was not triggered due to the existence of TBBConfig.cmake.

Initially, I added a message to the FindTBB.cmake, to debug if cmake would enter into that file. It did for my build of TBB.

@thewtex thewtex requested a review from sjh26 August 20, 2019 15:19
@sjh26
Copy link
Contributor

sjh26 commented Aug 20, 2019

Testing ....

@seanm
Copy link
Contributor

seanm commented Oct 29, 2019

CC: @seanm @vfonov

I stumbled upon this PR trying to figure out how to build ITK with TBB. I succeeded to build VTK with TBB, I just had to provide a path for each lib and the headers folder. But for ITK, it seems to want me to point to TBBConfig.cmake which doesn't seem to exist when one builds TBB oneself. I need to build TBB myself because I need to have ASan and TSan versions of it.

Is the purpose of this patch to make ITK work like VTK, where one would just need to point to the libraries?

@dzenanz
Copy link
Member

dzenanz commented Oct 29, 2019

If I need to specify path to 10 lib files once, that is fine but not ideal. If I need to do that frequently, it is super annoying.

The purpose of this patch seems to be ability to specify TBB via means other than TBBConfig.cmake.

The best solution is to bite the bullet and add code which generates TBBConfig.cmake by following the instructions provided by Intel.

If you think you can make TBB work without TBBConfig.cmake, start by removing config from the find command here.

Additional relevant links:
uxlfoundation/oneTBB#2
uxlfoundation/oneTBB#6
wjakob/tbb#55

@blowekamp
Copy link
Member

Here is a PR I use to get TBB working for me, which automatically downloaded it:
https://review.source.kitware.com/%23/c/23436/

As I recall the proposed idea was not general enough to be accepted.

@dyollb
Copy link
Contributor Author

dyollb commented Nov 15, 2019

I ended up patching the TBBConfig.cmake to include a tbb prefix. Our build of tbb has a prefix to avoid conflicts with thirdparty libs pulling in a potentially different version of tbb.
TBBConfig.cmake.zip

@dyollb dyollb closed this Nov 15, 2019
@dyollb dyollb deleted the improve_find_tbb branch July 1, 2022 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants