Skip to content

Fix issues in RISC-V Vector (RVV) Universal Intrinsic#27006

Merged
asmorkalov merged 4 commits intoopencv:4.xfrom
hanliutong:rvv-fix-ui-1024
Mar 4, 2025
Merged

Fix issues in RISC-V Vector (RVV) Universal Intrinsic#27006
asmorkalov merged 4 commits intoopencv:4.xfrom
hanliutong:rvv-fix-ui-1024

Conversation

@hanliutong
Copy link
Copy Markdown
Contributor

@hanliutong hanliutong commented Mar 3, 2025

This PR aims to make opencv_test_core pass on RVV, via following two parts:

  1. Fix bug in Universal Intrinsic when VLEN >= 512:
  1. Temporary fix Core_ConvertScale/ElemWiseTest.accuracy/0 test crashes with sigsegv on risc-v rvv with GCC 14.2 #26936
  • Disable 3 Universal Intrinsic code blocks on GCC
  • This is just a temporary fix until we figure out if it's our issue or GCC/something else's

This patch is tested under the following conditions:

  • Compier: GCC 14.2, Clang 19.1.7
  • Device: Muse-Pi (VLEN=256), QEMU (VLEN=512, 1024)

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 asmorkalov added this to the 4.12.0 milestone Mar 3, 2025
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Mar 3, 2025

I confirm that item 2 has been resolved.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Mar 3, 2025

@hanliutong Please take a look on warnings with MS Visual Studio.

asmorkalov
asmorkalov approved these changes Mar 4, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

👍

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Mar 4, 2025

@fengyuentau Please take a look too.

@asmorkalov asmorkalov self-assigned this Mar 4, 2025
@asmorkalov asmorkalov requested a review from fengyuentau March 4, 2025 06:30
fengyuentau
fengyuentau reviewed Mar 4, 2025
View reviewed changes
@fengyuentau
Copy link
Copy Markdown
Member

fengyuentau commented Mar 4, 2025

As for #26936, most probably it is due to gcc because I did not see the problem with clang.

OpenCV version: 4.12.0-dev
OpenCV VCS version: 4.11.0-174-gdbd3ef9a6f
Build type: Release
Compiler: /home/tao/workspace/fytao/k1/spacemit-toolchain-linux-glibc-x86_64-v1.0.4/bin/clang++  (ver 18.1.8)
Algorithm hint: ALGO_HINT_ACCURATE
HAL: YES (HAL RVV (ver 0.0.1))
Parallel framework: pthreads (nthreads=8)
CPU features: RVV
TEST: Skip tests with tags: 'mem_6gb', 'verylong'
Note: Google Test filter = Core_ConvertScale/ElemWiseTest.accuracy/0
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from Core_ConvertScale/ElemWiseTest
[ RUN      ] Core_ConvertScale/ElemWiseTest.accuracy/0, where GetParam() = 16-byte object <20-89 9B-E9 2A-00 00-00 A0-8A 9B-E9 2A-00 00-00>
[       OK ] Core_ConvertScale/ElemWiseTest.accuracy/0 (2184 ms)
[----------] 1 test from Core_ConvertScale/ElemWiseTest (2184 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (2186 ms total)
[  PASSED  ] 1 test.

@asmorkalov asmorkalov merged commit 97abffb into opencv:4.x Mar 4, 2025
27 of 28 checks passed
opencv-alalek
opencv-alalek reviewed Mar 4, 2025
View reviewed changes
modules/core/test/test_intrin_utils.hpp
{
auto c_signed = dataC.as_int(i);
dataC[i] = (LaneType)(c_signed == 0 ? -1 : -std::abs(c_signed));
}
Copy link
Copy Markdown
Contributor

@opencv-alalek opencv-alalek Mar 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why do we need to change that?

It is code for testing, all checks are below.
Code of input preparation - so it is OK.

Copy link
Copy Markdown
Contributor Author

@hanliutong hanliutong Mar 5, 2025

Choose a reason for hiding this comment

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

Because we need dataC to be all negative numbers, then v_check_all(c) will be true.

R c = dataC;
EXPECT_EQ(true, v_check_all(c));

The previous dataC *= (LaneType)-1; cannot handle the case where the length of dataC greater than 128 for int_8

@hanliutong hanliutong deleted the rvv-fix-ui-1024 branch March 6, 2025 06:10
@asmorkalov asmorkalov mentioned this pull request Mar 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fengyuentau fengyuentau fengyuentau left review comments

@opencv-alalek opencv-alalek opencv-alalek left review comments

@asmorkalov asmorkalov asmorkalov approved these changes

Assignees

@asmorkalov asmorkalov

Labels

bug platform: riscv

Projects

None yet

Milestone

4.12.0

Development

Successfully merging this pull request may close these issues.

Core_ConvertScale/ElemWiseTest.accuracy/0 test crashes with sigsegv on risc-v rvv with GCC 14.2

4 participants

@hanliutong @asmorkalov @fengyuentau @opencv-alalek