Skip to content

Commit 2168495

Browse files
cyyeverfacebook-github-bot
cyyever
authored and
facebook-github-bot
committed
Fix MSVC build (#4491)
Summary: X-link: facebookresearch/FBGEMM#1549 This PR fixes all warnings I encountered when trying to build FBGEMM on Windows and integrating the latest FBGEMM to PyTorch. Redundant public settings are removed. Other public settings are removed to `fbgemm_generic` because it is relied by other targets. Pull Request resolved: #4491 Reviewed By: cthi Differential Revision: D78318203 Pulled By: q10 fbshipit-source-id: 37c1d3999a4611e661312ec1bfe493031ccbbe8f
1 parent 16cf235 commit 2168495

File tree

5 files changed

+52
-105
lines changed

5 files changed

+52
-105
lines changed

‎CMakeLists.txt

Lines changed: 42 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,15 @@ option(FBGEMM_BUILD_FBGEMM_GPU "Build fbgemm_gpu library" OFF)
108108
option(FBGEMM_USE_IPO "Build fbgemm with interprocedural optimization" OFF)
109109
option(DISABLE_FBGEMM_AUTOVEC "Disable FBGEMM Autovec" OFF)
110110

111+
112+
if(FBGEMM_LIBRARY_TYPE STREQUAL "default")
113+
if(BUILD_SHARED_LIB)
114+
set(FBGEMM_LIBRARY_TYPE "shared")
115+
else()
116+
set(FBGEMM_LIBRARY_TYPE "static")
117+
endif()
118+
endif()
119+
111120
if(FBGEMM_BUILD_TESTS)
112121
enable_testing()
113122
endif()
@@ -138,7 +147,7 @@ include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules/FindGnuH2fIeee.cmake)
138147

139148
# We should default to a Release build
140149
if(NOT CMAKE_BUILD_TYPE OR CMAKE_BUILD_TYPE STREQUAL "")
141-
set(CMAKE_BUILD_TYPE "Release" CACHE STRING "" FORCE)
150+
set(CMAKE_BUILD_TYPE "Release" CACHE STRING "" FORCE)
142151
endif()
143152

144153
if(DISABLE_FBGEMM_AUTOVEC)
@@ -173,28 +182,26 @@ add_library(fbgemm_autovec OBJECT ${FBGEMM_AUTOVEC_SRCS})
173182
# Make libraries depend on defs.bzl
174183
add_custom_target(defs.bzl DEPENDS defs.bzl)
175184
add_dependencies(fbgemm_generic defs.bzl)
176-
add_dependencies(fbgemm_avx2 defs.bzl)
177-
add_dependencies(fbgemm_avx512 defs.bzl)
178-
add_dependencies(fbgemm_autovec defs.bzl)
185+
target_link_libraries(fbgemm_avx2 PUBLIC fbgemm_generic)
186+
target_link_libraries(fbgemm_avx512 PUBLIC fbgemm_generic)
187+
target_link_libraries(fbgemm_autovec PUBLIC fbgemm_generic)
188+
189+
target_include_directories(fbgemm_generic PUBLIC
190+
$<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}>
191+
$<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}/include>
192+
$<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}/bench>)
193+
194+
target_link_libraries(fbgemm_generic PUBLIC
195+
$<BUILD_INTERFACE:asmjit>
196+
$<BUILD_INTERFACE:cpuinfo>)
179197

