kou commented on code in PR #40927:
URL: https://github.com/apache/arrow/pull/40927#discussion_r1547020343
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -653,6 +653,18 @@ if(NOT WIN32 AND NOT APPLE)
else()
message(STATUS "Using the default linker because mold isn't found")
endif()
+ elseif(ARROW_USE_LLD AND CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
Review Comment:
We don't need to put `ARROW_USE_LLD` check into `if(NOT WIN32 AND NOT
APPLE)` block because `lld` works on Windows and Apple too.
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -653,6 +653,18 @@ if(NOT WIN32 AND NOT APPLE)
else()
message(STATUS "Using the default linker because mold isn't found")
endif()
+ elseif(ARROW_USE_LLD AND CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+ # For GCC make sure the compiler flag is supported and use it if so for
linking.
+ set(LLD_LINKER_FLAGS "-fuse-ld=lld")
+ check_cxx_compiler_flag("${LLD_LINKER_FLAGS}" CXX_SUPPORTS_LLD)
+ if(CXX_SUPPORTS_LLD)
Review Comment:
It seems that we can use `if(CMAKE_CXX_COMPILER_VERSION
VERSION_GREATER_EQUAL "9.1.0")` because GCC 8.5.0 document doesn't have
`-fuse-ld=lld` and 9.1.0 document has it:
* 8.5.0: https://gcc.gnu.org/onlinedocs/gcc-8.5.0/gcc/Link-Options.html
* 9.1.0: https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Link-Options.html
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -653,6 +653,18 @@ if(NOT WIN32 AND NOT APPLE)
else()
message(STATUS "Using the default linker because mold isn't found")
endif()
+ elseif(ARROW_USE_LLD AND CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+ # For GCC make sure the compiler flag is supported and use it if so for
linking.
+ set(LLD_LINKER_FLAGS "-fuse-ld=lld")
+ check_cxx_compiler_flag("${LLD_LINKER_FLAGS}" CXX_SUPPORTS_LLD)
+ if(CXX_SUPPORTS_LLD)
+ message(STATUS "Using optional LLVM LLD linker")
+ string(APPEND CMAKE_EXE_LINKER_FLAGS " ${LLD_LINKER_FLAGS}")
+ string(APPEND CMAKE_MODULE_LINKER_FLAGS " ${LLD_LINKER_FLAGS}")
+ string(APPEND CMAKE_SHARED_LINKER_FLAGS " ${LLD_LINKER_FLAGS}")
+ else()
Review Comment:
Could you also add support for `"Clang"`?
`clang++` may use GNU ld by default. So this option is useful for `"Clang"`
too.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
