-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[cpu] riscv64: update intrinsics #2929
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Zhang fei <zhangfei@iscas.ac.cn>
Signed-off-by: Zhang fei <zhangfei@iscas.ac.cn>
Signed-off-by: Zhang fei <zhangfei@iscas.ac.cn>
Signed-off-by: Zhang fei <zhangfei@iscas.ac.cn>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks. Do you know what becomes the minimal compiler version for GCC/clang after this change?
Tagging @pazamelin @aaronfranke @dmitry-gorokhov for awareness.
Signed-off-by: Zhang fei <zhangfei@iscas.ac.cn>
The following is an excerpt from rvv-intrinsic-doc. After the introduction of this PR, the minimum GCC version required will be GCC 14, and the minimum Clang version will be Clang 17. Furthermore, below is the configuration output when compiling OneDNN with different versions of the compiler. |
Thanks for the ping, great to see oneDNN's progress on RISC-V. Note that the vector extension is still not common, so compiling must be supported without it, and that appears to be the case here thankfully. Otherwise it's beyond my expertise to review this. |
cmake/platform.cmake
Outdated
#endif | ||
#if defined(__riscv_v_intrinsic) && __riscv_v_intrinsic < 12000 | ||
#error \"Wrong intrinsics version, v0.12 or higher is required for gcc or clang\" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#error \"Wrong intrinsics version, v0.12 or higher is required for gcc or clang\" | |
#error \"RISC-V intrinsics v0.12 or higher is required\" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Signed-off-by: Zhang fei <zhangfei@iscas.ac.cn>
Description
Currently, oneDNN can be compiled normally on RISC-V architecture platforms, but when using newer compilers and enabling vector extensions, it does not compile RVV-related code (specifically, the code introduced in #1521). Below is the configuration output information regarding RVV when using clang-17:
export CC=clang-17 CXX=clang++-17 cmake .. -DDNNL_CPU_RUNTIME=SEQ -DCMAKE_CXX_FLAGS=-march=rv64gcv -DCMAKE_C_FLAGS=-march=rv64gcv
This is because the RVV intrinsics in #1521 are based on the old version specifications, and the new versions all have the __riscv64 prefix, as detailed in rvv-intrinsic-doc. Currently, most open-source software projects (such as OpenCV, OpenBLAS, and SLEEF) have already adopted the new RVV intrinsics.
Based on #1521, the following modifications were made:
These changes enable Clang 17+ or GCC 14+ to properly compile RVV-related code, while older compiler versions will fall back to generic implementations.
Checklist
General
make test
andmake test_benchdnn_*
) pass locally for each commit?Tested on both QEMU and Banana Pi BPI-F3 (vector length is 256 bits). Due to the long testing time required for
matmul
, it has been omitted here, as this PR only modifies the names of intrinsics. Below are the test results:result.log