-
-
Notifications
You must be signed in to change notification settings - Fork 56.3k
Resolve Compilation Error for v_func Function in SIMD Emulator #25891
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@asmorkalov I removed the macros |
|
@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 |
|
@opencv-alalek @vpisarev @asmorkalov Hi, sorry for the delayed fix. I finally found out why this error occurs. That's because the However, on |
91428cd to
07db638
Compare
|
@opencv-alalek could you take a look again? |
|
This PR includes #26109. For this PR,you can only focus on intrin_math.hpp file. |
|
@WanliZhong I merged related PR. Please rebase and fix conflicts. |
|
OK, I will fix it later |
This reverts commit 86faf99.
258e756 to
139c254
Compare
|
@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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
As the discussion with Vadim, I will try to create new functions |
|
I have changed the implementation by using templates, how about this implementation? |
|
See #15839 |
|
excellent job! 👍 |
| inline _Tpvec v_setzero(_Tpvec /*unused*/) \ | ||
| { return v256_setzero_##suffix(); } \ | ||
| inline _Tpvec v_setall(_Tp v, _Tpvec /*unused*/) \ | ||
| { return v256_setall_##suffix(v); } \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you copy-paste code 3+ times, when we have anti-pattern and design should be changed.
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_<>.
|
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
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
Patch to opencv_extra has the same branch name.