Repository navigation
[C++] Add RISC-V Vector (RVV) support and dynamic dispatch - #51340
zhanchangbao-sanechips wants to merge 8 commits into
Conversation
…patch Adds support for the RISC‑V Vector (RVV) extension in the C++ runtime SIMD dispatch machinery, analogous to the existing NEON/SVE support for AArch64, and paves the way for upcoming RVV‑related optimizations.
|
Thanks for opening a pull request! This pull request has been automatically converted to a draft because its title doesn't match Arrow's required format. If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or After updating the title, you can mark the pull request as ready for review. See also: |
|
Hi @zhanchangbao-sanechips , as you probably have in seen the SVE-handling code above your changes, we are using xsimd for instruction set detection. Can we ask you to do that for RVV too? (you may need to contribute the required additions to xsimd if not already available) |
I referred to my previous PR: apache/orc#2639. I will follow the same approach as the SVE handling and use xsimd for RVV instruction set detection. |
…patch Use xsimd's cpu_features for RVV detection in cpu_info.cc, mirroring the existing SVE handling. The bundled xsimd 14.2.0 already provides rvv()/rvv_size_bytes().
|
@zhanchangbao-sanechips Is this ready for review? |
Not ready yet. Some CI failures are unresolved. I was tied up with other work these days. |
Fix trailing whitespace in DefineOptions.cmake and reformat the CpuInfo RVV test in io_util_test.cc to satisfy pre-commit (clang-format / cmake-format).
I've been busy with other matters recently, as well as preparing for the ORC port to RISC-V and its performance optimization for a talk at this year's RISC-V Summit, which caused some delays. I'm now back to the work on porting and optimizing Arrow on RISC-V. Please take some time to review it when you can. Thank you. |
| #endif | ||
|
|
||
| #if defined(ARROW_HAVE_RVV) || defined(ARROW_HAVE_RUNTIME_RVV) | ||
| # define ARROW_DISPATCH_TARGET_RVV(func) \ |
There was a problem hiding this comment.
I don't think that is right, we want per-vector width macros just like for SVE.
There was a problem hiding this comment.
I don't think that is right, we want per-vector width macros just like for SVE.
Thank you for your review,you're right, and I've split RVV into per-vector-width macros to match SVE.
| elseif(ARROW_CPU_FLAG STREQUAL "riscv64") | ||
| set(ARROW_RVV_MARCH "rv64gcv") | ||
| check_cxx_compiler_flag("-march=${ARROW_RVV_MARCH}" CXX_SUPPORTS_RVV) | ||
| if(CXX_SUPPORTS_RVV AND ARROW_RUNTIME_SIMD_LEVEL MATCHES "^(RVV|MAX)$") |
There was a problem hiding this comment.
Just like SVE, we probably want RVV128 etc.
| "SVE128" # fixed size SVE | ||
| "SVE256" # " | ||
| "SVE512" # " | ||
| "RVV" |
There was a problem hiding this comment.
Just like SVE, we probably want RVV128 etc.
Split the single RVV SIMD dispatch target into per-vector-width RVV128/RVV256/RVV512, mirroring the existing SVE handling.
pitrou
left a comment
There was a problem hiding this comment.
Looks mostly good to me now, a couple suggestions.
| string(REGEX MATCH "[0-9]+" RVV_VECTOR_BITS ${ARROW_SIMD_LEVEL}) | ||
| if(RVV_VECTOR_BITS) | ||
| set(ARROW_HAVE_RVV${RVV_VECTOR_BITS} ON) | ||
| add_definitions(-DARROW_HAVE_RVV${RVV_VECTOR_BITS}) |
There was a problem hiding this comment.
Shouldn't we also enable a compiler option for the given vector size, as in SVE above?
| set(ARROW_HAVE_RVV${RVV_VECTOR_BITS} ON) | ||
| add_definitions(-DARROW_HAVE_RVV${RVV_VECTOR_BITS}) | ||
| else() | ||
| set(ARROW_HAVE_RVV ON) |
There was a problem hiding this comment.
Perhaps call it ARROW_HAVE_RVV_SIZELESS just like SVE above?
| message(WARNING "ARROW_SIMD_LEVEL=${ARROW_SIMD_LEVEL} not supported by Arm.") | ||
| endif() | ||
| endif() | ||
| if(ARROW_CPU_FLAG STREQUAL "riscv64") |
There was a problem hiding this comment.
Nit: add line break as above.
| if(ARROW_CPU_FLAG STREQUAL "riscv64") | |
| if(ARROW_CPU_FLAG STREQUAL "riscv64") |
Rationale for this change
Add initial RISC-V Vector (RVV) support to Arrow C++. This mirrors the existing SVE dynamic-dispatch support on AArch64, paves the way for upcoming RVV‑related optimizations, providing:
DispatchLevel::RVVentry so RVV-optimized kernels can be compiled with-march=rv64gcvand selected at runtime.ARROW_SIMD_LEVEL=RVVfor compile-time RVV andARROW_RUNTIME_SIMD_LEVEL=RVV|MAXfor runtime dispatch, with graceful fallback (NONE) when the toolchain lacks RVV support.What changes are included
cpp/src/arrow/util/cpu_info.h/.cc: addRVV/RVV128/RVV256/RVV512hardware flags; detect V extension viagetauxval(AT_HWCAP)and VLEN via thevlenbCSR on Linux/riscv64.cpp/src/arrow/util/dispatch_internal.h: addDispatchLevel::RVVandARROW_DISPATCH_TARGET_RVVmacro, wired intoDynamicDispatch::IsSupported.cpp/cmake_modules/DefineOptions.cmake: addRVVtoARROW_SIMD_LEVELandARROW_RUNTIME_SIMD_LEVELoption lists.cpp/cmake_modules/SetupCxxFlags.cmake: riscv64 handling — check-march=rv64gcv, setARROW_HAVE_RVV(compile-time) /ARROW_HAVE_RUNTIME_RVV(runtime), defaultARROW_SIMD_LEVEL=NONE.cpp/src/arrow/util/simd.h: include<riscv_vector.h>when RVV is enabled.cpp/src/arrow/util/io_util_test.cc: extendCpuInfo.Basicto assert RVV detection and a matching VLEN flag.Are these changes tested?
Yes. Built and unit-tested on Sophgo SG2044 board with GCC 14.3.1.

Built with --DARROW_SIMD_LEVEL=RVV -DARROW_RUNTIME_SIMD_LEVEL=RVV, config passed.
All unittest passed.
Are there any user-facing changes?
No user-facing API changes; adds build-time options
ARROW_SIMD_LEVEL=RVVandARROW_RUNTIME_SIMD_LEVEL=RVV|MAXfor riscv64.GitHub Issue: #51334