Resolve Compilation Error for v_func Function in SIMD Emulator by WanliZhong · Pull Request #25891 · opencv/opencv

WanliZhong · 2024-07-09T16:47:01Z

This PR tries to fix the compilation error introduced by #24941
Error detail: #24941 (comment)
Merged after #26109

Additionally, this PR adds the missing documentation for the v_log function.

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

force_builders=Custom
build_image:Custom=simd-emulator
buildworker:Custom=linux-1,linux-4

WanliZhong · 2024-07-09T17:07:36Z

@asmorkalov I removed the macros OPENCV_HAL_MATH_HAVE_XXX defined in intrin_cpp.hpp. This change will fix the error. I passed the compilation and the tests in the local machine with -DCV_FORCE_SIMD128_CPP=1

vpisarev · 2024-07-10T06:38:03Z

@opencv-alalek, could you please review it? Frankly speaking, I don't understand logic behind all those myriads of flags that we have, so you are the best person to review it

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

WanliZhong · 2024-08-10T12:46:50Z

@opencv-alalek @vpisarev @asmorkalov Hi, sorry for the delayed fix. I finally found out why this error occurs. That's because the intrin_cpp.hpp will only generate functions for SIMD128 (v_float32x4 and v_float64x2).

However, on AVX2, AVX512, and LASX platforms, SIMD256 or SIMD512 are used, which has more lanes. As a result, tests or other codes cannot find the corresponding functions. In this case, I made the general implementation of v_float work, and then fixed this error.

asmorkalov · 2024-09-11T04:57:17Z

@opencv-alalek could you take a look again?

WanliZhong · 2024-09-11T05:04:32Z

This PR includes #26109. For this PR，you can only focus on intrin_math.hpp file.

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

asmorkalov · 2024-09-13T07:58:11Z

@WanliZhong I merged related PR. Please rebase and fix conflicts.

WanliZhong · 2024-09-13T08:21:13Z

OK, I will fix it later

This reverts commit 86faf99.

WanliZhong · 2024-09-14T11:00:37Z

@vpisarev @asmorkalov I still don't know how to use the template to rewrite the code. The problem is vector datatype can use the template to represent, but v_setall() and v_setzero() are dependent on the specific vector width functions. So how can we generate different functions for different backends by using a template? Currently my code still use macro functions

asmorkalov

👍

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

WanliZhong · 2024-09-18T07:20:35Z

As the discussion with Vadim, I will try to create new functions v_setall, v_setzero, v_load etc. The implementation will be like v_setall(float vlaue, float32x4 /*unused*/) for 128-bit, v_setall(float, float32x8 /*unused*/) for 256-bit ... Using the unused parameters to distinguish among platforms. Then use template to rewrite the v_func.

WanliZhong · 2024-09-23T06:36:36Z

I have changed the implementation by using templates, how about this implementation?

opencv-alalek · 2024-09-27T07:28:43Z

See #15839

vpisarev · 2024-09-27T19:37:44Z

excellent job! 👍

opencv-alalek · 2024-09-20T06:33:17Z

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

