leezu commented on a change in pull request #17559: URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479439896
########## File path: CMakeLists.txt ########## @@ -278,6 +283,25 @@ if(USE_MKLDNN) set_target_properties(dnnl PROPERTIES CXX_CLANG_TIDY "") # don't lint 3rdparty dependency endif() +if(USE_INTGEMM) + message(STATUS "Using intgemm") + include(FetchContent) + FetchContent_Declare( + intgemm + GIT_REPOSITORY https://github.com/kpu/intgemm.git + GIT_TAG 0f05c3ebd037eacdf8cff165736fea2b0d125023 + ) + FetchContent_GetProperties(intgemm) + if(NOT intgemm_POPULATED) + FetchContent_Populate(intgemm) + add_subdirectory(${intgemm_SOURCE_DIR} ${intgemm_BINARY_DIR} EXCLUDE_FROM_ALL) + endif() + include_directories(${intgemm_SOURCE_DIR}) + #intgemm generates a config header based on AVX512 support in the compiler. + include_directories(${intgemm_BINARY_DIR}) Review comment: It's bad practice to modify the gobal list of default include directories. If the intgemm CMakeLists.txt is correct, these two lines are not needed as `target_link_libraries(mxnet PRIVATE intgemm)` will handle the includes automatically. ########## File path: CMakeLists.txt ########## @@ -64,6 +64,11 @@ 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() +if ((CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64") AND (NOT CMAKE_CROSSCOMPILING)) + 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: If `USE_INTGEMM` is only supported on x86, you should use `cmake_dependent_option`. If it's supported but not recommended on other architectures, your current approach is fine, though you may want to update the docstring to clarify. ---------------------------------------------------------------- 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]
