Skip to content

Commit 9106671

Browse files
HyukjinKwonHyukjinKwon
authored
GH-48980: [C++] Use COMPILE_OPTIONS instead of deprecated COMPILE_FLAGS (#48981)
### Rationale for this change Arrow requires CMake 3.25 but was still using deprecated `COMPILE_FLAGS` property. Recommanded to use `COMPILE_OPTIONS` (introduced in CMake 3.11). ### What changes are included in this PR? Replaced `COMPILE_FLAGS` with `COMPILE_OPTIONS` across `CMakeLists.txt` files, converted space separated strings to semicolon-separated lists, and removed obsolete TODO comments. ### Are these changes tested? Yes, through CI build and existing tests. ### Are there any user-facing changes? No. * GitHub Issue: #48980 Authored-by: Hyukjin Kwon <gurwls223@apache.org> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
1 parent 85c18a0 commit 9106671

5 files changed

Lines changed: 26 additions & 27 deletions

File tree

‎cpp/cmake_modules/SetupCxxFlags.cmake‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,26 +54,27 @@ if(ARROW_CPU_FLAG STREQUAL "x86")
5454
# sets available, but they are not set by MSVC (unlike other compilers).
5555
# See https://github.com/AcademySoftwareFoundation/OpenImageIO/issues/4265
5656
add_definitions(-D__SSE2__ -D__SSE4_1__ -D__SSE4_2__)
57-
set(ARROW_AVX2_FLAG "/arch:AVX2")
57+
set(ARROW_AVX2_FLAGS "/arch:AVX2")
5858
# MSVC has no specific flag for BMI2, it seems to be enabled with AVX2
59-
set(ARROW_BMI2_FLAG "/arch:AVX2")
59+
set(ARROW_BMI2_FLAGS "/arch:AVX2")
6060
set(ARROW_AVX512_FLAG "/arch:AVX512")
6161
set(CXX_SUPPORTS_SSE4_2 TRUE)
6262
else()
6363
set(ARROW_SSE4_2_FLAG "-msse4.2")
64-
set(ARROW_AVX2_FLAG "-march=haswell")
64+
set(ARROW_AVX2_FLAGS "-march=haswell")
6565
set(ARROW_BMI2_FLAG "-mbmi2")
6666
# skylake-avx512 consists of AVX512F,AVX512BW,AVX512VL,AVX512CD,AVX512DQ
6767
set(ARROW_AVX512_FLAG "-march=skylake-avx512")
6868
# Append the avx2/avx512 subset option also, fix issue ARROW-9877 for homebrew-cpp
69-
set(ARROW_AVX2_FLAG "${ARROW_AVX2_FLAG} -mavx2")
69+
list(APPEND ARROW_AVX2_FLAGS "-mavx2")
7070
set(ARROW_AVX512_FLAG
7171
"${ARROW_AVX512_FLAG} -mavx512f -mavx512cd -mavx512vl -mavx512dq -mavx512bw")
7272
check_cxx_compiler_flag(${ARROW_SSE4_2_FLAG} CXX_SUPPORTS_SSE4_2)
7373
endif()
7474
if(CMAKE_SIZEOF_VOID_P EQUAL 8)
7575
# Check for AVX extensions on 64-bit systems only, as 32-bit support seems iffy
76-
check_cxx_compiler_flag(${ARROW_AVX2_FLAG} CXX_SUPPORTS_AVX2)
76+
list(JOIN ARROW_AVX2_FLAGS " " ARROW_AVX2_FLAGS_COMMAND_LINE)
77+
check_cxx_compiler_flag("${ARROW_AVX2_FLAGS_COMMAND_LINE}" CXX_SUPPORTS_AVX2)
7778
if(MINGW)
7879
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65782
7980
message(STATUS "Disable AVX512 support on MINGW for now")
@@ -494,7 +495,8 @@ if(ARROW_CPU_FLAG STREQUAL "x86")
494495
if(NOT CXX_SUPPORTS_AVX2)
495496
message(FATAL_ERROR "AVX2 required but compiler doesn't support it.")
496497
endif()
497-
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} ${ARROW_AVX2_FLAG}")
498+
list(JOIN ARROW_AVX2_FLAGS " " ARROW_AVX2_FLAGS_COMMAND_LINE)
499+
string(APPEND CXX_COMMON_FLAGS " ${ARROW_AVX2_FLAGS_COMMAND_LINE}")
498500
add_definitions(-DARROW_HAVE_AVX2 -DARROW_HAVE_BMI2 -DARROW_HAVE_SSE4_2)
499501
elseif(ARROW_SIMD_LEVEL STREQUAL "SSE4_2")
500502
if(NOT CXX_SUPPORTS_SSE4_2)

