Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Include all mkldnn headers in CD builds #18355

Merged
merged 2 commits into from
May 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 17 additions & 20 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -240,24 +240,26 @@ if(USE_MKLDNN)
set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} /EHsc /Gy /MT")
endif()

set(MKLDNN_BUILD_TESTS OFF CACHE INTERNAL "" FORCE)
set(MKLDNN_BUILD_EXAMPLES OFF CACHE INTERNAL "" FORCE)
set(MKLDNN_ARCH_OPT_FLAGS "" CACHE INTERNAL "" FORCE)
set(MKLDNN_ENABLE_JIT_PROFILING OFF CACHE INTERNAL "" FORCE)
set(MKLDNN_LIBRARY_TYPE STATIC CACHE INTERNAL "" FORCE)
set(DNNL_ENABLE_CONCURRENT_EXEC ON CACHE INTERNAL "" FORCE)

if(NOT USE_OPENMP)
function(load_mkldnn)
Copy link
Member

Choose a reason for hiding this comment

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

Why need put them into a function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Functions have their own scope in cmake. We don't want to change any of the settings outside of this function (such as the install target name for the include files)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it's really about setting the CMAKE_INSTALL_INCLUDEDIR differently for mkldnn which is introduced in this PR

set(MKLDNN_BUILD_TESTS OFF CACHE INTERNAL "" FORCE)
set(MKLDNN_BUILD_EXAMPLES OFF CACHE INTERNAL "" FORCE)
set(MKLDNN_ARCH_OPT_FLAGS "" CACHE INTERNAL "" FORCE)
set(MKLDNN_ENABLE_JIT_PROFILING OFF CACHE INTERNAL "" FORCE)
set(MKLDNN_LIBRARY_TYPE STATIC CACHE INTERNAL "" FORCE)
set(DNNL_ENABLE_CONCURRENT_EXEC ON CACHE INTERNAL "" FORCE)

if(NOT USE_OPENMP)
set(MKLDNN_CPU_RUNTIME SEQ CACHE INTERNAL "" FORCE)
endif()

add_subdirectory(3rdparty/mkldnn)
endif()

set(CMAKE_INSTALL_INCLUDEDIR "${CMAKE_INSTALL_INCLUDEDIR}/mkldnn")
add_subdirectory(3rdparty/mkldnn)
endfunction()
load_mkldnn()
include_directories(3rdparty/mkldnn/include)
include_directories(${PROJECT_BINARY_DIR}/3rdparty/mkldnn/include)
add_definitions(-DMXNET_USE_MKLDNN=1)
list(APPEND mxnet_LINKER_LIBS dnnl)
set(INSTALL_MKLDNN ON)
Copy link
Member

Choose a reason for hiding this comment

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

Why it's removed? @yzhliu FYI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This variable isn't used anywhere but in this line

endif()

# Allow Cuda compiles outside of src tree to find things in 'src' and 'include'
Expand Down Expand Up @@ -838,22 +840,17 @@ if(MSVC AND USE_MXNET_LIB_NAMING)
endif()


# NOTE: Public headers will be installed into ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}, see
# https://cmake.org/cmake/help/v3.13/variable/CMAKE_INSTALL_PREFIX.html
# https://cmake.org/cmake/help/v3.13/module/GNUInstallDirs.html
include(GNUInstallDirs)
install(TARGETS ${MXNET_INSTALL_TARGETS}
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
)

# NOTE: Public headers will be installed into ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR}, see
# https://cmake.org/cmake/help/v3.0/variable/CMAKE_INSTALL_PREFIX.html
# https://cmake.org/cmake/help/v3.0/module/GNUInstallDirs.html

install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/dlpack/include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/dmlc-core/include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
if(USE_MKLDNN)
install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/mkldnn/include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
endif()
install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/mshadow/mshadow/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/mshadow)
install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/include/mxnet/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/mxnet)
install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/3rdparty/tvm/nnvm/include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
Expand Down
6 changes: 3 additions & 3 deletions cd/python/pypi/pypi_package.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ export mxnet_variant=${1:?"Please specify the mxnet variant"}
# and CD unpacks it to a directory called cd_misc
# Nov. 2019 Update: With v1.1, MKL-DNN is renaming to DNNL. Hence changing the prefix of file name.
if [ -f "cd_misc/dnnl_version.h" ]; then
mkdir -p 3rdparty/mkldnn/build/install/include
cp cd_misc/dnnl_version.h 3rdparty/mkldnn/build/install/include/.
cp cd_misc/dnnl_config.h 3rdparty/mkldnn/build/install/include/.
mkdir -p 3rdparty/mkldnn/include
cp cd_misc/dnnl_version.h 3rdparty/mkldnn/include/.
cp cd_misc/dnnl_config.h 3rdparty/mkldnn/include/.
fi

# Create wheel workspace
Expand Down
2 changes: 1 addition & 1 deletion tools/pip/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def skip_markdown_comments(md):
package_data = {'mxnet': [os.path.join('mxnet', os.path.basename(LIB_PATH[0]))],
'dmlc_tracker': []}
if variant != 'NATIVE':
shutil.copytree(os.path.join(CURRENT_DIR, 'mxnet-build/3rdparty/mkldnn/build/install/include'),
shutil.copytree(os.path.join(CURRENT_DIR, 'mxnet-build/3rdparty/mkldnn/include'),
os.path.join(CURRENT_DIR, 'mxnet/include/mkldnn'))
if platform.system() == 'Linux':
libdir, mxdir = os.path.dirname(LIB_PATH[0]), os.path.join(CURRENT_DIR, 'mxnet')
Expand Down