leezu commented on a change in pull request #17559:
URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r476832030
##########
File path: CMakeLists.txt
##########
@@ -64,6 +64,13 @@ if(USE_MKL_IF_AVAILABLE AND (NOT APPLE) AND (NOT MSVC) AND
(CMAKE_HOST_SYSTEM_PR
else()
option(USE_MKLDNN "Build with MKL-DNN support" OFF)
endif()
+#gcc 4 doesn't support AVX2 and SSSE3 support doesn't work with target
attributes so ban gcc < 5 from intgemm.
+if ((CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64") AND (NOT
CMAKE_CROSSCOMPILING) AND
+ ((NOT CMAKE_COMPILER_IS_GNUCC) OR (CMAKE_CXX_COMPILER_VERSION
VERSION_GREATER_EQUAL 5.0)))
+ option(USE_INTGEMM "Build with x86 intgemm library for
low-precision multiplication" ON)
+else()
+ option(USE_INTGEMM "Build with x86 intgemm library for
low-precision multiplication" OFF)
+endif()
Review comment:
gcc7 is already required. You don't need any special handling here.
##########
File path: CMakeLists.txt
##########
@@ -474,6 +490,13 @@ endif()
FILE(GLOB_RECURSE SOURCE "src/*.cc" "src/*.h" "include/*.h")
FILE(GLOB_RECURSE CUDA "src/*.cu" "src/*.cuh")
+if (USE_INTGEMM)
+ list(APPEND SOURCE "3rdparty/intgemm/intgemm.cc")
+else()
+ FILE(GLOB_RECURSE INTGEMM_OPERATOR_SOURCE
"src/operator/contrib/intgemm/*.cc" "src/operator/contrib/intgemm/*.h")
+ list(REMOVE_ITEM SOURCE ${INTGEMM_OPERATOR_SOURCE})
+endif()
Review comment:
`add_subdirectory` makes available all exported targets. You should only
need a call à la `target_link_libraries(mxnet PRIVATE intgemm)`
##########
File path: CMakeLists.txt
##########
@@ -278,6 +285,15 @@ if(USE_MKLDNN)
set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint
3rdparty dependency
endif()
+if(USE_INTGEMM)
+ message(STATUS "Using intgemm")
+ add_subdirectory(3rdparty/intgemm)
Review comment:
Do we expect a large number of users to compile with intgemm? If not,
why force all users to download intgemm instead of making it optional?
```
FetchContent_Declare(
intgemm
GIT_REPOSITORY https://github.com/kpu/intgemm.git
GIT_TAG 77095a0cd5da8a22c4e7ed77588b527d3ee4ada9
)
FetchContent_GetProperties(catch)
if(NOT catch_POPULATED)
FetchContent_Populate(catch)
add_subdirectory(${catch_SOURCE_DIR} ${catch_BINARY_DIR})
endif()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
