Convert ImgWarp from SSE SIMD to HAL - 2.8x faster on Power (VSX) and…#15358
Merged
alalek merged 5 commits intoopencv:3.4from Aug 31, 2019
ChipKerchner:imgwarpToHal
Merged
Convert ImgWarp from SSE SIMD to HAL - 2.8x faster on Power (VSX) and…#15358alalek merged 5 commits intoopencv:3.4from ChipKerchner:imgwarpToHal
alalek merged 5 commits intoopencv:3.4from
ChipKerchner:imgwarpToHal
Conversation
added 2 commits
August 20, 2019 13:47
Member
|
@terfendail Could you please check performance on x86 (SSE2, SSE4.2, AVX2 CPU baselines)? |
Contributor
Performance for SSE2 baseline
[94/1970]
Performance for SSE3 baseline
[40/1970]
Performance for SSE4_2 baseline
Performance for AVX2 baseline
|
terfendail
reviewed
Aug 28, 2019
|
|
||
| #if CV_TRY_SSE4_1 | ||
| Ptr<opt_SSE4_1::WarpPerspectiveLine_SSE4> pwarp_impl_sse4; | ||
| if(CV_CPU_HAS_SUPPORT_SSE4_1) |
Contributor
There was a problem hiding this comment.
This part is necessary to dynamically call to specific implementation in case SSE4_1-instructions are available.
It looks like existing performance for SSE4 is better than UI-based performance for SSE2 for linear interpolation(at least for some cases) so IMO it make sense to retain dynamic dispatching of opt_SSE4_1::WarpPerspectiveLine_SSE4 here.
@alalek What's your opinion on this?
Member
There was a problem hiding this comment.
Lets keep SSE4_1 optimization here.
What is about performance difference with AVX2 baseline?
Contributor
There was a problem hiding this comment.
AVX2 baseline is better than SSE4_2 baseline both before and after UI implementation. AVX2 baseline also provide better performance improvement. However AVX2 baseline before UI implementation provide worse performance than SSE4_2 after. So IMO it make sense to implement UI-related dynamic dispatching for the module.
Contributor
Author
There was a problem hiding this comment.
I changed it from class function and dispatching to static functions. Hopefully this is what you are suggesting.
Do you want me to re-add in the line(s) ?
if(CV_CPU_HAS_SUPPORT_SSE4_1)
Contributor
There was a problem hiding this comment.
Yes. Checking for CV_CPU_HAS_SUPPORT_SSE4_1 allows to decide on SSE4 availability at runtime and dispatch execution accordingly. Preprocessor check for CV_TRY_SSE4_1 is performed to decide whether dynamic dispatching of the code is possible and necessary(AFAIK this feature is implemented for x86/64 only and could be completely disabled by the user for some reason)
Contributor
There was a problem hiding this comment.
I think it would be better to retain SSE4_1 implementation the way it was to make this PR cleaner
Contributor
Author
There was a problem hiding this comment.
Re-add dynamic runtime and dispatch execution.
terfendail
reviewed
Aug 28, 2019
modules/imgproc/src/imgwarp.cpp
Outdated
| if (pwarp_impl_sse4) | ||
| pwarp_impl_sse4->processNN(M, xy, X0, Y0, W0, bw); | ||
| #if CV_SIMD128_64F | ||
| if (pwarp_impl_CV_SIMD) |
Contributor
There was a problem hiding this comment.
I think this check is redundant, because the pointer couldn't be empty. I also think that creation of the class is redundant too, because it stores no additional information. IMO it would be better to just call to static functions processNN and process.
added 2 commits
August 28, 2019 09:33
terfendail
reviewed
Aug 30, 2019
modules/imgproc/src/imgwarp.hpp
Outdated
| } | ||
|
|
||
| #if CV_TRY_SSE4_1 | ||
| void WarpPerspectiveLine_ProcessNN_SSE41(const double *M, short* xy, double X0, double Y0, double W0, int bw); |
Contributor
There was a problem hiding this comment.
I think it would be better to retain SSE4_1 optimizations inside opt_SSE4_1 namespace to avoid possible name conflicts
Merged
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Convert ImgWarp from SSE SIMD to HAL - 2.8x faster on Power (VSX) and 15% speedup on x86.