Add option to enable "sve" optimization level on armv9 #830
Conversation
Signed-off-by: Lizhen You <lyou@nvidia.com>
cpp/cmake/modules/FindSVE.cmake
Outdated
|
|
||
| # Check for SVE support | ||
| message("Checking for SVE support") | ||
| SET(CMAKE_REQUIRED_FLAGS "-march=armv9-a+sve") |
There was a problem hiding this comment.
Please use lowercase for the CMake function names, like set(...), if(...), else(), mark_as_advanced(...).
There was a problem hiding this comment.
I just found the FindAVX.cmake is using the uppercase as well; So it is reasonable to use uppercase for FindSVE.cmake for code consistency?
in FindAVX.cmake:
ACRO(CHECK_SSE lang type flags)
SET(__FLAG_I 1)
SET(CMAKE_REQUIRED_FLAGS_SAVE ${CMAKE_REQUIRED_FLAGS})
FOREACH(__FLAG ${flags})
IF(NOT ${lang}${type}FOUND)
SET(CMAKE_REQUIRED_FLAGS ${__FLAG})
CHECK_CXX_SOURCE_RUNS("${${type}CODE}" ${lang}HAS${type}${__FLAG_I})
IF(${lang}HAS${type}${__FLAG_I})
SET(${lang}${type}FOUND
TRUE
CACHE BOOL "${lang} ${type} support"
)
SET(${lang}${type}_FLAGS
"${__FLAG}"
CACHE STRING "${lang} ${type} flags"
)
ENDIF()
MATH(EXPR __FLAG_I "${__FLAG_I}+1")
ENDIF()
ENDFOREACH()
There was a problem hiding this comment.
Let's prefer "good CMake style" over consistency here, and go lower-case. Maybe as a follow-up we can fix FindAVX.cmake.
There was a problem hiding this comment.
Does the latest code look good to you? The "sve" optimization flag can enhance the performance on AArch64 machine significantly which is around 17% in my run. Hope we can integrate this flag in soon time.
There was a problem hiding this comment.
@LizYou please see my comment below about the style checker (git pre-commit hooks). This PR cannot be merged while the style checker is not passing.
cjnolet
added
improvement
|
/ok to test 0d939b1 |
|
@LizYou it looks like we have some style checker errors. We use Git pre-commit hooks to automatically format code changes so they pass the style checker upon commit. Please see the following section of out contributor guide for information on how to configure pre-commit hooks. |
|
/ok to test f4d80ce |
No description provided.