GAPI Fluid SIMD:Add support of new several types for the Merge3. by anna-khakimova · Pull Request #21797 · opencv/opencv

anna-khakimova · 2022-03-30T14:37:38Z

Support of the new several types was added.
Fixes for the Split/Merge and ConvertTo issues.

force_builders=Linux AVX2,Linux32,Win32,Custom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.2a
build_gapi_standalone:Win64=ade-0.1.2a
Xbuild_gapi_standalone:Mac=ade-0.1.2a
build_gapi_standalone:Linux x64 Debug=ade-0.1.2a

build_image:Custom=centos:7
buildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.2a

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

sivanov-work · 2022-04-01T05:44:18Z

modules/gapi/src/backends/fluid/gfluidutils.hpp

        return;                                            \
    }

+#define MERGE3_(T, OP, ...)                                \


Cannot find place where is it used, could you point me out?

Yes. It is used here.

discussed offline:
to move out repeated conditions into beginning of function to simplify condition calculation complexity in actual type dispatching routing

sivanov-work

LGTM

sivanov-work · 2022-04-01T09:14:14Z

modules/gapi/src/backends/fluid/gfluidutils.hpp

        return;                                            \
    }

+#define MERGE3_(T, OP, ...)                                \


sivanov-work · 2022-04-01T10:02:37Z

modules/gapi/src/backends/fluid/gfluidutils.hpp

        return;                                            \
    }

+#define MERGE3_(T, OP, ...)                                \


discussed offline:
to move out repeated conditions into beginning of function to simplify condition calculation complexity in actual type dispatching routing

dmatveev · 2022-04-01T12:29:59Z

modules/gapi/perf/cpu/gapi_core_perf_tests_cpu.cpp

 INSTANTIATE_TEST_CASE_P(Merge3PerfTestCPU, Merge3PerfTest,
    Combine(Values(AbsExact().to_compare_f()),
            Values(szSmall128, szVGA, sz720p, sz1080p),
+            Values(CV_8U),


Should other types be listed here?

..and in GPU tests, too.

dmatveev · 2022-04-01T12:32:46Z

modules/gapi/src/backends/fluid/gfluidutils.hpp

+                                                           \
+        OP<T>(__VA_ARGS__);                                \
+        return;                                            \
+    }


Macro function bodies considered harmful, is there any reasonable issues with rewriting it to templates?

dmatveev · 2022-04-01T12:33:46Z

modules/gapi/src/backends/fluid/gfluidutils.hpp

+        GAPI_DbgAssert(dst.length() == src1.length());     \
+        GAPI_DbgAssert(dst.length() == src2.length());     \
+        GAPI_DbgAssert(dst.length() == src3.length());     \


what is the reason to verify this?
normally in the framework we rely on our output buffers which are allocated per our needs.
it is not coming somewhere from the outside.

asmorkalov · 2022-10-17T06:57:53Z

@anna-khakimova @dmatveev Do you plan to finish the PR in meantime?

asmorkalov · 2023-04-11T10:31:26Z

@dmatveev @TolyaTalamanov Friendly reminder.

asmorkalov · 2023-05-05T07:43:14Z

@dmatveev @TolyaTalamanov Friendly reminder.

dmatveev · 2023-05-11T16:11:12Z

Will have a look at this one too.

dmatveev · 2023-05-23T08:11:33Z

@rgarnov can you please have a look?

dmatveev

Tested locally, these changes don't bring any regressions on my end but so far extend Fluid's merge3 with extra data types.

Can be merged IMO

Geometric mean (ms)

                                               Name of Test                                                  4x   anna     anna
                                                                                                                  simd     simd
                                                                                                                            vs
                                                                                                                            4x
                                                                                                                        (x-factor)
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 128x128, 8UC1, { gapi.kernel_package })      -   0.016     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 128x128, 16UC1, { gapi.kernel_package })     -   0.019     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 128x128, 16SC1, { gapi.kernel_package })     -   0.019     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 128x128, 32FC1, { gapi.kernel_package })     -   0.025     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 128x128, { gapi.kernel_package })          0.016   -       -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 640x480, 8UC1, { gapi.kernel_package })      -   0.099     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 640x480, 16UC1, { gapi.kernel_package })     -   0.175     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 640x480, 16SC1, { gapi.kernel_package })     -   0.178     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 640x480, 32FC1, { gapi.kernel_package })     -   0.347     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 640x480, { gapi.kernel_package })          0.103   -       -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 1280x720, 8UC1, { gapi.kernel_package })     -   0.255     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 1280x720, 16UC1, { gapi.kernel_package })    -   0.621     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 1280x720, 16SC1, { gapi.kernel_package })    -   0.613     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 1280x720, 32FC1, { gapi.kernel_package })    -   1.729     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 1280x720, { gapi.kernel_package })         0.258   -       -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 1920x1080, 8UC1, { gapi.kernel_package })    -   0.781     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 1920x1080, 16UC1, { gapi.kernel_package })   -   1.985     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 1920x1080, 16SC1, { gapi.kernel_package })   -   1.975     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 1920x1080, 32FC1, { gapi.kernel_package })   -   4.211     -
TestPerformance::Merge3PerfTestFluid/Merge3PerfTest::(compare_f, 1920x1080, { gapi.kernel_package })        0.744   -       -
TestPerformance::Merge4PerfTestFluid/Merge4PerfTest::(compare_f, 128x128, { gapi.kernel_package })          0.019 0.020    0.96
TestPerformance::Merge4PerfTestFluid/Merge4PerfTest::(compare_f, 640x480, { gapi.kernel_package })          0.127 0.129    0.98
TestPerformance::Merge4PerfTestFluid/Merge4PerfTest::(compare_f, 1280x720, { gapi.kernel_package })         0.361 0.348    1.04
TestPerformance::Merge4PerfTestFluid/Merge4PerfTest::(compare_f, 1920x1080, { gapi.kernel_package })        1.228 1.221    1.01

