Closed rcrowder closed 5 years ago
Sorry I didn't see the ready to review part. I'll start the review process tomorrow.
@rcrowder Which version of TBBConfig.cmake are you using? TBBConfig.cmake doesn't ship with my distro's TBB package. I tried TBBConfig.cmake from MKL-DNN but it also fails.
There's only a minor style issue. Otherwise every line looks good to me! Thank you very much! Also thanks for fixing all my typos. I should have been more careful. :smile: Thanks.
@marty1885 I've updated the indentations to match your code style. I've been too used to using 2-spaces in Python code :)
The TBB I used on Windows, OSX, and Ubuntu, is the binary releases from here [1] (2019 Update 8). The binary releases (archive files) have the TBBConfig.cmake file that can be used with these changes. As I mention in the Readme.md file, the TBBROOT
environment variable needs to be setup before running CMake for the find_package
[2] to correctly discover the binary release/built version of TBB.
The Intel MKL-DNN version of TBBConfig.cmake looks like it may be specific to that repository. I'm going to try setting up an example using MKL-DDN and Etaler together. To see if the use of TBB in both libraries conflict. I don't think they will when using dynamic library linking. But they may if someone tries to use both libraries statically linked to an exe.
@rcrowder The reason I left TBB to the system is to utilize the fact that there's basically no way for Etaler to conflict with TBB and the system TBB is always dynamically linked.
I'm still getting errors with TBB from my distro using TBBConfig from TBB's release. Which I guess most users will be using the TBB provided by the distro.
-- Missed required Intel TBB component: tbb
-- Missed required Intel TBB component: tbbmalloc
-- Missed required Intel TBB component: tbbmalloc_proxy
CMake Error at CMakeLists.txt:47 (find_package):
Found package configuration file:
/usr/local/share/cmake/TBB/TBBConfig.cmake
but it set TBB_FOUND to FALSE so package "TBB" is considered to be NOT
FOUND.
-- Configuring incomplete, errors occurred!
Even though
> sudo pacman -S intel-tbb
warning: intel-tbb-2019.7-1 is up to date -- reinstalling
resolving dependencies...
looking for conflicting packages...
Packages (1) intel-tbb-2019.7-1
Total Installed Size: 2.54 MiB
Net Upgrade Size: 0.00 MiB
:: Proceed with installation? [Y/n] n
I'm not very familiar with how TBB deals with finding itself. Am I doing anything wrong?
@marty1885 I've refactored the way that it finds TBB. It now looks for a tbb/tbb.h
file on your system (e.g. installed via a distro or homebrew). If it exists then it continues exactly the same way you had it before. If it cannot find that header file, it tries to use the find_package TBB cmake module method. The TBB module is part of the TBB binary release, and ties in with the build instructions I've added for MSVC.
@rcrowder Sorry for being late, I'm finally back from my final exam hell. Thanks! it works now! And all looks good.
Fixes #1
Changes required to get Etaler to build using Visual Studio 2019 on Windows.
All the changes have been tested and work on Windows (MSVC), Ubuntu, and Mac OSX.
I needed to add CMake
generate_export_header
to automate the production of macros [1]. The macros (i.e.ETALER_EXPORT
) have been added in all the places neededto get the tests and examples to compile and run. More macros will be required when other structures/classes are required to be made visible in future tests/examples.@marty1885 This PR is now ready for review.