Skip to content

Universal intrinsics implementation with RISC-V vector extension#18228

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
joy2myself:rvv
Dec 2, 2020
Merged

Universal intrinsics implementation with RISC-V vector extension#18228
opencv-pushbot merged 1 commit intoopencv:masterfrom
joy2myself:rvv

Conversation

@joy2myself
Copy link
Copy Markdown
Contributor

@joy2myself joy2myself commented Aug 31, 2020 •
edited
Loading

This PR From GSoC Project "Optimize OpenCV for RISC-V"

Project introduction

The main objective of the project is adding implementation of OpenCV wide universal intrinsics for RISC-V vector in OpenCV Hardware Acceleration Layer (HAL).
This PR is now working in progress.

Two previous PRs related to the project

Following two previous PRs mainly provided the toolchain files for cross-compiling OpenCV with target riscv64 on Linux platform. The first PR is with Clang and the second is with riscv-gnu-toolchain.
#17922
#18227

Main contents of my work

Added toolchain file for target riscv64-clang.
Added toolchain file for target riscv64-gcc.
Added universal intrinsics implementation with RISC-V vector extension.
Added some native intrinsics in C++ that are unsupported by compiler temporarily.

Current progress

Universal intrinsics implementation with RISC-V vector extension is almost done. But the way universal intrinsic vector types are declared right now needs some changes. See the following issue for specific reasons.
riscv-collab/riscv-gnu-toolchain#701
Use empty functions instead of missing native intrinsics temporarily.

Near future work

Change the universal intrinsic vector types with in-memory vectors and add extra loads/stores to get functional code.
Implement missing native intrinsics in C++ which are now still empty functions.

Long future work

Performance improvements. Maybe the current universal intrinsic framework need some change to fit scalable vector architecture.
RISC-V vector extension and rvv-intrinsics are still in the process of development. The compiler support is not completely ready at present and there may be changes in riscv-v-spec and rvv-intrinsics in the future. Therefore, this implementation needs to be maintained continuously according to the development of RISC-V.

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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to 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

@joy2myself joy2myself changed the title Universal intrinsics implementation with risc-v vector extension Universal intrinsics implementation with RISC-V vector extension Aug 31, 2020
@asmorkalov asmorkalov self-requested a review August 31, 2020 14:40
asmorkalov
asmorkalov requested changes Sep 2, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

/home/alexander/Projects/riscv/opencv/modules/core/test/test_intrin_utils.hpp:337:46: error: no matching function for call to 'v_reinterpret_as_f32(cv::hal_baseline::v_float64x2&)'
  337 |         v_float32 vf32 = v_reinterpret_as_f32(r1); out.a.clear(); v_store((float*)out.a.d, vf32); EXPECT_EQ(data.a, out.a);
      |                          ~~~~~~~~~~~~~~~~~~~~^~~~

modules/core/include/opencv2/core/hal/intrin_rvv.hpp

v_uint8x16() {}
explicit v_uint8x16(vuint8m1_t v) : val(v) {}
v_uint8x16(uchar v0, uchar v1, uchar v2, uchar v3, uchar v4, uchar v5, uchar v6, uchar v7,
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy constructor and operator= required for build for all v_int, v_uint, v_float, etc structures.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Sep 2, 2020 

/home/alexander/Projects/riscv/opencv/modules/core/test/test_intrin_utils.hpp:339:46: error: no matching function for call to 'v_reinterpret_as_f64(cv::hal_baseline::v_float32x4&)'
  339 |         v_float64 vf64 = v_reinterpret_as_f64(r1); out.a.clear(); v_store((double*)out.a.d, vf64); EXPECT_EQ(data.a, out.a);
      |

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Sep 2, 2020 

In file included from /home/alexander/Projects/riscv/opencv/modules/core/test/test_intrin128.simd.hpp:19,
                 from /home/alexander/Projects/riscv/opencv/modules/core/test/test_intrin.cpp:6:
/home/alexander/Projects/riscv/opencv/modules/core/test/test_intrin_utils.hpp: In instantiation of 'opencv_test::hal::intrin128::cpu_baseline::TheTest<R>& opencv_test::hal::intrin128::cpu_baseline::TheTest<R>::test_popcount() [with R = cv::hal_baseline::v_int8x16]':
/home/alexander/Projects/riscv/opencv/modules/core/test/test_intrin_utils.hpp:1641:24:   required from here
/home/alexander/Projects/riscv/opencv/modules/core/test/test_intrin_utils.hpp:829:35: error: conversion from 'cv::hal_baseline::v_int8x16' to non-scalar type 'opencv_test::hal::intrin128::cpu_baseline::Data<cv::hal_baseline::v_uint8x16>' requested
  829 |         Data<Ru> resB = v_popcount(a);
      |                         ~~~~~~~~~~^~~

@Qiyu8 Qiyu8 mentioned this pull request Sep 24, 2020
Merged
@joy2myself joy2myself force-pushed the rvv branch 2 times, most recently from bbc991b to a39a21c Compare November 2, 2020 10:56
@joy2myself joy2myself marked this pull request as ready for review November 16, 2020 06:19
@vpisarev vpisarev self-assigned this Dec 1, 2020
@vpisarev
Copy link
Copy Markdown
Contributor

vpisarev commented Dec 1, 2020

👍

@vpisarev vpisarev requested a review from asmorkalov December 1, 2020 22:00
@vpisarev vpisarev mentioned this pull request Dec 1, 2020
Closed
@vpisarev
Copy link
Copy Markdown
Contributor

vpisarev commented Dec 1, 2020

@asmorkalov, you requested some changes. Are you happy with what has been done. I tested this PR and from side I do not see any showstoppers, except for the two:

@joy2myself:

  1. please, squash the commits into one; I've sent you the instructions by e-mail.
  2. please, copy our checklist about licenses and other conditions from another pull request into the first message of this pull request and click the checkboxes. Without your "written" agreement with our conditions that we put into each pull request we cannot integrate your patch

@joy2myself joy2myself force-pushed the rvv branch 3 times, most recently from 9a0bb08 to d41c61a Compare December 2, 2020 06:14
@joy2myself
Copy link
Copy Markdown
Contributor Author

joy2myself commented Dec 2, 2020

Hi dear @asmorkalov and @vpisarev ,

I reverted some unnessesary commits before squash. I tested the current version and it works as well as before. Even so, I recommend that you test it again before merging to make sure there are no problems.

@opencv-pushbot opencv-pushbot merged commit 7d7d907 into opencv:master Dec 2, 2020
@vpisarev vpisarev mentioned this pull request Dec 11, 2020
Merged
6 tasks
@alalek alalek mentioned this pull request Apr 9, 2021
Merged
@hanliutong hanliutong mentioned this pull request Dec 27, 2021
Merged
6 tasks
@alalek alalek mentioned this pull request Sep 26, 2022
Merged
@alalek alalek mentioned this pull request Dec 6, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov Awaiting requested review from asmorkalov

Assignees

@vpisarev vpisarev

Labels

GSoC platform: riscv

Projects

None yet

Milestone

4.5.1

Development

Successfully merging this pull request may close these issues.

5 participants

@joy2myself @asmorkalov @vpisarev @opencv-pushbot @alalek