‎cpp/examples/parquet/CMakeLists.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ endif()
4343
if(UNIX)
4444
foreach(FILE ${PARQUET_EXAMPLES_WARNING_SUPPRESSIONS})
4545
set_property(SOURCE ${FILE}
46-
APPEND_STRING
47-
PROPERTY COMPILE_FLAGS "-Wno-unused-variable")
46+
APPEND
47+
PROPERTY COMPILE_OPTIONS "-Wno-unused-variable")
4848
endforeach()
4949
endif()
5050

‎cpp/src/arrow/CMakeLists.txt‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -322,22 +322,24 @@ endfunction()
322322
macro(append_runtime_avx2_src SRCS SRC)
323323
if(ARROW_HAVE_RUNTIME_AVX2)
324324
list(APPEND ${SRCS} ${SRC})
325-
set_source_files_properties(${SRC} PROPERTIES COMPILE_FLAGS ${ARROW_AVX2_FLAG})
325+
set_source_files_properties(${SRC} PROPERTIES COMPILE_OPTIONS "${ARROW_AVX2_FLAGS}")
326326
endif()
327327
endmacro()
328328

329329
macro(append_runtime_avx2_bmi2_src SRCS SRC)
330330
if(ARROW_HAVE_RUNTIME_AVX2 AND ARROW_HAVE_RUNTIME_BMI2)
331331
list(APPEND ${SRCS} ${SRC})
332-
set_source_files_properties(${SRC} PROPERTIES COMPILE_FLAGS
333-
"${ARROW_AVX2_FLAG} ${ARROW_BMI2_FLAG}")
332+
set_source_files_properties(${SRC}
333+
PROPERTIES COMPILE_OPTIONS
334+
"${ARROW_AVX2_FLAGS};${ARROW_BMI2_FLAG}")
334335
endif()
335336
endmacro()
336337

337338
macro(append_runtime_avx512_src SRCS SRC)
338339
if(ARROW_HAVE_RUNTIME_AVX512)
339340
list(APPEND ${SRCS} ${SRC})
340-
set_source_files_properties(${SRC} PROPERTIES COMPILE_FLAGS ${ARROW_AVX512_FLAG})
341+
separate_arguments(AVX512_FLAG_LIST NATIVE_COMMAND "${ARROW_AVX512_FLAG}")
342+
set_source_files_properties(${SRC} PROPERTIES COMPILE_OPTIONS "${AVX512_FLAG_LIST}")
341343
endif()
342344
endmacro()
343345

@@ -912,8 +914,8 @@ if(ARROW_FILESYSTEM)
912914
# Suppress documentation warnings from google-cloud-cpp headers
913915
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|AppleClang")
914916
set_source_files_properties(filesystem/gcsfs.cc filesystem/gcsfs_internal.cc
915-
PROPERTIES COMPILE_FLAGS
916-
"-Wno-documentation -Wno-documentation-deprecated-sync"
917+
PROPERTIES COMPILE_OPTIONS
918+
"-Wno-documentation;-Wno-documentation-deprecated-sync"
917919
)
918920
endif()
919921
endif()

