Rewrite Universal Intrinsic code: gapi module (fluid part). #24324
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 tasks
|
After my various tests, the unit test case failures seem to be due to specific compiler and simulator versions and not related to code modifications🤔 In the following test environment, this patch did not introduce new failure test cases. Test Environment:
And then, the following test failed both with and without this patch: |
|
@TolyaTalamanov @dmatveev please review the PR. |
mshabunin
approved these changes
Oct 5, 2023
asmorkalov
pushed a commit
that referenced
this pull request
Oct 5, 2023
Rewrite Universal Intrinsic code: float related part #24325 The goal of this series of PRs is to modify the SIMD code blocks guarded by CV_SIMD macro: rewrite them by using the new Universal Intrinsic API. The series of PRs is listed below: #23885 First patch, an example #23980 Core module #24058 ImgProc module, part 1 #24132 ImgProc module, part 2 #24166 ImgProc module, part 3 #24301 Features2d and calib3d module #24324 Gapi module This patch (hopefully) is the last one in the series. This patch mainly involves 3 parts 1. Add some modifications related to float (CV_SIMD_64F) 2. Use `#if (CV_SIMD || CV_SIMD_SCALABLE)` instead of `#if CV_SIMD || CV_SIMD_SCALABLE`, then we can get the `CV_SIMD` module that is not enabled for `CV_SIMD_SCALABLE` by looking for `if CV_SIMD` 3. Summary of `CV_SIMD` blocks that remains unmodified: Updated comments - Some blocks will cause test fail when enable for RVV, marked as `TODO: enable for CV_SIMD_SCALABLE, ....` - Some blocks can not be rewrited directly. (Not commented in the source code, just listed here) - ./modules/core/src/mathfuncs_core.simd.hpp (Vector type wrapped in class/struct) - ./modules/imgproc/src/color_lab.cpp (Array of vector type) - ./modules/imgproc/src/color_rgb.simd.hpp (Array of vector type) - ./modules/imgproc/src/sumpixels.simd.hpp (fixed length algorithm, strongly ralated with `CV_SIMD_WIDTH`) These algorithms will need to be redesigned to accommodate scalable backends. ### 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
hanliutong
added a commit
to hanliutong/opencv
that referenced
this pull request
Oct 7, 2023
Rewrite Universal Intrinsic code: float related part opencv#24325 The goal of this series of PRs is to modify the SIMD code blocks guarded by CV_SIMD macro: rewrite them by using the new Universal Intrinsic API. The series of PRs is listed below: opencv#23885 First patch, an example opencv#23980 Core module opencv#24058 ImgProc module, part 1 opencv#24132 ImgProc module, part 2 opencv#24166 ImgProc module, part 3 opencv#24301 Features2d and calib3d module opencv#24324 Gapi module This patch (hopefully) is the last one in the series. This patch mainly involves 3 parts 1. Add some modifications related to float (CV_SIMD_64F) 2. Use `#if (CV_SIMD || CV_SIMD_SCALABLE)` instead of `#if CV_SIMD || CV_SIMD_SCALABLE`, then we can get the `CV_SIMD` module that is not enabled for `CV_SIMD_SCALABLE` by looking for `if CV_SIMD` 3. Summary of `CV_SIMD` blocks that remains unmodified: Updated comments - Some blocks will cause test fail when enable for RVV, marked as `TODO: enable for CV_SIMD_SCALABLE, ....` - Some blocks can not be rewrited directly. (Not commented in the source code, just listed here) - ./modules/core/src/mathfuncs_core.simd.hpp (Vector type wrapped in class/struct) - ./modules/imgproc/src/color_lab.cpp (Array of vector type) - ./modules/imgproc/src/color_rgb.simd.hpp (Array of vector type) - ./modules/imgproc/src/sumpixels.simd.hpp (fixed length algorithm, strongly ralated with `CV_SIMD_WIDTH`) These algorithms will need to be redesigned to accommodate scalable backends. ### 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
6 tasks
|
@TolyaTalamanov @dmatveev Friendly reminder. |
|
Oh wow, this is big! If tests passed in the CI, it should be ok. :) Thanks for the contribution! |
Merged
4 tasks
thewoz
pushed a commit
to thewoz/opencv
that referenced
this pull request
Jan 4, 2024
Rewrite Universal Intrinsic code: float related part opencv#24325 The goal of this series of PRs is to modify the SIMD code blocks guarded by CV_SIMD macro: rewrite them by using the new Universal Intrinsic API. The series of PRs is listed below: opencv#23885 First patch, an example opencv#23980 Core module opencv#24058 ImgProc module, part 1 opencv#24132 ImgProc module, part 2 opencv#24166 ImgProc module, part 3 opencv#24301 Features2d and calib3d module opencv#24324 Gapi module This patch (hopefully) is the last one in the series. This patch mainly involves 3 parts 1. Add some modifications related to float (CV_SIMD_64F) 2. Use `#if (CV_SIMD || CV_SIMD_SCALABLE)` instead of `#if CV_SIMD || CV_SIMD_SCALABLE`, then we can get the `CV_SIMD` module that is not enabled for `CV_SIMD_SCALABLE` by looking for `if CV_SIMD` 3. Summary of `CV_SIMD` blocks that remains unmodified: Updated comments - Some blocks will cause test fail when enable for RVV, marked as `TODO: enable for CV_SIMD_SCALABLE, ....` - Some blocks can not be rewrited directly. (Not commented in the source code, just listed here) - ./modules/core/src/mathfuncs_core.simd.hpp (Vector type wrapped in class/struct) - ./modules/imgproc/src/color_lab.cpp (Array of vector type) - ./modules/imgproc/src/color_rgb.simd.hpp (Array of vector type) - ./modules/imgproc/src/sumpixels.simd.hpp (fixed length algorithm, strongly ralated with `CV_SIMD_WIDTH`) These algorithms will need to be redesigned to accommodate scalable backends. ### 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal of this series of PRs is to modify the SIMD code blocks guarded by CV_SIMD macro: rewrite them by using the new Universal Intrinsic API.
This is the modification to the gapi module, especially the fluid part.
All modifications to the gapi module have been completed, but this PR is marked as draft because many use cases failed in the test, and I am looking for the reason.
FAILED 76 tests on RVV QEMU, listed below:
FAILED 36 tests on RVV QEMU without this patch, listed below:
[ FAILED ] 36 tests, listed below:
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/2, where GetParam() = (8UC3, 1280x720, SAME_TYPE, 0x3fb1d0, DIV, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/3, where GetParam() = (8UC3, 1280x720, SAME_TYPE, 0x3fb1d0, DIV, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/6, where GetParam() = (8UC3, 1280x720, SAME_TYPE, 0x3fb1d0, MUL, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/7, where GetParam() = (8UC3, 1280x720, SAME_TYPE, 0x3fb1d0, MUL, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/10, where GetParam() = (8UC3, 1280x720, 8UC1, 0x3fb1d0, DIV, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/11, where GetParam() = (8UC3, 1280x720, 8UC1, 0x3fb1d0, DIV, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/14, where GetParam() = (8UC3, 1280x720, 8UC1, 0x3fb1d0, MUL, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/15, where GetParam() = (8UC3, 1280x720, 8UC1, 0x3fb1d0, MUL, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/18, where GetParam() = (8UC3, 1280x720, 32FC1, 0x3fb1d0, DIV, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/19, where GetParam() = (8UC3, 1280x720, 32FC1, 0x3fb1d0, DIV, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/22, where GetParam() = (8UC3, 1280x720, 32FC1, 0x3fb1d0, MUL, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/23, where GetParam() = (8UC3, 1280x720, 32FC1, 0x3fb1d0, MUL, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/26, where GetParam() = (8UC3, 128x128, SAME_TYPE, 0x3fb1d0, DIV, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/27, where GetParam() = (8UC3, 128x128, SAME_TYPE, 0x3fb1d0, DIV, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/30, where GetParam() = (8UC3, 128x128, SAME_TYPE, 0x3fb1d0, MUL, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/31, where GetParam() = (8UC3, 128x128, SAME_TYPE, 0x3fb1d0, MUL, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/34, where GetParam() = (8UC3, 128x128, 8UC1, 0x3fb1d0, DIV, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/35, where GetParam() = (8UC3, 128x128, 8UC1, 0x3fb1d0, DIV, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/38, where GetParam() = (8UC3, 128x128, 8UC1, 0x3fb1d0, MUL, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/39, where GetParam() = (8UC3, 128x128, 8UC1, 0x3fb1d0, MUL, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/42, where GetParam() = (8UC3, 128x128, 32FC1, 0x3fb1d0, DIV, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/43, where GetParam() = (8UC3, 128x128, 32FC1, 0x3fb1d0, DIV, true, 1, true)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/46, where GetParam() = (8UC3, 128x128, 32FC1, 0x3fb1d0, MUL, true, 1, false)
[ FAILED ] MathOpTestFluid/MathOpTest.MatricesAccuracyTest/47, where GetParam() = (8UC3, 128x128, 32FC1, 0x3fb1d0, MUL, true, 1, true)
[ FAILED ] AbsDiffCTestFluid/AbsDiffCTest.AccuracyTest/12, where GetParam() = (8UC3, 1280x720, SAME_TYPE, 0x3fb504)
[ FAILED ] AbsDiffCTestFluid/AbsDiffCTest.AccuracyTest/13, where GetParam() = (8UC3, 128x128, SAME_TYPE, 0x3fb504)
[ FAILED ] AbsDiffCTestFluid/AbsDiffCTest.AccuracyTest/14, where GetParam() = (16UC3, 1280x720, SAME_TYPE, 0x3fb504)
[ FAILED ] AbsDiffCTestFluid/AbsDiffCTest.AccuracyTest/15, where GetParam() = (16UC3, 128x128, SAME_TYPE, 0x3fb504)
[ FAILED ] AbsDiffCTestFluid/AbsDiffCTest.AccuracyTest/16, where GetParam() = (16SC3, 1280x720, SAME_TYPE, 0x3fb504)
[ FAILED ] AbsDiffCTestFluid/AbsDiffCTest.AccuracyTest/17, where GetParam() = (16SC3, 128x128, SAME_TYPE, 0x3fb504)
[ FAILED ] MathOperatorArithmeticTestFluid/MathOperatorMatScalarTest.OperatorAccuracyTest/3, where GetParam() = (8UC1, 1280x720, SAME_TYPE, 0x48d782, AbsExact(), DIV)
[ FAILED ] MathOperatorArithmeticTestFluid/MathOperatorMatScalarTest.OperatorAccuracyTest/7, where GetParam() = (8UC1, 1280x720, SAME_TYPE, 0x48d782, AbsExact(), DIVR)
[ FAILED ] MathOperatorArithmeticTestFluid/MathOperatorMatScalarTest.OperatorAccuracyTest/11, where GetParam() = (8UC1, 128x128, SAME_TYPE, 0x48d782, AbsExact(), DIV)
[ FAILED ] MathOperatorArithmeticTestFluid/MathOperatorMatScalarTest.OperatorAccuracyTest/15, where GetParam() = (8UC1, 128x128, SAME_TYPE, 0x48d782, AbsExact(), DIVR)
[ FAILED ] MathOperatorArithmeticTestFluid/MathOperatorMatScalarTest.OperatorAccuracyTest/23, where GetParam() = (16SC1, 1280x720, SAME_TYPE, 0x48d782, AbsExact(), DIVR)
[ FAILED ] MathOperatorArithmeticTestFluid/MathOperatorMatScalarTest.OperatorAccuracyTest/31, where GetParam() = (16SC1, 128x128, SAME_TYPE, 0x48d782, AbsExact(), DIVR)
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.