Skip to content

GAPI Fluid: SIMD for ConvertTo.#21777

Merged
alalek merged 2 commits intoopencv:4.xfrom
anna-khakimova:ak/convertto_simd
Mar 29, 2022
Merged

GAPI Fluid: SIMD for ConvertTo.#21777
alalek merged 2 commits intoopencv:4.xfrom
anna-khakimova:ak/convertto_simd

Conversation

@anna-khakimova
Copy link
Copy Markdown
Member

@anna-khakimova anna-khakimova commented Mar 25, 2022

SIMD for ConvertTo

Performance:
FluidConvertToSIMD.xlsx

force_builders=Linux AVX2,Custom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
Xbuild_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

build_image:Custom=centos:7
buildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

Xbuild_image:Custom=ubuntu-openvino-2021.3.0:20.04
build_image:Custom Win=openvino-2021.4.1
build_image:Custom Mac=openvino-2021.2.0

buildworker:Custom Win=windows-3

test_modules:Custom=gapi,python2,python3,java
test_modules:Custom Win=gapi,python2,python3,java
test_modules:Custom Mac=gapi,python2,python3,java

buildworker:Custom=linux-1
# disabled due high memory usage: test_opencl:Custom=ON
Xtest_opencl:Custom=OFF
Xtest_bigdata:Custom=1
Xtest_filter:Custom=*

CPU_BASELINE:Custom Win=AVX512_SKX
CPU_BASELINE:Custom=SSE4_2

@anna-khakimova anna-khakimova force-pushed the ak/convertto_simd branch 2 times, most recently from 9263c1a to 1d900f5 Compare March 25, 2022 08:42
CV_CPU_DISPATCH_MODES_ALL); \
}

//CONVERTTO_NOCOEF_SIMD(uchar, uchar)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it commented?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

template<typename SRC>
CV_ALWAYS_INLINE
typename std::enable_if<std::is_same<SRC, short>::value ||
std::is_same<SRC, ushort>::value, void>::type
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it takes a lot of time on review to expand this conditions in mind. Also i see that these SFINAE repeat again and again

I suggest to introduce MACRO with such condition incapsulation in the way, eventually:

typename std::enable_if<SHORT_OR_USHORT>::type 
convertto_simd_nocoeff_impl(...)
<and other functions with similar SFINAE> 
...
#define SHORT_OR_USHORT std::is_same<SRC, short>::value ||  std::is_same<SRC, ushort>::value

etc

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied.

Copy link
Copy Markdown
Contributor

@sivanov-work sivanov-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Can we apply MACRO in SFINAE approach for other operation dispatchering functions in next PRs?

@anna-khakimova
Copy link
Copy Markdown
Member Author

LGTM Can we apply MACRO in SFINAE approach for other operation dispatchering functions in next PRs?

There is sense to do it in the separate PR in the bounds of a refactoring task.

Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@anna-khakimova
Copy link
Copy Markdown
Member Author

@alalek Could you please take a look this PR and merge if it looks good for you?

@alalek alalek merged commit be38d4e into opencv:4.x Mar 29, 2022
@opencv-pushbot opencv-pushbot mentioned this pull request Apr 23, 2022
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
GAPI Fluid: SIMD for ConvertTo.

* GAPI Fluid: SIMD for convertto.

* Applied comments
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.

4 participants