Skip to content

Commit 39a55ea

Browse files
cyyeverfacebook-github-bot
authored andcommitted
Use newer CMake module features (#4377)
Summary: X-link: facebookresearch/FBGEMM#1452 There are some fixes: 1. use Python module because PythonInterp is deprecated, 2. use OMP targets without writing the flags globally. 3. fix the linking issues of shared libraries with object libraries. 4. default FBGEMM_LIBRARY_TYPE is treated as shared or static depending on BUILD_SHARED_LIB, this is consistent with CMake behavior. Pull Request resolved: #4377 Reviewed By: spcyppt Differential Revision: D76957716 Pulled By: q10 fbshipit-source-id: 51a06e680a668bb9ab410c0edf5fcd3080510457
1 parent 5d3d665 commit 39a55ea

File tree

3 files changed

+17
-28
lines changed

3 files changed

+17
-28
lines changed

CMakeLists.txt

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules")
1717
# Define function to extract filelists from defs.bzl file
1818
function(get_filelist name outputvar)
1919
execute_process(
20-
COMMAND "${PYTHON_EXECUTABLE}" -c
20+
COMMAND "${Python_EXECUTABLE}" -c
2121
"exec(open('defs.bzl').read());print(';'.join(${name}))"
2222
WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
2323
OUTPUT_VARIABLE _tempvar
2424
RESULT_VARIABLE _resvar
2525
ERROR_VARIABLE _errvar)
26-
if (NOT "${_resvar}" EQUAL "0")
27-
message(WARNING "Failed to execute Python (${PYTHON_EXECUTABLE})\n"
26+
if(NOT "${_resvar}" EQUAL "0")
27+
message(WARNING "Failed to execute Python (${Python_EXECUTABLE})\n"
2828
"Result: ${_resvar}\n"
2929
"Error: ${_errvar}\n")
3030
endif()
@@ -94,7 +94,7 @@ project(fbgemm VERSION 0.1 LANGUAGES CXX C)
9494
include(GNUInstallDirs)
9595

9696
# Load Python
97-
find_package(PythonInterp)
97+
find_package(Python)
9898

9999
set(FBGEMM_LIBRARY_TYPE "default"
100100
CACHE STRING
@@ -136,7 +136,7 @@ endif()
136136
include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules/FindGnuH2fIeee.cmake)
137137

138138
# We should default to a Release build
139-
if (NOT CMAKE_BUILD_TYPE OR CMAKE_BUILD_TYPE STREQUAL "")
139+
if(NOT CMAKE_BUILD_TYPE OR CMAKE_BUILD_TYPE STREQUAL "")
140140
set(CMAKE_BUILD_TYPE "Release" CACHE STRING "" FORCE)
141141
endif()
142142

@@ -146,12 +146,8 @@ endif()
146146

147147
# Check if compiler supports OpenMP
148148
find_package(OpenMP)
149-
if (OpenMP_FOUND)
149+
if(OpenMP_FOUND)
150150
message(STATUS "OpenMP found! OpenMP_C_INCLUDE_DIRS = ${OpenMP_C_INCLUDE_DIRS}")
151-
include_directories(${OpenMP_C_INCLUDE_DIRS})
152-
set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
153-
set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
154-
set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")
155151
else()
156152
message(WARNING "OpenMP is not supported by the compiler")
157153
endif()
@@ -267,7 +263,7 @@ if(NOT TARGET asmjit)
267263

268264
# Build asmjit
269265
# For MSVC shared build, asmjit needs to be shared also.
270-
if (MSVC AND (FBGEMM_LIBRARY_TYPE STREQUAL "shared"))
266+
if(MSVC AND (FBGEMM_LIBRARY_TYPE STREQUAL "shared"))
271267
set(ASMJIT_STATIC OFF)
272268
else()
273269
set(ASMJIT_STATIC ON)
@@ -333,23 +329,18 @@ target_include_directories(fbgemm_autovec BEFORE
333329
PRIVATE "${ASMJIT_SRC_DIR}/src"
334330
PRIVATE "${CPUINFO_SOURCE_DIR}/include")
335331

336-
if(FBGEMM_LIBRARY_TYPE STREQUAL "default")
337-
add_library(fbgemm
338-
$<TARGET_OBJECTS:fbgemm_generic>
339-
$<TARGET_OBJECTS:fbgemm_avx2>
340-
$<TARGET_OBJECTS:fbgemm_avx512>
341-
$<TARGET_OBJECTS:fbgemm_autovec>)
342-
elseif(FBGEMM_LIBRARY_TYPE STREQUAL "shared")
332+
if((FBGEMM_LIBRARY_TYPE STREQUAL "default" AND BUILD_SHARED_LIB) OR FBGEMM_LIBRARY_TYPE STREQUAL "shared")
343333
add_library(fbgemm SHARED
344334
$<TARGET_OBJECTS:fbgemm_generic>
345335
$<TARGET_OBJECTS:fbgemm_avx2>
346336
$<TARGET_OBJECTS:fbgemm_avx512>
347337
$<TARGET_OBJECTS:fbgemm_autovec>)
338+
set_property(TARGET fbgemm PROPERTY POSITION_INDEPENDENT_CODE ON)
348339
set_property(TARGET fbgemm_generic PROPERTY POSITION_INDEPENDENT_CODE ON)
349340
set_property(TARGET fbgemm_avx2 PROPERTY POSITION_INDEPENDENT_CODE ON)
350341
set_property(TARGET fbgemm_avx512 PROPERTY POSITION_INDEPENDENT_CODE ON)
351342
set_property(TARGET fbgemm_autovec PROPERTY POSITION_INDEPENDENT_CODE ON)
352-
elseif(FBGEMM_LIBRARY_TYPE STREQUAL "static")
343+
elseif((FBGEMM_LIBRARY_TYPE STREQUAL "default" AND NOT BUILD_SHARED_LIB) OR FBGEMM_LIBRARY_TYPE STREQUAL "static")
353344
add_library(fbgemm STATIC
354345
$<TARGET_OBJECTS:fbgemm_generic>
355346
$<TARGET_OBJECTS:fbgemm_avx2>
@@ -375,15 +366,13 @@ target_include_directories(fbgemm BEFORE
375366
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}>
376367
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}/include>)
377368

378-
target_link_libraries(fbgemm
369+
target_link_libraries(fbgemm PUBLIC
379370
$<BUILD_INTERFACE:asmjit>
380371
$<BUILD_INTERFACE:cpuinfo>)
381-
add_dependencies(fbgemm
382-
asmjit
383-
cpuinfo)
384372

385373
if(OpenMP_FOUND)
386-
target_link_libraries(fbgemm OpenMP::OpenMP_CXX)
374+
target_link_libraries(fbgemm PUBLIC OpenMP::OpenMP_CXX)
375+
target_link_libraries(fbgemm_generic PUBLIC OpenMP::OpenMP_CXX)
387376
endif()
388377

389378
install(

bench/CMakeLists.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ macro(add_benchmark BENCHNAME)
6060
target_link_options(${BENCHNAME} PRIVATE "-fsanitize=${USE_SANITIZER}")
6161
endif()
6262

63-
if(${OpenMP_FOUND})
64-
target_link_libraries(${BENCHNAME} "${OpenMP_CXX_LIBRARIES}")
63+
if(OpenMP_FOUND)
64+
target_link_libraries(${BENCHNAME} OpenMP::OpenMP_CXX)
6565
endif()
6666

6767
if(${MKL_FOUND})

test/CMakeLists.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ macro(add_gtest TESTNAME)
7474

7575
target_link_libraries(${TESTNAME} gtest gmock gtest_main fbgemm)
7676

77-
if(${OpenMP_FOUND})
78-
target_link_libraries(${TESTNAME} ${OpenMP_CXX_LIBRARIES})
77+
if(OpenMP_FOUND)
78+
target_link_libraries(${TESTNAME} OpenMP::OpenMP_CXX)
7979
endif()
8080

8181
add_dependencies(${TESTNAME} gtest fbgemm)

0 commit comments

Comments
 (0)