Upgrade RISC-V Vector intrinsic and cleanup the obsolete RVV backend. by hanliutong · Pull Request #25883 · opencv/opencv

hanliutong · 2024-07-08T11:29:55Z

This patch upgrade RISC-V Vector intrinsic from v0.10 to v0.12/v1.0:

Update cmake check and options;
Upgrade RVV implement for Universal Intrinsic;
Upgrade RVV optimized DNN kernel.
Cleanup the obsolete RVV backend (intrin_rvv.hpp) and compatable header file.

With this patch, RVV backend require Clang 17+ or GCC 14+ (which means __riscv_v_intrinsic >= 12000, see https://godbolt.org/z/es7ncETE3)

This patch is test with Clang 17.0.6 (require extra -DWITH_PNG=OFF due to ICE), Clang 18.1.8 and GCC 14.1.0 on QEMU and k230 (with --gtest_filter="*hal_*").

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

asmorkalov · 2024-07-09T08:04:37Z

RISC-V config in CI failed:

-- Performing Test HAVE_CXX_MARCH_RV64GCV (check file: cmake/checks/cpu_rvv.cpp)
CMake Error at cmake/OpenCVCompilerOptimizations.cmake:602 (message):
-- Performing Test HAVE_CXX_MARCH_RV64GCV - Failed
  Required baseline optimization is not supported: RVV
  (CPU_BASELINE_REQUIRE=RVV)
Call Stack (most recent call first):
  cmake/OpenCVCompilerOptions.cmake:335 (include)
-- RVV is not supported by C++ compiler
-- Optimization RVV is not available, skipped
-- Performing Test HAVE_CPU_BASELINE_FLAGS
  CMakeLists.txt:687 (include)


-- Performing Test HAVE_CPU_BASELINE_FLAGS - Success

hanliutong · 2024-07-09T08:09:53Z

--   C/C++:
--     Built as dynamic libs?:      YES
--     C++ standard:                11
--     C++ Compiler:                /opt/rvv-llvm/bin/clang++  (ver 16.0.6)

clang 16 work with RVV intrinsic v0.11, but we need v0.12 or higher.

With this patch, RVV backend require Clang 17+ or GCC 14+ (which means __riscv_v_intrinsic >= 12000, see https://godbolt.org/z/es7ncETE3)

Maybe we need upgrade LLVM version on the CI server

asmorkalov · 2024-07-09T08:27:46Z

@mshabunin could you take a look?

mshabunin · 2024-07-12T10:22:08Z

Updated workflow (use clang-17 in precommit): opencv/ci-gha-workflow#176

asmorkalov · 2024-07-12T13:18:06Z

@mshabunin I merged CI patch and re-triggered RISC-V jobs.

mshabunin · 2024-07-13T15:39:04Z

Restarting pipelines didn't help. Perhaps we need to update PR itself.

hanliutong · 2024-07-13T16:17:57Z

Should I push an empty commit or do something else?

mshabunin · 2024-07-15T08:58:28Z

@hanliutong , yes, please push a new commit or rebase the whole branch.

hanliutong · 2024-07-15T09:26:35Z

@mshabunin rebased, please re-trigger the CI

cmake/checks/cpu_rvv.cpp

mshabunin · 2024-07-16T22:01:50Z

There is another issue with T-Head compiler - now it can not compile dnn inline kernels, because they use new intrinsics version:

/work/opencv/modules/dnn/src/layers/cpu_kernels/conv_depthwise.simd.hpp:267:26: error: '__riscv_vsetvl_e32m8' was not declared in this scope
  267 |                     vl = __riscv_vsetvl_e32m8(avl);
      |                          ^~~~~~~~~~~~~~~~~~~~

It seems that now both CV_RVV and CV_RVV071 are defined in cv_cpu_dispatch.h:

this one set by the macro passed to the compiler by the cmake toolchain file (__riscv_vector_071)

opencv/modules/core/include/opencv2/core/cv_cpu_dispatch.h

Lines 149 to 152 in 53a5b85

    
           #if defined(__riscv) && defined(__riscv_vector) && defined(__riscv_vector_071) 
        
           # include<riscv_vector.h> 
        
           # define CV_RVV071 1 
        
           #endif

and another one set by CPU feature detection mechanism (CV_CPU_COMPILE_RVV)

opencv/modules/core/include/opencv2/core/cv_cpu_dispatch.h

Lines 186 to 189 in 53a5b85

    
           #if defined CV_CPU_COMPILE_RVV 
        
           #  define CV_RVV 1 
        
           #  include <riscv_vector.h> 
        
           #endif

Let's try to combine both pieces into one, so that inline snippets in dnn wouldn't be compiled. Something like this:

#ifdef CV_CPU_COMPILE_RVV
#  ifdef __riscv_vector_071
#    define CV_RVV071 1
#  else
#    define CV_RVV 1
#  endif
#include <riscv_vector.h>
#endif

mshabunin · 2024-07-17T12:24:27Z

There are some issues left with XuanTie toolchain (https://github.com/XUANTIE-RV/xuantie-gnu-toolchain/, I used prebuilt 2.10.0 - https://www.xrvm.cn/community/download?id=4333581795569242112)

Command line:

$ PATH=/work/chains/xuantie-toolchain-2.10.0/bin:${PATH} \
  cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_TOOLCHAIN_FILE=/opencv/platforms/linux/riscv64-071-gcc.toolchain.cmake -DBUILD_SHARED_LIBS=OFF -DWITH_OPENCL=OFF -DCORE=C908V -DCMAKE_BUILD_TYPE=Release ../opencv
$ ninja

Linker errors:

/work/chains/xuantie-toolchain-2.10.0/bin/../lib/gcc/riscv64-unknown-linux-gnu/10.4.0/../../../../riscv64-unknown-linux-gnu/bin/ld: lib/libopencv_dnn.a(convolution_layer.cpp.o): in function `.L62':
convolution_layer.cpp:(.text._ZNK2cv3dnn22DeConvolutionLayerImpl13MatMulInvokerclERKNS_5RangeE[_ZNK2cv3dnn22DeConvolutionLayerImpl13MatMulInvokerclERKNS_5RangeE]+0x44e): undefined reference to `cv::dnn::opt_RVV::fastGEMM(float const*, unsigned long, float const*, unsigned long, float*, unsigned long, int, int, int)'
/work/chains/xuantie-toolchain-2.10.0/bin/../lib/gcc/riscv64-unknown-linux-gnu/10.4.0/../../../../riscv64-unknown-linux-gnu/bin/ld: lib/libopencv_dnn.a(fully_connected_layer.cpp.o): in function `.L51':
fully_connected_layer.cpp:(.text._ZNK2cv3dnn23FullyConnectedLayerImpl14FullyConnectedclERKNS_5RangeE[_ZNK2cv3dnn23FullyConnectedLayerImpl14FullyConnectedclERKNS_5RangeE]+0x134): undefined reference to `cv::dnn::opt_RVV::fastGEMM1T(float const*, float const*, unsigned long, float const*, float*, int, int)'
/work/chains/xuantie-toolchain-2.10.0/bin/../lib/gcc/riscv64-unknown-linux-gnu/10.4.0/../../../../riscv64-unknown-linux-gnu/bin/ld: lib/libopencv_dnn.a(conv_depthwise.cpp.o): in function `.L0 ':
conv_depthwise.cpp:(.text._ZNSt17_Function_handlerIFvRKN2cv5RangeEEZNS0_3dnn12runDepthwiseERKNS0_11_InputArrayERKNS0_12_OutputArrayERKNS0_3PtrINS5_8FastConvEEEPNS5_14dnn4_v2024052115ActivationLayerERKSt6vectorIfSaIfEEbEUlS3_E_E9_M_invokeERKSt9_Any_dataS3_+0x1b2): undefined reference to `cv::dnn::opt_RVV::fastDepthwiseConv(float const*, int, int, int, int, int, int, int, int, float const*, float const*, float const*, int, int, float*, int, int, int)'
collect2: error: ld returned 1 exit status

It seems that optimized functions are declared/called but not defined with current preprocessor macros state.

hanliutong · 2024-07-18T06:22:12Z

Fixed by using macro CV_TRY_RVV and CV_RVV together when calling in-place optimizing functions:

--- #if CV_TRY_RVV
+++ #if CV_TRY_RVV && CV_RVV

then the in-place optimizing function only called with the upsteam toolchain which using new RVV intrinsic.

#if CV_TRY_RVV && CV_RVV
    if(useRVV)
        opt_RVV::fastConv(wptr, wstep, biasptr, rowbuf0, data_out0 + ofs0,
                        outShape, bsz, vsz, vsz_a, outZp, multptr, cn0 == 0, cn1 == inpCn);
    else
#endif

Note: macro state:

Macro	state with upstream	state with XuanTie
`CV_CPU_COMPILE_RVV`	defined	defined
`CV_RVV`	1	0
`CV_RVV071`	0	1
`CV_TRY_RVV`	1	1

asmorkalov · 2024-07-19T06:12:33Z

@hanliutong @mshabunin DO we need the table above somewhere in code as comment?

hanliutong · 2024-07-19T07:14:01Z

added on cv_cpu_dispatch.h

Upgrade RISC-V Vector intrinsic and cleanup the obsolete RVV backend. opencv#25883 This patch upgrade RISC-V Vector intrinsic from `v0.10` to `v0.12`/`v1.0`: - Update cmake check and options; - Upgrade RVV implement for Universal Intrinsic; - Upgrade RVV optimized DNN kernel. - Cleanup the obsolete RVV backend (`intrin_rvv.hpp`) and compatable header file. With this patch, RVV backend require Clang 17+ or GCC 14+ (which means `__riscv_v_intrinsic >= 12000`, see https://godbolt.org/z/es7ncETE3) This patch is test with Clang 17.0.6 (require extra `-DWITH_PNG=OFF` due to ICE), Clang 18.1.8 and GCC 14.1.0 on QEMU and k230 (with `--gtest_filter="*hal_*"`). ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [ ] The PR is proposed to the proper branch - [ ] There is a reference to the original bug report and related work - [ ] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [ ] The feature is well documented and sample code can be built with the project CMake

hanliutong mentioned this pull request Jul 8, 2024

Add FP16 support for RISC-V #25743

Merged

6 tasks

asmorkalov requested a review from mshabunin July 8, 2024 12:17

asmorkalov added platform: riscv cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) labels Jul 8, 2024

asmorkalov assigned mshabunin Jul 8, 2024

asmorkalov added this to the 4.11.0 milestone Jul 8, 2024

hanliutong added 2 commits July 15, 2024 09:23

Upgrade RVV Intrinsic.

0285318

Clean up the obsolete RVV backend.

c750f57

hanliutong force-pushed the rvv-intrin-upgrade branch from c0974f5 to c750f57 Compare July 15, 2024 09:23

mshabunin reviewed Jul 16, 2024

View reviewed changes

cmake/checks/cpu_rvv.cpp Outdated Show resolved Hide resolved

Update cpu_rvv.cpp

e8a1297

Update cv_cpu_dispatch.h

f424956

hanliutong requested a review from mshabunin July 17, 2024 02:13

Fix RVV related macro.

3c26069

mshabunin approved these changes Jul 18, 2024

View reviewed changes

Add comments for RVV-related macro.

b7db426

asmorkalov approved these changes Jul 19, 2024

View reviewed changes

asmorkalov merged commit b5ea321 into opencv:4.x Jul 19, 2024

asmorkalov mentioned this pull request Jul 25, 2024

5.x merge 4.x #25953

Merged

mshabunin mentioned this pull request Feb 11, 2025

[RISCV] Incompatible change for RVV intrinsics #22608

Closed

Uh oh!

Conversation

hanliutong commented Jul 8, 2024

Pull Request Readiness Checklist

Uh oh!

asmorkalov commented Jul 9, 2024

Uh oh!

hanliutong commented Jul 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asmorkalov commented Jul 9, 2024

Uh oh!

mshabunin commented Jul 12, 2024

Uh oh!

asmorkalov commented Jul 12, 2024

Uh oh!

mshabunin commented Jul 13, 2024

Uh oh!

hanliutong commented Jul 13, 2024

Uh oh!

mshabunin commented Jul 15, 2024

Uh oh!

hanliutong commented Jul 15, 2024

Uh oh!

Uh oh!

mshabunin commented Jul 16, 2024

Uh oh!

mshabunin commented Jul 17, 2024

Uh oh!

hanliutong commented Jul 18, 2024

Uh oh!

asmorkalov commented Jul 19, 2024

Uh oh!

hanliutong commented Jul 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hanliutong commented Jul 9, 2024 •

edited

Loading