HAL mul8x8to16 added by savuor · Pull Request #25506 · opencv/opencv

savuor · 2024-04-29T01:48:47Z

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 another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the 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

modules/core/src/hal_replacement.hpp

asmorkalov · 2024-05-02T14:52:08Z

modules/core/src/arithm.cpp

+    int res;
+    res = cv_hal_mul8u16u(/* src1_data */ &ua, /* src1_step */ 1, /* src2_data */ &ub, /* src2_step */ 1,
+                             /* dst_data */ &uc, /* dst_step */ 1, /* width */ 1, /* height */ 1, /* scale */ 1);
+    if (res == CV_HAL_ERROR_NOT_IMPLEMENTED)


The HAL function may return not implemented status, if size is to small or data is not aligned. Static pre-check does not work.

asmorkalov · 2024-05-06T06:08:21Z

modules/core/src/arithm.cpp

+                               void* usrdata)
+{
+    double scale = *((double*)usrdata);
+    cv_hal_mul8s16s((schar*)src1, step1, (schar*)src2, step2, (short*)dst, step, width, height, scale);


cv_hal_mul8s16s and cv_hal_mul8u16u return value is not checked. They may return error or not implemented status.

Have an ugly solution for this, please take a look

asmorkalov · 2024-05-06T06:08:47Z

modules/core/src/arithm.cpp


-    arithm_op(src1, src2, dst, noArray(), dtype, getMulTab(),
-              true, &scale, std::abs(scale - 1.0) < DBL_EPSILON ? OCL_OP_MUL : OCL_OP_MUL_SCALE);
+    static bool halMul8to16available = (cv_hal_mul8u16u != hal_ni_mul8u16u) && (cv_hal_mul8s16s != hal_ni_mul8s16s);


What if HAL implements only one of them?

vpisarev · 2024-05-14T17:35:06Z

Can we replace getMulTab(bool extended) function with getMulFunc(int src1depth, int src2depth, int dstdepth)?
We can pass such a function into arithm_op. arithm_op will first try to get direct function for the original src1, src2 and dst depths and if it fails, it will make another try with normalized types (in the latter case the explicit conversion 8u->32f is done for each of the inputs in the case of 8u x 8u -> 32f op).

asmorkalov · 2024-05-16T06:02:47Z

modules/core/src/arithm.cpp

        Mat src1 = psrc1->getMat(), src2 = psrc2->getMat(), dst = _dst.getMat();
        Size sz = getContinuousSize2D(src1, src2, dst, src1.channels());
-        tab[depth1](src1.ptr(), src1.step, src2.ptr(), src2.step, dst.ptr(), dst.step, sz.width, sz.height, usrdata);
+        if (extendedFunc(src1.ptr(), src1.step, src2.ptr(), src2.step,


if (!extendedFunc || extendedFunc() != 0)

asmorkalov · 2024-05-16T07:38:28Z

modules/core/src/arithm.cpp

+{
+    double scale = *((double*)usrdata);
+    int res = cv_hal_mul8u16u(src1, step1, src2, step2, (ushort *)dst, step, width, height, scale);
+    if (res == 0 || res == CV_HAL_ERROR_NOT_IMPLEMENTED)


Use constant instead of 0.

asmorkalov

👍

HAL mul8x8to16 added opencv#25506 Fixes opencv#25034 See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake

HAL mul8x8to16 added opencv#25506 Fixes opencv#25034 ### Pull Request Readiness Checklist See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request - [x] I agree to contribute to the project under Apache 2 License. - [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV - [x] The PR is proposed to the proper branch - [x] There is a reference to the original bug report and related work - [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable Patch to opencv_extra has the same branch name. - [x] The feature is well documented and sample code can be built with the project CMake

asmorkalov added feature category: core labels Apr 29, 2024

asmorkalov added this to the 4.10.0 milestone Apr 29, 2024

savuor marked this pull request as ready for review April 29, 2024 21:44

vpisarev reviewed May 2, 2024

View reviewed changes

modules/core/src/hal_replacement.hpp Outdated Show resolved Hide resolved

asmorkalov assigned vpisarev May 2, 2024

asmorkalov requested changes May 2, 2024

View reviewed changes

asmorkalov reviewed May 6, 2024

View reviewed changes

savuor mentioned this pull request May 12, 2024

HAL for sub8x32f added #25574

Merged

6 tasks

savuor force-pushed the rv/hal_mul16 branch from 9828e4b to dd65b52 Compare May 15, 2024 23:56

asmorkalov reviewed May 16, 2024

View reviewed changes

asmorkalov approved these changes May 16, 2024

View reviewed changes

Rostislav Vasilikhin and others added 13 commits May 17, 2024 16:01

HAL mul8x8to16 added

d02e8ba

8s added

d9a108b

Code review fixes.

4c263a4

trying to fix HAL search

64ad518

ugly fix for HAL

d26f4f3

separate 8u and 16u

3fd4a01

extendedFunc implemented

43be67b

compile fix

703fb46

minor

7db6d65

fixes

59e41ee

fixed scalar mode

17ae223

minor

104ff32

default dtype fix

8545784

savuor force-pushed the rv/hal_mul16 branch from be88741 to 8545784 Compare May 17, 2024 14:11

asmorkalov approved these changes May 20, 2024

View reviewed changes

asmorkalov merged commit 69af621 into opencv:4.x May 20, 2024

savuor deleted the rv/hal_mul16 branch May 20, 2024 10:19

mshabunin mentioned this pull request Jun 14, 2024

Merge 4.x -> 5.x #25745

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HAL mul8x8to16 added#25506

HAL mul8x8to16 added#25506
asmorkalov merged 13 commits intoopencv:4.xfrom
savuor:rv/hal_mul16

savuor commented Apr 29, 2024 •

edited

Loading

Uh oh!

Uh oh!

asmorkalov May 2, 2024

Uh oh!

asmorkalov May 6, 2024

Uh oh!

savuor May 7, 2024

Uh oh!

asmorkalov May 6, 2024

Uh oh!

savuor May 7, 2024

Uh oh!

vpisarev commented May 14, 2024 •

edited

Loading

Uh oh!

asmorkalov May 16, 2024

Uh oh!

asmorkalov May 16, 2024

Uh oh!

asmorkalov left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

savuor commented Apr 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Readiness Checklist

Uh oh!

Uh oh!

asmorkalov May 2, 2024

Choose a reason for hiding this comment

Uh oh!

asmorkalov May 6, 2024

Choose a reason for hiding this comment

Uh oh!

savuor May 7, 2024

Choose a reason for hiding this comment

Uh oh!

asmorkalov May 6, 2024

Choose a reason for hiding this comment

Uh oh!

savuor May 7, 2024

Choose a reason for hiding this comment

Uh oh!

vpisarev commented May 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asmorkalov May 16, 2024

Choose a reason for hiding this comment

Uh oh!

asmorkalov May 16, 2024

Choose a reason for hiding this comment

Uh oh!

asmorkalov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

savuor commented Apr 29, 2024 •

edited

Loading

vpisarev commented May 14, 2024 •

edited

Loading