Allow cmake to build as both a standalone project as well as a subproject (ie add_subdirectory(mkl-dnn)) #44
Conversation
…ject (ie add_subdirectory(mkl-dnn))
There was a problem hiding this comment.
Hi @cjolivier01,
Thank you for the contribution!
Seems to be very reasonable changes.
Few minor things need to be changed before merging.
tests/gtests/CMakeLists.txt
Outdated
| # TODO: enable me! | ||
| #file(GLOB API_TEST_CASES_SRC api_tests/*.cpp) | ||
| #file(GLOB PRIM_TEST_CASES_SRC test_*.cpp) | ||
| file(GLOB PRIM_TEST_CASES_SRC RELATIVE ${CMAKE_SOURCE_DIR}/tests/gtests | ||
| file(GLOB PRIM_TEST_CASES_SRC RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}/../gtests |
There was a problem hiding this comment.
This change is incompatible with cmake 2.8.11.
For some reasons PRIM_TEST_CASES_SRC = ../../test_sum.cpp;../../test_reorder.cpp;... rather than PRIM_TEST_CASES_SRC = test_sum.cpp;test_reorder.cpp;....
Is there any particular reason why you do this trick: ../gtests, even though you are already in this directory?
There was a problem hiding this comment.
Let me check. I found the multiple gtest directories a little confusing.
src/CMakeLists.txt
Outdated
| ${CMAKE_SOURCE_DIR}/src | ||
| ${CMAKE_SOURCE_DIR}/src/common | ||
| ${CMAKE_SOURCE_DIR}/src/cpu/xbyak | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/.. |
There was a problem hiding this comment.
Is there any specific reason for adding this line?
${CMAKE_CURRENT_SOURCE_DIR}/..
tests/gtests/gtest/CMakeLists.txt
Outdated
| ${CMAKE_SOURCE_DIR}/tests/gtests/gtest) | ||
| include_directories(${CMAKE_CURRENT_SOURCE_DIR} | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/.. | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/gtest) |
There was a problem hiding this comment.
The same as here: what this line stands for?
${CMAKE_CURRENT_SOURCE_DIR}/gtest
|
Does that cover them all? |
|
Thank you so much for the contribution! Ideally I should've asked you to unite 1st and 3rd commits together, and then do merge-via-rebase, cause we try to keep history linear. But that would take yet another day. So I decided to do squashing-merge. Thanks once more! Looking forward for new PRs :) |
Fixup: convolution result (BWD_D) may be incorrect when stride_w>1, ic>16, and iw<17
No description provided.