Skip to content

Fix RISC-V HAL solve/SVD and BGRtoLab#27046

Merged
asmorkalov merged 3 commits intoopencv:4.xfrom
amane-ame:solve_color_fix
Mar 21, 2025
Merged

Fix RISC-V HAL solve/SVD and BGRtoLab#27046
asmorkalov merged 3 commits intoopencv:4.xfrom
amane-ame:solve_color_fix

Conversation

@amane-ame
Copy link
Copy Markdown
Contributor

@amane-ame amane-ame commented Mar 11, 2025

Closes #27044.

Also suppressed some warnings in other 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

@amane-ame amane-ame changed the title Fix RISC-V HAL solve/SVD and BGRtoLab. Fix RISC-V HAL solve/SVD and BGRtoLab Mar 11, 2025
@amane-ame
Copy link
Copy Markdown
Contributor Author

cc @asmorkalov This can prevent potential test failure in 5.x.

@asmorkalov
Copy link
Copy Markdown
Contributor

cc @mshabunin

@asmorkalov asmorkalov added this to the 4.12.0 milestone Mar 12, 2025
@fengyuentau fengyuentau self-requested a review March 13, 2025 08:46
@mshabunin
Copy link
Copy Markdown
Contributor

@amane-ame , please resolve merge conflict.

@amane-ame
Copy link
Copy Markdown
Contributor Author

@mshabunin Done.

@asmorkalov
Copy link
Copy Markdown
Contributor

roundeven, roundevenf, roundevenl are available since C++23. Not sure if it's universal solution. @mshabunin What so you think?

@fengyuentau
Copy link
Copy Markdown
Member

My performance results perf.zip

@fengyuentau
Copy link
Copy Markdown
Member

@amane-ame I suggest you to solve conflicts with the following cli git commands instead of the GitHub webpage tool to ensure that your commits are continuous in your branch.

git remote add upstream https://github.com/opencv/opencv
git fetch upstream
git rebase upstream/4.x

@amane-ame
Copy link
Copy Markdown
Contributor Author

@fengyuentau Uh, rebase will mess up the order of commits and comments in PR, because GitHub sorts them by commit date. If you are okay with this, it's fine.

@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Mar 18, 2025

roundeven, roundevenf, roundevenl are available since C++23. Not sure if it's universal solution. @mshabunin What so you think?

Let's restore rint + fesetround - it should be thread-local. Perhaps we can try to reduce number of rounding-mode switching by wrapping whole table initialization in fesetround calls.

See also: https://en.cppreference.com/w/cpp/numeric/fenv

amane-ame and others added 2 commits March 18, 2025 17:56
Co-authored-by: Liutong HAN <liutong2020@iscas.ac.cn>
Co-authored-by: Liutong HAN <liutong2020@iscas.ac.cn>
@amane-ame
Copy link
Copy Markdown
Contributor Author

amane-ame commented Mar 18, 2025

@mshabunin I can use std::remainder instead of roundeven and std::rint. It's fenv irrelevant.

@mshabunin
Copy link
Copy Markdown
Contributor

@amane-ame , how does it affect performance? Or is the table initialized just once and it doesn't matter much?

I thought that something like this would be good from both performance and safety standpoints:

int old_mode = fesetround(...);
// init whole table using rint
fesetround(old_mode);

@amane-ame
Copy link
Copy Markdown
Contributor Author

@mshabunin std::remainder should not have any performance impact, since LabTable will only be initialized once in a process.

Co-authored-by: Liutong HAN <liutong2020@iscas.ac.cn>
@asmorkalov asmorkalov self-assigned this Mar 21, 2025
@asmorkalov asmorkalov merged commit 46bd22a into opencv:4.x Mar 21, 2025
27 of 28 checks passed
@asmorkalov asmorkalov mentioned this pull request Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

calib3d and photo test failures on RISC-V platforms

4 participants