180198
# On Windows:
181199
# 1) Adding definition of ASMJIT_STATIC to avoid generating asmjit function
182200
# calls with _dllimport attribute
183-
# 2) MSVC uses /MD in default cxx compiling flags,
184-
# Need to change it to /MT in static case
185201
if(MSVC)
186-
set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4244 /wd4267 /wd4305 /wd4309")
202+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4244 /wd4267 /wd4305 /wd4309")
187203
if(FBGEMM_LIBRARY_TYPE STREQUAL "static")
188-
target_compile_definitions(fbgemm_generic PRIVATE ASMJIT_STATIC)
189-
target_compile_definitions(fbgemm_avx2 PRIVATE ASMJIT_STATIC)
190-
target_compile_definitions(fbgemm_avx512 PRIVATE ASMJIT_STATIC)
191-
foreach(flag_var
192-
CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE
193-
CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO)
194-
if(${flag_var} MATCHES "/MD")
195-
string(REGEX REPLACE "/MD" "/MT" ${flag_var} "${${flag_var}}")
196-
endif(${flag_var} MATCHES "/MD")
197-
endforeach(flag_var)
204+
target_compile_definitions(fbgemm_generic PUBLIC ASMJIT_STATIC)
198205
endif()
199206
target_compile_options(fbgemm_avx2 PRIVATE "/arch:AVX2")
200207
target_compile_options(fbgemm_avx512 PRIVATE "/arch:AVX512")
@@ -238,11 +245,9 @@ else(MSVC)
238245
endif(MSVC)
239246