asmorkalov · 2023-05-29T11:12:50Z

@opencv-alalek Could you take a look on CI failures. It looks like some update is required.

dmatveev · 2023-05-29T15:45:10Z

Yep the main build runs ok:

-- ADE: Downloading v0.1.2a.zip from https://github.com/opencv/ade/archive/v0.1.2a.zip

But there's a separate ADE step where another (older) version is mentioned:

CMake Error: The source directory "/build/precommit_linux64_no_opt/build/3rdparty/ade/ade-0.1.1f" does not exist.

opencv-alalek · 2023-05-30T17:09:50Z

0.1.1f

Because it is mentioned in CI configuration section of PR's description.

dmatveev · 2023-05-31T11:27:42Z

Because it is mentioned in CI configuration section of PR's description.

Cool! Didn't know it still works given that there's Github Actions :)

…supported_types GAPI Fluid SIMD:Add support of new several types for the Merge3 - Support of the new several types was added. - Fixes for the Split/Merge and ConvertTo issues.

GAPI Fluid SIMD:Add support of new several types for the Merge3.

3601b29

anna-khakimova requested review from TolyaTalamanov and sivanov-work March 30, 2022 14:37

anna-khakimova assigned dmatveev Mar 30, 2022

anna-khakimova added optimization category: g-api / gapi labels Mar 30, 2022

anna-khakimova added this to the 4.6.0 milestone Mar 30, 2022

Fixes for the Split/Merge and ConvertTo issues.

f09e84c

sivanov-work reviewed Apr 1, 2022

View reviewed changes

anna-khakimova requested a review from sivanov-work April 1, 2022 09:01

Applied comments

9d6b84f

sivanov-work approved these changes Apr 1, 2022

View reviewed changes

dmatveev reviewed Apr 1, 2022

View reviewed changes

asmorkalov removed this from the 4.8.0 milestone May 5, 2023

dmatveev added this to the 4.8.0 milestone May 23, 2023

Merge branch '4.x' into ak/merge3_extend_supported_types

42653f5

dmatveev force-pushed the ak/merge3_extend_supported_types branch from 753c404 to 42653f5 Compare May 29, 2023 09:38

dmatveev approved these changes May 29, 2023

View reviewed changes

asmorkalov merged commit 6d3dd24 into opencv:4.x May 31, 2023

asmorkalov mentioned this pull request Jul 12, 2023

(5.x) Merge 4.x #23979

Merged

Uh oh!

Conversation

anna-khakimova commented Mar 30, 2022 • edited by asmorkalov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sivanov-work left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asmorkalov commented Oct 17, 2022

Uh oh!

asmorkalov commented Apr 11, 2023

Uh oh!

asmorkalov commented May 5, 2023

Uh oh!

dmatveev commented May 11, 2023

Uh oh!

dmatveev commented May 23, 2023

Uh oh!

dmatveev left a comment

Choose a reason for hiding this comment

Uh oh!

asmorkalov commented May 29, 2023

Uh oh!

dmatveev commented May 29, 2023

Uh oh!

opencv-alalek commented May 30, 2023

Uh oh!

dmatveev commented May 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

anna-khakimova commented Mar 30, 2022 •

edited by asmorkalov

Loading