+    inline _Tpvec v_setzero(_Tpvec /*unused*/)                                    \
+    { return v256_setzero_##suffix(); }                                           \
+    inline _Tpvec v_setall(_Tp v, _Tpvec /*unused*/)                              \
+    { return v256_setall_##suffix(v); }                                           \


if you copy-paste code 3+ times, when we have anti-pattern and design should be changed.

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

2. replaced 'v_setall(LaneType x, VecType dummy)' with 'v_setall_<VecType>(LaneType x)' 3. added tests for the new v_setzero_<> and v_setall_<>.

…re not used

vpisarev · 2024-10-02T18:28:53Z

Let's move forward

Add support for v_sin and v_cos (Sine and Cosine) #25892 This PR aims to implement `v_sincos(v_float16 x)`, `v_sincos(v_float32 x)` and `v_sincos(v_float64 x)`. Merged after #25891 and #26023 **NOTE:** Also, the patch changes already added `v_exp`, `v_log` and `v_erf` to pass parameters by reference instead of by value, to match API of other universal intrinsics. TODO: - [x] double and half float precision - [x] tests for them - [x] doc to explain the implementation ### 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 - [x] 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

…v#25891) * use 2 parms for now to identify the error * Revert "use 2 parms for now to identify the error" This reverts commit 86faf99. * replace += with = * add v_log ref * refactor intrin_math code * Add include guard to `intrin_math.hpp` to prevent multiple inclusions * rename VX to V; make fp64 impl in neon be optional * add v_setall, v_setzero for all backends; rewrite the intrin_math * fix error on rvv_scalable * let v_erf use v_exp_default_32f function * 1. replaced 'v_setzero(VecType dummy)' with 'v_setzero_<VecType>()' 2. replaced 'v_setall(LaneType x, VecType dummy)' with 'v_setall_<VecType>(LaneType x)' 3. added tests for the new v_setzero_<> and v_setall_<>. * gcc does not seem to like static_assert in functions even when they are not used * trying to fix compile errors in Debug mode on Linux --------- Co-authored-by: Vadim Pisarevsky <vadim.pisarevsky@gmail.com>

Add support for v_sin and v_cos (Sine and Cosine) opencv#25892 This PR aims to implement `v_sincos(v_float16 x)`, `v_sincos(v_float32 x)` and `v_sincos(v_float64 x)`. Merged after opencv#25891 and opencv#26023 **NOTE:** Also, the patch changes already added `v_exp`, `v_log` and `v_erf` to pass parameters by reference instead of by value, to match API of other universal intrinsics. TODO: - [x] double and half float precision - [x] tests for them - [x] doc to explain the implementation ### 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 - [x] 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

…v#25891) * use 2 parms for now to identify the error * Revert "use 2 parms for now to identify the error" This reverts commit 86faf99. * replace += with = * add v_log ref * refactor intrin_math code * Add include guard to `intrin_math.hpp` to prevent multiple inclusions * rename VX to V; make fp64 impl in neon be optional * add v_setall, v_setzero for all backends; rewrite the intrin_math * fix error on rvv_scalable * let v_erf use v_exp_default_32f function * 1. replaced 'v_setzero(VecType dummy)' with 'v_setzero_<VecType>()' 2. replaced 'v_setall(LaneType x, VecType dummy)' with 'v_setall_<VecType>(LaneType x)' 3. added tests for the new v_setzero_<> and v_setall_<>. * gcc does not seem to like static_assert in functions even when they are not used * trying to fix compile errors in Debug mode on Linux --------- Co-authored-by: Vadim Pisarevsky <vadim.pisarevsky@gmail.com>

Add support for v_sin and v_cos (Sine and Cosine) opencv#25892 This PR aims to implement `v_sincos(v_float16 x)`, `v_sincos(v_float32 x)` and `v_sincos(v_float64 x)`. Merged after opencv#25891 and opencv#26023 **NOTE:** Also, the patch changes already added `v_exp`, `v_log` and `v_erf` to pass parameters by reference instead of by value, to match API of other universal intrinsics. TODO: - [x] double and half float precision - [x] tests for them - [x] doc to explain the implementation ### 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 - [x] 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

WanliZhong marked this pull request as ready for review July 9, 2024 17:03

WanliZhong requested a review from asmorkalov July 9, 2024 17:03

WanliZhong added this to the 4.11.0 milestone Jul 9, 2024

WanliZhong mentioned this pull request Jul 9, 2024

Add support for v_sin and v_cos (Sine and Cosine) #25892

Merged

9 tasks

WanliZhong added bug category: core labels Jul 9, 2024

asmorkalov requested a review from opencv-alalek July 9, 2024 18:54

asmorkalov approved these changes Jul 9, 2024

View reviewed changes

asmorkalov assigned opencv-alalek Jul 10, 2024

opencv-alalek reviewed Jul 12, 2024

View reviewed changes

modules/core/include/opencv2/core/hal/intrin_cpp.hpp Show resolved Hide resolved

asmorkalov changed the title ~~Resolve Compilation Error for v_func Function in SIMD Emulator~~ WIP: Resolve Compilation Error for v_func Function in SIMD Emulator Aug 30, 2024

WanliZhong force-pushed the v_func-def-bug branch from 91428cd to 07db638 Compare September 10, 2024 17:29

WanliZhong mentioned this pull request Sep 12, 2024

Replace operators with wrapper functions on universal intrinsics backends #26109

Merged

6 tasks

asmorkalov reviewed Sep 13, 2024

View reviewed changes

modules/core/include/opencv2/core/hal/intrin_math.hpp Outdated Show resolved Hide resolved

WanliZhong added 7 commits September 14, 2024 01:19

use 2 parms for now to identify the error

27a0cdd

Revert "use 2 parms for now to identify the error"

d1b7d56

This reverts commit 86faf99.

replace += with =

065fa30

add v_log ref

5159b2b

refactor intrin_math code

960ecd2

Add include guard to intrin_math.hpp to prevent multiple inclusions

ce453e6

rename VX to V; make fp64 impl in neon be optional

139c254

WanliZhong force-pushed the v_func-def-bug branch from 258e756 to 139c254 Compare September 14, 2024 10:49

asmorkalov changed the title ~~WIP: Resolve Compilation Error for v_func Function in SIMD Emulator~~ Resolve Compilation Error for v_func Function in SIMD Emulator Sep 17, 2024

asmorkalov approved these changes Sep 17, 2024

View reviewed changes

modules/core/include/opencv2/core/hal/intrin_math.hpp Outdated Show resolved Hide resolved

WanliZhong changed the title ~~Resolve Compilation Error for v_func Function in SIMD Emulator~~ WIP: Resolve Compilation Error for v_func Function in SIMD Emulator Sep 18, 2024

WanliZhong added 3 commits September 19, 2024 19:42

add v_setall, v_setzero for all backends; rewrite the intrin_math

a71c63c

fix error on rvv_scalable

9ef06c2

let v_erf use v_exp_default_32f function

994f6f1

WanliZhong changed the title ~~WIP: Resolve Compilation Error for v_func Function in SIMD Emulator~~ Resolve Compilation Error for v_func Function in SIMD Emulator Sep 19, 2024

vpisarev self-requested a review September 27, 2024 19:37

vpisarev approved these changes Sep 27, 2024

View reviewed changes

opencv-alalek reviewed Sep 28, 2024

View reviewed changes

vpisarev added 3 commits October 2, 2024 15:24

1. replaced 'v_setzero(VecType dummy)' with 'v_setzero_<VecType>()'

b2d02c8

2. replaced 'v_setall(LaneType x, VecType dummy)' with 'v_setall_<VecType>(LaneType x)' 3. added tests for the new v_setzero_<> and v_setall_<>.

gcc does not seem to like static_assert in functions even when they a…

75262fb

…re not used

trying to fix compile errors in Debug mode on Linux

ad6b38b

vpisarev merged commit 783fe72 into opencv:4.x Oct 2, 2024

mshabunin mentioned this pull request Oct 7, 2024

RISC-V: fix build with RVV 0.7.1 #26266

Merged

vpisarev mentioned this pull request Oct 9, 2024

New CPU HAL for OpenCV 5.0 #25019

Open

asmorkalov mentioned this pull request Oct 23, 2024

5.x merge 4.x #26358

Merged

mshabunin mentioned this pull request Jan 10, 2025

build opencv-4.11.0 on Fedora rawhide fails only on PPC64LE #26749

Closed

4 tasks

Uh oh!

Conversation

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

Uh oh!

Pull Request Readiness Checklist

Uh oh!

WanliZhong commented Jul 9, 2024

Uh oh!

vpisarev commented Jul 10, 2024

Uh oh!

Uh oh!

WanliZhong commented Aug 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asmorkalov commented Sep 11, 2024

Uh oh!

WanliZhong commented Sep 11, 2024

Uh oh!

Uh oh!

asmorkalov commented Sep 13, 2024

Uh oh!

WanliZhong commented Sep 13, 2024

Uh oh!

WanliZhong commented Sep 14, 2024

Uh oh!

asmorkalov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

WanliZhong commented Sep 18, 2024

Uh oh!

WanliZhong commented Sep 23, 2024

Uh oh!

opencv-alalek commented Sep 27, 2024

Uh oh!

vpisarev commented Sep 27, 2024

Uh oh!

opencv-alalek Sep 20, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vpisarev commented Oct 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

WanliZhong commented Jul 9, 2024 •

edited

Loading

WanliZhong commented Aug 10, 2024 •

edited

Loading