You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "HyukjinKwon (via GitHub)" <gi...@apache.org> on 2026/01/26 00:09:08 UTC
[PR] GH-48980: [C++] Use COMPILE_OPTIONS instead of deprecated COMPILE_FLAGS [arrow]
HyukjinKwon opened a new pull request, #48981:
URL: https://github.com/apache/arrow/pull/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.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48980: [C++] Use COMPILE_OPTIONS instead of deprecated COMPILE_FLAGS [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #48981:
URL: https://github.com/apache/arrow/pull/48981#discussion_r2726058248
##########
cpp/src/arrow/CMakeLists.txt:
##########
@@ -322,22 +322,26 @@ endfunction()
macro(append_runtime_avx2_src SRCS SRC)
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}")
Review Comment:
Can we define `ARROW_AVX2_FLAG` as a list not string something like the following?
```diff
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")
```
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48980: [C++] Use COMPILE_OPTIONS instead of deprecated COMPILE_FLAGS [arrow]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #48981:
URL: https://github.com/apache/arrow/pull/48981#issuecomment-3822571469
Thank you!!!
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48980: [C++] Use COMPILE_OPTIONS instead of deprecated COMPILE_FLAGS [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #48981:
URL: https://github.com/apache/arrow/pull/48981
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48980: [C++] Use COMPILE_OPTIONS instead of deprecated COMPILE_FLAGS [arrow]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #48981:
URL: https://github.com/apache/arrow/pull/48981#discussion_r2726487346
##########
cpp/src/arrow/CMakeLists.txt:
##########
@@ -322,22 +322,26 @@ endfunction()
macro(append_runtime_avx2_src SRCS SRC)
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}")
Review Comment:
Will do!
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48980: [C++] Use COMPILE_OPTIONS instead of deprecated COMPILE_FLAGS [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #48981:
URL: https://github.com/apache/arrow/pull/48981#issuecomment-3797437230
:warning: GitHub issue #48980 **has been automatically assigned in GitHub** to PR creator.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48980: [C++] Use COMPILE_OPTIONS instead of deprecated COMPILE_FLAGS [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #48981:
URL: https://github.com/apache/arrow/pull/48981#issuecomment-3810846418
Revision: 3a2165c6a05cf1df55ea7175f4d77387f3987e85
Submitted crossbow builds: [ursacomputing/crossbow @ actions-44d4d33360](https://github.com/ursacomputing/crossbow/branches/all?query=actions-44d4d33360)
|Task|Status|
|----|------|
|example-cpp-minimal-build-static|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022983/job/61730152197)|
|example-cpp-minimal-build-static-system-dependency|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022553/job/61730150705)|
|example-cpp-tutorial|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022046/job/61730148491)|
|test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/21437023215/job/61730152771)|
|test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022137/job/61730148909)|
|test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022928/job/61730151900)|
|test-debian-12-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/21437023592/job/61730154002)|
|test-debian-12-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/21437023100/job/61730152422)|
|test-debian-experimental-cpp-gcc-15|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022542/job/61730150400)|
|test-fedora-42-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022021/job/61730148440)|
|test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022953/job/61730152118)|
|test-ubuntu-22.04-cpp-20|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022604/job/61730150800)|
|test-ubuntu-22.04-cpp-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/21437023581/job/61730154113)|
|test-ubuntu-22.04-cpp-emscripten|[](https://github.com/ursacomputing/crossbow/actions/runs/21437023535/job/61730153980)|
|test-ubuntu-22.04-cpp-no-threading|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022585/job/61730151211)|
|test-ubuntu-24.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022101/job/61730148692)|
|test-ubuntu-24.04-cpp-bundled-offline|[](https://github.com/ursacomputing/crossbow/actions/runs/21437021997/job/61730148639)|
|test-ubuntu-24.04-cpp-gcc-13-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022844/job/61730151542)|
|test-ubuntu-24.04-cpp-gcc-14|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022762/job/61730151447)|
|test-ubuntu-24.04-cpp-minimal-with-formats|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022499/job/61730150420)|
|test-ubuntu-24.04-cpp-thread-sanitizer|[](https://github.com/ursacomputing/crossbow/actions/runs/21437022610/job/61730150886)|
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48980: [C++] Use COMPILE_OPTIONS instead of deprecated COMPILE_FLAGS [arrow]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on PR #48981:
URL: https://github.com/apache/arrow/pull/48981#issuecomment-3810834495
@github-actions crossbow submit -g cpp
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48980: [C++] Use COMPILE_OPTIONS instead of deprecated COMPILE_FLAGS [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #48981:
URL: https://github.com/apache/arrow/pull/48981#discussion_r2738861262
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -494,7 +495,8 @@ if(ARROW_CPU_FLAG STREQUAL "x86")
if(NOT CXX_SUPPORTS_AVX2)
message(FATAL_ERROR "AVX2 required but compiler doesn't support it.")
endif()
- set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} ${ARROW_AVX2_FLAG}")
+ list(JOIN ARROW_AVX2_FLAGS " " ARROW_AVX2_FLAGS_COMMAND_LINE)
+ set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} ${ARROW_AVX2_FLAGS_COMMAND_LINE}")
Review Comment:
```suggestion
string(APPEND CXX_COMMON_FLAGS "${ARROW_AVX2_FLAGS_COMMAND_LINE}")
```
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org