Skip to content

GAPI FLUID: SIMD for SubRC kernel#21231

Merged
alalek merged 2 commits intoopencv:4.xfrom
alexgiving:atrutnev/SIMD_SubRC_fluid
Dec 17, 2021
Merged

GAPI FLUID: SIMD for SubRC kernel#21231
alalek merged 2 commits intoopencv:4.xfrom
alexgiving:atrutnev/SIMD_SubRC_fluid

Conversation

@alexgiving
Copy link
Copy Markdown
Member

@alexgiving alexgiving commented Dec 10, 2021 •
edited
Loading

Performance report

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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to 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
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
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
build_gapi_standalone:Custom=ade-0.1.1f

build_image:Custom=ubuntu-openvino-2021.4.1:20.04
build_image:Custom Win=openvino-2021.4.1
build_image:Custom Mac=openvino-2021.4.1

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
test_opencl:Custom=OFF
test_bigdata:Custom=1
test_filter:Custom=*

@alexgiving alexgiving force-pushed the atrutnev/SIMD_SubRC_fluid branch from ae2b4df to f46b0b1 Compare December 10, 2021 08:08
@dmatveev dmatveev self-assigned this Dec 13, 2021
@dmatveev dmatveev added this to the 4.5.5 milestone Dec 13, 2021
@dmatveev
Copy link
Copy Markdown
Contributor

dmatveev commented Dec 13, 2021

@anna-khakimova any review here?

anna-khakimova
anna-khakimova reviewed Dec 14, 2021
View reviewed changes
modules/gapi/src/backends/fluid/gfluidcore.cpp Outdated
return saturate<DST>(x - y, roundf);
}

// there is no difference between sub and subr (due to documentation)
Copy link
Copy Markdown
Member

@anna-khakimova anna-khakimova Dec 14, 2021

Choose a reason for hiding this comment

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

Could you please give me a link to this documents?

Copy link
Copy Markdown
Member Author

@alexgiving alexgiving Dec 14, 2021

Choose a reason for hiding this comment

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

https://docs.opencv.org/4.x/da/dd3/group__gapi__math.html#:~:text=%E2%97%86%C2%A0-,subRC(),-GMat%20cv%3A%3Agapi

Copy link
Copy Markdown
Member

@anna-khakimova anna-khakimova Dec 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I haven't seen such statement in the mentioned documentation.

Copy link
Copy Markdown
Member Author

@alexgiving alexgiving Dec 14, 2021

Choose a reason for hiding this comment

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

I mean the order of arguments. It is the same as it in sub function

Copy link
Copy Markdown
Member

@anna-khakimova anna-khakimova Dec 14, 2021

Choose a reason for hiding this comment

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

The difference is obvious. SubC: SRC - C; SubRC: C - SRC.
It's necessary either use sub() instead of subr() and then remove subr() at all, or roll back these changes and leave everything as it was.

Copy link
Copy Markdown
Member

@anna-khakimova anna-khakimova Dec 14, 2021

Choose a reason for hiding this comment

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

I would prefer the second variant.

Copy link
Copy Markdown
Member Author

@alexgiving alexgiving Dec 15, 2021

Choose a reason for hiding this comment

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

Applied

@anna-khakimova
Copy link
Copy Markdown
Member

anna-khakimova commented Dec 14, 2021

@alexgiving Could you please resolve the conflicts?

@anna-khakimova
Copy link
Copy Markdown
Member

anna-khakimova commented Dec 14, 2021

@alexgiving could you please change the performance report. It's necessary to leave 2 numbers after the decimal point.

@alexgiving alexgiving force-pushed the atrutnev/SIMD_SubRC_fluid branch from f46b0b1 to 5b3c528 Compare December 15, 2021 14:35
anna-khakimova
anna-khakimova approved these changes Dec 17, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@anna-khakimova anna-khakimova 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. Thank you!

dmatveev
dmatveev approved these changes Dec 17, 2021
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.

👍 LGTM!

@alexgiving
Copy link
Copy Markdown
Member Author

alexgiving commented Dec 17, 2021

@alalek could you merge pr please?

@alalek alalek merged commit 04b2752 into opencv:4.x Dec 17, 2021
@alalek alalek mentioned this pull request Dec 30, 2021
Merged
@alalek alalek mentioned this pull request Feb 22, 2022
Merged
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
Merge pull request opencv#21231 from alexgiving:atrutnev/SIMD_SubRC_f…
1cfc89b 
…luid

GAPI FLUID: SIMD for SubRC kernel

* SIMD for SubRC

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

Reviewers

@dmatveev dmatveev dmatveev approved these changes

@anna-khakimova anna-khakimova anna-khakimova approved these changes

Assignees

@dmatveev dmatveev

Labels

category: g-api / gapi optimization

Projects

None yet

Milestone

4.5.5

Development

Successfully merging this pull request may close these issues.

4 participants

@alexgiving @dmatveev @anna-khakimova @alalek