Skip to content

GAPI Fluid: SIMD for DivRC kernel.#21530

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

GAPI Fluid: SIMD for DivRC kernel.#21530
alalek merged 4 commits intoopencv:4.xfrom
anna-khakimova:ak/simd_divrc

Conversation

@anna-khakimova
Copy link
Copy Markdown
Member

@anna-khakimova anna-khakimova commented Jan 28, 2022 •
edited
Loading

Add SIMD for GAPI Fluid DivRC kernel.

Performance report:
FluidDivRCSIMD.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 marked this pull request as ready for review February 17, 2022 19:25
@anna-khakimova anna-khakimova force-pushed the ak/simd_divrc branch 2 times, most recently from 9e87de6 to 50443db Compare February 18, 2022 11:49
@anna-khakimova anna-khakimova added this to the 4.6.0 milestone Feb 18, 2022
sivanov-work
sivanov-work reviewed Feb 22, 2022
View reviewed changes
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.

@anna-khakimova
I checked out 4 files from 6 and have understood that it takes too much time then i supposed.
If this is urgent then I suggest you to find another reviewer: I'm very sorry but I have much load on this week. Furthermore we need to finish another one PR for DX11/OpenCL which had started before DivRC and got several unresolved questions.

Sorry, i cannot proceed with this PR on this week

modules/gapi/src/backends/fluid/gfluidcore.cpp
template<typename DST, typename SRC>
static void run_arithm_rs(Buffer &dst, const View &src, const float scalar[4], Arithm arithm,
float scale=1)
CV_ALWAYS_INLINE void run_arithm_rs(Buffer &dst, const View &src, const float scalar[],
Copy link
Copy Markdown
Contributor

@sivanov-work sivanov-work Feb 18, 2022

Choose a reason for hiding this comment

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

is scalar size changed from 4?

Copy link
Copy Markdown
Member Author

@anna-khakimova anna-khakimova Mar 1, 2022

Choose a reason for hiding this comment

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

The scalar array is a pointer to scratch buffer that was filled in a special way.

modules/gapi/src/backends/fluid/gfluidcore.cpp Outdated
GMatDesc bufdesc = { CV_32F, 1, bufsize };
Buffer buffer(bufdesc);
scratch = std::move(buffer);
scratch = std::move(Buffer(GMatDesc(CV_32F, 1, cv::Size(buflen, 1))));
Copy link
Copy Markdown
Contributor

@sivanov-work sivanov-work Feb 18, 2022

Choose a reason for hiding this comment

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

suggest to remove std::mode or swap it if available

modules/gapi/src/backends/fluid/gfluidcore_func.simd.hpp
template<typename scalar_t>
using univ_zero_vec_type_of_t = typename univ_zero_vec_type_of<scalar_t>::type;

template<> struct univ_zero_vec_type_of<uchar> { using type = v_uint8; };
Copy link
Copy Markdown
Contributor

@sivanov-work sivanov-work Feb 22, 2022

Choose a reason for hiding this comment

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

what does it mean - univ?

modules/gapi/src/backends/fluid/gfluidcore_func.simd.hpp
template<typename scalar_t>
using zero_vec_type_of_t = typename zero_vec_type_of<scalar_t>::type;

template<> struct zero_vec_type_of<uchar> { using type = v_int16; };
Copy link
Copy Markdown
Contributor

@sivanov-work sivanov-work Feb 22, 2022

Choose a reason for hiding this comment

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

int16 for uchar? is it not mistake?

Copy link
Copy Markdown
Member Author

@anna-khakimova anna-khakimova Mar 1, 2022

Choose a reason for hiding this comment

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

It's not mistake it's implementation specifics.

modules/gapi/src/backends/fluid/gfluidcore_func.simd.hpp
} \
} \
default: \
GAPI_Assert(chan <= 4); \
Copy link
Copy Markdown
Contributor

@sivanov-work sivanov-work Feb 22, 2022

Choose a reason for hiding this comment

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

i think there may be warning for walking through switch-cases

Copy link
Copy Markdown
Member Author

@anna-khakimova anna-khakimova Mar 1, 2022

Choose a reason for hiding this comment

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

There is no warnings.

alexgiving
alexgiving approved these changes Feb 24, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@alexgiving alexgiving left a comment

Choose a reason for hiding this comment

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

👍

rgarnov
rgarnov reviewed Feb 28, 2022
View reviewed changes
rgarnov
rgarnov approved these changes Mar 1, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@rgarnov rgarnov left a comment

Choose a reason for hiding this comment

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

Looks good

alalek
alalek reviewed Mar 1, 2022
View reviewed changes
modules/gapi/perf/common/gapi_core_perf_tests_inl.hpp

// FIXIT Unstable input data for divide
initMatsRandU(type, sz, dtype, false);
//This condition need as workaround the bug in the OpenCV.
Copy link
Copy Markdown
Member

@alalek alalek Mar 1, 2022

Choose a reason for hiding this comment

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

BTW, It makes sense to add reference to the mentioned bug. #21044

Copy link
Copy Markdown
Member Author

@anna-khakimova anna-khakimova Mar 2, 2022

Choose a reason for hiding this comment

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

@alalek I propose to apply this comment in the next PR in order not to restart the CI checks once again.

dmatveev
dmatveev approved these changes Mar 2, 2022
View reviewed changes
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.

Approved but please address Alexander's comment

@alalek alalek merged commit 9c7adb7 into opencv:4.x Mar 2, 2022
@opencv-pushbot opencv-pushbot mentioned this pull request Apr 23, 2022
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
Merge pull request opencv#21530 from anna-khakimova:ak/simd_divrc
6caf3f0 
* GAPI Fluid: SIMD for DivRC kernel.

* Fluid: Div kernel's SIMD refactoring

* SIMD for DivRC 3 channel case

* Applied comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalek alalek alalek left review comments

@sivanov-work sivanov-work sivanov-work left review comments

@dmatveev dmatveev dmatveev approved these changes

@rgarnov rgarnov rgarnov approved these changes

@alexgiving alexgiving alexgiving approved these changes

Assignees

@dmatveev dmatveev

Labels

category: g-api / gapi optimization

Projects

None yet

Milestone

4.6.0

Development

Successfully merging this pull request may close these issues.

6 participants

@anna-khakimova @dmatveev @alalek @rgarnov @alexgiving @sivanov-work