Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Jan 26, 2026

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-actions
Copy link

⚠️ GitHub issue #48980 has been automatically assigned in GitHub to PR creator.

if(ARROW_HAVE_RUNTIME_AVX2)
list(APPEND ${SRCS} ${SRC})
set_source_files_properties(${SRC} PROPERTIES COMPILE_FLAGS ${ARROW_AVX2_FLAG})
separate_arguments(AVX2_FLAG_LIST NATIVE_COMMAND "${ARROW_AVX2_FLAG}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we define ARROW_AVX2_FLAG as a list not string something like the following?

diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake
index f4ff0bded3..564c2a7fc4 100644
--- a/cpp/cmake_modules/SetupCxxFlags.cmake
+++ b/cpp/cmake_modules/SetupCxxFlags.cmake
@@ -54,26 +54,27 @@ if(ARROW_CPU_FLAG STREQUAL "x86")
     # sets available, but they are not set by MSVC (unlike other compilers).
     # See https://github.com/AcademySoftwareFoundation/OpenImageIO/issues/4265
     add_definitions(-D__SSE2__ -D__SSE4_1__ -D__SSE4_2__)
-    set(ARROW_AVX2_FLAG "/arch:AVX2")
+    set(ARROW_AVX2_FLAGS "/arch:AVX2")
     # MSVC has no specific flag for BMI2, it seems to be enabled with AVX2
-    set(ARROW_BMI2_FLAG "/arch:AVX2")
+    set(ARROW_BMI2_FLAGS "/arch:AVX2")
     set(ARROW_AVX512_FLAG "/arch:AVX512")
     set(CXX_SUPPORTS_SSE4_2 TRUE)
   else()
     set(ARROW_SSE4_2_FLAG "-msse4.2")
-    set(ARROW_AVX2_FLAG "-march=haswell")
+    set(ARROW_AVX2_FLAGS "-march=haswell")
     set(ARROW_BMI2_FLAG "-mbmi2")
     # skylake-avx512 consists of AVX512F,AVX512BW,AVX512VL,AVX512CD,AVX512DQ
     set(ARROW_AVX512_FLAG "-march=skylake-avx512")
     # Append the avx2/avx512 subset option also, fix issue ARROW-9877 for homebrew-cpp
-    set(ARROW_AVX2_FLAG "${ARROW_AVX2_FLAG} -mavx2")
+    list(APPEND ARROW_AVX2_FLAGS "-mavx2")
     set(ARROW_AVX512_FLAG
         "${ARROW_AVX512_FLAG} -mavx512f -mavx512cd -mavx512vl -mavx512dq -mavx512bw")
     check_cxx_compiler_flag(${ARROW_SSE4_2_FLAG} CXX_SUPPORTS_SSE4_2)
   endif()
   if(CMAKE_SIZEOF_VOID_P EQUAL 8)
     # Check for AVX extensions on 64-bit systems only, as 32-bit support seems iffy
-    check_cxx_compiler_flag(${ARROW_AVX2_FLAG} CXX_SUPPORTS_AVX2)
+    list(JOIN ${ARROW_AVX2_FLAGS} " " ARROW_AVX2_FLAGS_COMMAND_LINE)
+    check_cxx_compiler_flag(${ARROW_AVX2_FLAGS_COMMAND_LINE} CXX_SUPPORTS_AVX2)
     if(MINGW)
       # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65782
       message(STATUS "Disable AVX512 support on MINGW for now")

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Jan 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants