| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
GAPI FLUID: SIMD for SubRC kernel #21231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
alexgiving
force-pushed
the
atrutnev/SIMD_SubRC_fluid
branch
from
ae2b4df to
f46b0b1
Compare
|
@anna-khakimova any review here? |
| @@ -72,10 +72,11 @@ static inline DST sub(SRC1 x, SRC2 y) | |||
| return saturate<DST>(x - y, roundf); | |||
| } | |||
|
|
|||
| // there is no difference between sub and subr (due to documentation) | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied
|
@alexgiving Could you please resolve the conflicts? |
|
@alexgiving could you please change the performance report. It's necessary to leave 2 numbers after the decimal point. |
alexgiving
force-pushed
the
atrutnev/SIMD_SubRC_fluid
branch
from
f46b0b1 to
5b3c528
Compare
There was a problem hiding this 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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
đź‘Ť LGTM!
|
@alalek could you merge pr please? |
…luid GAPI FLUID: SIMD for SubRC kernel * SIMD for SubRC * Reverse subrc
Performance report
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.