leezu commented on a change in pull request #17559: URL: https://github.com/apache/incubator-mxnet/pull/17559#discussion_r479496991
########## 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: > If you want to use FetchContent then the directory is variable and there needs to be some mechanism for the MXNet code to find intgemm. Essentially you need to update the intgemm cmakelists file to ensure it encapsulates the headers as part of the intgemm target. Once you do that, cmake will handle finding intgemm header files correctly. The reason that things break once you use `FetchContent` is that the `intgemm/CMakeLists.txt` is not correct. Currently you're hardcoding the ` #include "../../../../3rdparty/intgemm/intgemm.h"` path inside the MXNet source files. It works, but it doesn't follow the best practice. ---------------------------------------------------------------- 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]