‎cpp/src/arrow/flight/sql/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ endif()
9595

9696
if(MSVC)
9797
# Suppress warnings caused by Protobuf (casts)
98-
set_source_files_properties(protocol_internal.cc PROPERTIES COMPILE_FLAGS "/wd4267")
98+
set_source_files_properties(protocol_internal.cc PROPERTIES COMPILE_OPTIONS "/wd4267")
9999
endif()
100100
foreach(LIB_TARGET ${ARROW_FLIGHT_SQL_LIBRARIES})
101101
target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_FLIGHT_SQL_EXPORTING)

‎cpp/src/parquet/CMakeLists.txt‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ set_source_files_properties("${PARQUET_THRIFT_SOURCE_DIR}/parquet_types.cpp"
144144

145145
if(NOT MSVC)
146146
set_source_files_properties(src/parquet/parquet_types.cpp
147-
PROPERTIES COMPILE_FLAGS -Wno-unused-variable)
147+
PROPERTIES COMPILE_OPTIONS -Wno-unused-variable)
148148
endif()
149149

150150
#
@@ -200,14 +200,12 @@ if(ARROW_HAVE_RUNTIME_AVX2)
200200
# violation with -DCMAKE_BUILD_TYPE=MinSizeRel. CMAKE_CXX_FLAGS_RELEASE
201201
# will force inlining as much as possible.
202202
# See also: ARROW-15664 and ARROW-15678
203-
#
204-
# TODO: Use COMPILE_OPTIONS instead of COMPILE_FLAGS when we require
205-
# CMake 3.11 or later.
206-
set(AVX2_FLAGS "${ARROW_AVX2_FLAG}")
203+
set(AVX2_FLAGS ${ARROW_AVX2_FLAGS})
207204
if(NOT MSVC)
208-
string(APPEND AVX2_FLAGS " ${CMAKE_CXX_FLAGS_RELEASE}")
205+
separate_arguments(RELEASE_FLAGS NATIVE_COMMAND "${CMAKE_CXX_FLAGS_RELEASE}")
206+
list(APPEND AVX2_FLAGS ${RELEASE_FLAGS})
209207
endif()
210-
set_source_files_properties(level_comparison_avx2.cc PROPERTIES COMPILE_FLAGS
208+
set_source_files_properties(level_comparison_avx2.cc PROPERTIES COMPILE_OPTIONS
211209
"${AVX2_FLAGS}")
212210
# WARNING: DO NOT BLINDLY COPY THIS CODE FOR OTHER BMI2 USE CASES.
213211
# This code is always guarded by runtime dispatch which verifies
@@ -218,14 +216,11 @@ if(ARROW_HAVE_RUNTIME_AVX2)
218216
# violation with -DCMAKE_BUILD_TYPE=MinSizeRel. CMAKE_CXX_FLAGS_RELEASE
219217
# will force inlining as much as possible.
220218
# See also: ARROW-15664 and ARROW-15678
221-
#
222-
# TODO: Use COMPILE_OPTIONS instead of COMPILE_FLAGS when we require
223-
# CMake 3.11 or later.
224219
if(ARROW_HAVE_RUNTIME_BMI2)
225220
# Need to pass ARROW_HAVE_BMI2 for level_conversion_inc.h to compile
226221
# the BMI2 path.
227-
set(BMI2_FLAGS "${AVX2_FLAGS} ${ARROW_BMI2_FLAG} -DARROW_HAVE_BMI2")
228-
set_source_files_properties(level_conversion_bmi2.cc PROPERTIES COMPILE_FLAGS
222+
set(BMI2_FLAGS ${AVX2_FLAGS} ${ARROW_BMI2_FLAG} -DARROW_HAVE_BMI2)
223+
set_source_files_properties(level_conversion_bmi2.cc PROPERTIES COMPILE_OPTIONS
229224
"${BMI2_FLAGS}")
230225
endif()
231226
endif()

0 commit comments

Comments
 (0)