240247
if(USE_SANITIZER)
241-
target_compile_options(fbgemm_generic PRIVATE
248+
target_link_options(fbgemm_generic PUBLIC
242249
"-fsanitize=${USE_SANITIZER}" "-fno-omit-frame-pointer")
243-
target_compile_options(fbgemm_avx2 PRIVATE
244-
"-fsanitize=${USE_SANITIZER}" "-fno-omit-frame-pointer")
245-
target_compile_options(fbgemm_avx512 PRIVATE
250+
target_compile_options(fbgemm_generic PUBLIC
246251
"-fsanitize=${USE_SANITIZER}" "-fno-omit-frame-pointer")
247252
endif()
248253

@@ -303,78 +308,35 @@ if(NOT TARGET cpuinfo)
303308
set_property(TARGET cpuinfo PROPERTY POSITION_INDEPENDENT_CODE ON)
304309
endif()
305310

306-
target_include_directories(fbgemm_generic BEFORE
307-
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}>
308-
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}/include>
309-
PRIVATE "${ASMJIT_SRC_DIR}/src"
310-
PRIVATE "${CPUINFO_SOURCE_DIR}/include")
311-
312-
target_include_directories(fbgemm_avx2 BEFORE
313-
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}>
314-
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}/include>
315-
PRIVATE "${ASMJIT_SRC_DIR}/src"
316-
PRIVATE "${CPUINFO_SOURCE_DIR}/include")
317-
318-
target_include_directories(fbgemm_avx512 BEFORE
319-
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}>
320-
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}/include>
321-
PRIVATE "${ASMJIT_SRC_DIR}/src"
322-
PRIVATE "${CPUINFO_SOURCE_DIR}/include")
323-
324-
target_include_directories(fbgemm_autovec BEFORE
325-
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}>
326-
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}/include>
327-
PRIVATE "${ASMJIT_SRC_DIR}/src"
328-
PRIVATE "${CPUINFO_SOURCE_DIR}/include")
329-
330-
if((FBGEMM_LIBRARY_TYPE STREQUAL "default" AND BUILD_SHARED_LIB) OR FBGEMM_LIBRARY_TYPE STREQUAL "shared")
331-
add_library(fbgemm SHARED
332-
$<TARGET_OBJECTS:fbgemm_generic>
333-
$<TARGET_OBJECTS:fbgemm_avx2>
334-
$<TARGET_OBJECTS:fbgemm_avx512>
335-
$<TARGET_OBJECTS:fbgemm_autovec>)
336-
set_property(TARGET fbgemm PROPERTY POSITION_INDEPENDENT_CODE ON)
337-
set_property(TARGET fbgemm_generic PROPERTY POSITION_INDEPENDENT_CODE ON)
338-
set_property(TARGET fbgemm_avx2 PROPERTY POSITION_INDEPENDENT_CODE ON)
339-
set_property(TARGET fbgemm_avx512 PROPERTY POSITION_INDEPENDENT_CODE ON)
340-
set_property(TARGET fbgemm_autovec PROPERTY POSITION_INDEPENDENT_CODE ON)
341-
elseif((FBGEMM_LIBRARY_TYPE STREQUAL "default" AND NOT BUILD_SHARED_LIB) OR FBGEMM_LIBRARY_TYPE STREQUAL "static")
342-
add_library(fbgemm STATIC
343-
$<TARGET_OBJECTS:fbgemm_generic>
344-
$<TARGET_OBJECTS:fbgemm_avx2>
345-
$<TARGET_OBJECTS:fbgemm_avx512>
346-
$<TARGET_OBJECTS:fbgemm_autovec>)
311+
file(WRITE "${CMAKE_BINARY_DIR}/fbgemm_lib_dummy.cc" "")
312+
if(FBGEMM_LIBRARY_TYPE STREQUAL "shared")
313+
add_library(fbgemm SHARED "${CMAKE_BINARY_DIR}/fbgemm_lib_dummy.cc")
314+
elseif(FBGEMM_LIBRARY_TYPE STREQUAL "static")
315+
add_library(fbgemm STATIC "${CMAKE_BINARY_DIR}/fbgemm_lib_dummy.cc")
347316
#MSVC need to define FBGEMM_STATIC for fbgemm_generic also to
348317
#avoid generating _dllimport functions.
349-
target_compile_definitions(fbgemm_generic PRIVATE FBGEMM_STATIC)
350-
target_compile_definitions(fbgemm_avx2 PRIVATE FBGEMM_STATIC)
351-
target_compile_definitions(fbgemm_avx512 PRIVATE FBGEMM_STATIC)
352-
target_compile_definitions(fbgemm_autovec PRIVATE FBGEMM_STATIC)
353-
target_compile_definitions(fbgemm PRIVATE FBGEMM_STATIC)
318+
target_compile_definitions(fbgemm_generic PUBLIC FBGEMM_STATIC)
354319
else()
355320
message(FATAL_ERROR "Unsupported library type ${FBGEMM_LIBRARY_TYPE}")
356321
endif()
357-
358-
if(USE_SANITIZER)
359-
target_link_options(fbgemm PRIVATE
360-
"-fsanitize=${USE_SANITIZER}" "-fno-omit-frame-pointer")
361-
endif()
362-
363-
target_include_directories(fbgemm BEFORE
364-
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}>
365-
PUBLIC $<BUILD_INTERFACE:${FBGEMM_SOURCE_DIR}/include>)
322+
set_property(TARGET fbgemm PROPERTY POSITION_INDEPENDENT_CODE ON)
323+
set_property(TARGET fbgemm_generic PROPERTY POSITION_INDEPENDENT_CODE ON)
324+
set_property(TARGET fbgemm_avx2 PROPERTY POSITION_INDEPENDENT_CODE ON)
325+
set_property(TARGET fbgemm_avx512 PROPERTY POSITION_INDEPENDENT_CODE ON)
326+
set_property(TARGET fbgemm_autovec PROPERTY POSITION_INDEPENDENT_CODE ON)
366327

367328
target_link_libraries(fbgemm PUBLIC
368-
$<BUILD_INTERFACE:asmjit>
369-
$<BUILD_INTERFACE:cpuinfo>)
329+
fbgemm_generic
330+
fbgemm_avx2
331+
fbgemm_avx512
332+
fbgemm_autovec)
370333

371334
if(OpenMP_FOUND)
372-
target_link_libraries(fbgemm PUBLIC OpenMP::OpenMP_CXX)
373335
target_link_libraries(fbgemm_generic PUBLIC OpenMP::OpenMP_CXX)
374336
endif()
375337

376338
install(
377-
TARGETS fbgemm
339+
TARGETS fbgemm fbgemm_generic fbgemm_avx2 fbgemm_avx512 fbgemm_autovec
378340
EXPORT fbgemmLibraryConfig
379341
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
380342
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}

‎bench/BenchUtils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ void performance_test(
381381
if (std::abs(C_ref[0][i] - C_fb[0][i]) > 1e-3) {
382382
fprintf(
383383
stderr,
384-
"Error: too high diff between fp32 ref %f and fp16 %f at %ld\n",
384+
"Error: too high diff between fp32 ref %f and fp16 %f at %zu\n",
385385
C_ref[0][i],
386386
C_fb[0][i],
387387
i);

‎bench/CMakeLists.txt

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -45,20 +45,15 @@ macro(add_benchmark BENCHNAME)
4545
../test/EmbeddingSpMDMTestUtils.cc
4646
)
4747

48-
target_compile_options(
49-
${BENCHNAME}
50-
PRIVATE
51-
"-m64" "-mavx2" "-mfma" "-masm=intel"
52-
)
48+
if(NOT MSVC)
49+
target_compile_options(
50+
${BENCHNAME}
51+
PRIVATE
52+
"-m64" "-mavx2" "-mfma" "-masm=intel"
53+
)
54+
endif()
5355

5456
target_link_libraries(${BENCHNAME} fbgemm)
55-
add_dependencies(${BENCHNAME} fbgemm)
56-
57-
if (USE_SANITIZER)
58-
target_compile_options(${BENCHNAME} PRIVATE
59-
"-fsanitize=${USE_SANITIZER}" "-fno-omit-frame-pointer")
60-
target_link_options(${BENCHNAME} PRIVATE "-fsanitize=${USE_SANITIZER}")
61-
endif()
6257

6358
if(OpenMP_FOUND)
6459
target_link_libraries(${BENCHNAME} OpenMP::OpenMP_CXX)
@@ -100,8 +95,4 @@ if(FBGEMM_BUILD_BENCHMARKS)
10095
add_dependencies(run_benchmarks
10196
${BENCHMARKS})
10297

103-
set_source_files_properties(
104-
bench/GEMMsBenchmark.cc
105-
PROPERTIES COMPILE_FLAGS "-Wno-unused-variable")
106-
10798
endif()

‎bench/SparseDenseMMInt8Benchmark.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ int main(int /*unused*/, char** /*unused*/) {
166166
if (std::abs(ctDataRef_u8[i] - ctDataIntrin_u8[i]) > 0) {
167167
fprintf(
168168
stderr,
169-
"Error: Results differ ref %d and test %d at %ld\n",
169+
"Error: Results differ ref %d and test %d at %zu\n",
170170
ctDataRef_u8[i],
171171
ctDataIntrin_u8[i],
172172
i);

‎test/CMakeLists.txt

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,13 @@ set(CMAKE_C_STANDARD_REQUIRED ON)
1717
if(FBGEMM_BUILD_TESTS AND NOT TARGET gtest)
1818
#Download Googletest framework from github if
1919
#GOOGLETEST_SOURCE_DIR is not specified.
20+
set(INSTALL_GTEST OFF)
2021
if(NOT DEFINED GOOGLETEST_SOURCE_DIR)
2122
set(GOOGLETEST_SOURCE_DIR "${FBGEMM_SOURCE_DIR}/external/googletest"
2223
CACHE STRING "googletest source directory from submodules")
2324
endif()
2425

2526
#build Googletest framework
26-
#MSVC needs gtest_for_shared_crt to select right runtime lib
27-
if (MSVC AND FBGEMM_LIBRARY_TYPE STREQUAL "shared")
28-
message(WARNING "gtest_force_shared_crt is ON")
29-
set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
30-
else()
31-
message(WARNING "gtest_force_shared_crt is OFF")
32-
endif()
3327
add_subdirectory("${GOOGLETEST_SOURCE_DIR}" "${FBGEMM_BINARY_DIR}/googletest")
3428
# add flags required for mac build
3529
if(NOT MSVC)

0 commit comments

Comments
 (0)