Skip to content

Fix LoongArch Macro Definition#23920

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
loongson-zn:4.x
Jul 14, 2023
Merged

Fix LoongArch Macro Definition#23920
asmorkalov merged 1 commit intoopencv:4.xfrom
loongson-zn:4.x

Conversation

@loongson-zn
Copy link
Copy Markdown
Contributor

@loongson-zn loongson-zn commented Jul 4, 2023 •
edited
Loading

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

@asmorkalov asmorkalov added the platform: loongson Loonson CPU architecure and LASX simd label Jul 4, 2023
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Jul 4, 2023

@fengyuentau @WanliZhong Could review and test the PR?

@loongson-zn
Copy link
Copy Markdown
Contributor Author

loongson-zn commented Jul 4, 2023 

$ gcc -E -dM -</dev/null  | grep loongarch
#define __loongarch64 1
#define __loongarch_grlen 64
#define __loongarch_hard_float 1
#define __loongarch_frlen 64
#define __loongarch__ 1
#define __loongarch_double_float 1
#define _LOONGARCH_ARCH "loongarch64"

@loongson-zn
Copy link
Copy Markdown
Contributor Author

loongson-zn commented Jul 5, 2023

@CNClareChen I think Macro was written incorrectly due to carelessness in PR-22040

@CNClareChen
Copy link
Copy Markdown
Contributor

CNClareChen commented Jul 5, 2023 •
edited
Loading

@CNClareChen I think Macro was written incorrectly due to carelessness in PR-22040

Thank you for fixing this error. In previous compiler versions, there was a "__loongarch" macro. It is now more appropriate to use the "_loongarch_ " macro.

@loongson-zn
Copy link
Copy Markdown
Contributor Author

loongson-zn commented Jul 5, 2023

ty

@fengyuentau
Copy link
Copy Markdown
Member

fengyuentau commented Jul 6, 2023

Thank you for contribution! I tested your PR on our loongson server and the failed test cases are listed as follows:

calib3d:
[----------] Global test environment tear-down
[==========] 152 tests from 64 test cases ran. (134526 ms total)
[  PASSED  ] 151 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] Calib3d_StereoBM.regression # This is a legacy issue IIRC

 1 FAILED TEST
  YOU HAVE 2 DISABLED TESTS

core:
[----------] Global test environment tear-down
[ SKIPSTAT ] 14 tests skipped
[ SKIPSTAT ] TAG='mem_6gb' skip 1 tests
[ SKIPSTAT ] TAG='skip_other' skip 13 tests
[==========] 11648 tests from 262 test cases ran. (161299 ms total)
[  PASSED  ] 11633 tests.
[  FAILED  ] 15 tests, listed below:
[  FAILED  ] Core/HasNonZeroAllZeros.hasNonZeroAllZeros/21, where GetParam() = (5, 320x240)
[  FAILED  ] Core/HasNonZeroAllZeros.hasNonZeroAllZeros/22, where GetParam() = (5, 127x113)
[  FAILED  ] Core/HasNonZeroAllZeros.hasNonZeroAllZeros/23, where GetParam() = (5, 1x113)
[  FAILED  ] Core/HasNonZeroAllZeros.hasNonZeroAllZeros/25, where GetParam() = (6, 320x240)
[  FAILED  ] Core/HasNonZeroAllZeros.hasNonZeroAllZeros/26, where GetParam() = (6, 127x113)
[  FAILED  ] Core/HasNonZeroAllZeros.hasNonZeroAllZeros/27, where GetParam() = (6, 1x113)
[  FAILED  ] Core/HasNonZeroNegZeros.hasNonZeroNegZeros/1, where GetParam() = (5, 320x240)
[  FAILED  ] Core/HasNonZeroNegZeros.hasNonZeroNegZeros/2, where GetParam() = (5, 127x113)
[  FAILED  ] Core/HasNonZeroNegZeros.hasNonZeroNegZeros/3, where GetParam() = (5, 1x113)
[  FAILED  ] Core/HasNonZeroNegZeros.hasNonZeroNegZeros/5, where GetParam() = (6, 320x240)
[  FAILED  ] Core/HasNonZeroNegZeros.hasNonZeroNegZeros/6, where GetParam() = (6, 127x113)
[  FAILED  ] Core/HasNonZeroNegZeros.hasNonZeroNegZeros/7, where GetParam() = (6, 1x113)
[  FAILED  ] Core/HasNonZeroRandom.hasNonZeroRandom/17, where GetParam() = (4, 320x240)
[  FAILED  ] Core/HasNonZeroRandom.hasNonZeroRandom/18, where GetParam() = (4, 127x113)
[  FAILED  ] Core/HasNonZeroRandom.hasNonZeroRandom/19, where GetParam() = (4, 1x113)

15 FAILED TESTS
  YOU HAVE 15 DISABLED TESTS

highgui:
[----------] Global test environment tear-down
[==========] 3 tests from 1 test case ran. (1 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 3 tests, listed below:
[  FAILED  ] Highgui_GUI.regression
[  FAILED  ] Highgui_GUI.trackbar_unsafe
[  FAILED  ] Highgui_GUI.trackbar

 3 FAILED TESTS

Full log is available here: logs-10294a8.zip

Tests are done with your branch with build flags:

cmake -B build -DCMAKE_INSTALL_PREFIX=build/install opencv/
cmake --build build/ --target install -j 30

Test scripts:

#!/bin/bash

export OPENCV_TEST_DATA_PATH=/home/opencv-cn/Workspace/opencv/opencv_extra/testdata
export OPENCV_DNN_TEST_DATA_PATH=/home/opencv-cn/Workspace/opencv-ci/dnn-models

bin=(opencv_test_calib3d opencv_test_core opencv_test_dnn opencv_test_features2d opencv_test_flann opencv_test_gapi opencv_test_highgui opencv_test_imgcodecs opencv_test_imgproc opencv_test_ml opencv_test_objdetect opencv_test_photo opencv_test_stitching opencv_test_video opencv_test_videoio)

PREFIX=./build/bin
LOGS=logs

if [ ! -d "/path/to/dir" ]
then
	mkdir ${LOGS}
fi

for i in "${bin[@]}"
do
	${PREFIX}/$i | tee ${LOGS}/$i.txt
done

/cc @asmorkalov

@CNClareChen
Copy link
Copy Markdown
Contributor

CNClareChen commented Jul 7, 2023

@fengyuentau Those failed test are caused by opencl not this patch. It is recommended to use the “-DWITH_OPENCL=OFF” build flag when building opencv on the loongarch platform.

@asmorkalov asmorkalov requested a review from fengyuentau July 12, 2023 11:54
@fengyuentau
Copy link
Copy Markdown
Member

fengyuentau commented Jul 14, 2023 •
edited
Loading

It is recommended to use the “-DWITH_OPENCL=OFF” build flag when building opencv on the loongarch platform.

Rebuilt with -DWITH_OPENCL=OFF and failed tests are the same as #23920 (comment).

UPDATE:

Tested without this patch and those tests are still failed. So they are indeed not related to this patch.

@asmorkalov asmorkalov added this to the 4.9.0 milestone Jul 14, 2023
@asmorkalov asmorkalov merged commit 1f7025f into opencv:4.x Jul 14, 2023
@asmorkalov asmorkalov mentioned this pull request Jul 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fengyuentau fengyuentau fengyuentau approved these changes

Assignees

@fengyuentau fengyuentau

Labels

category: core platform: loongson Loonson CPU architecure and LASX simd

Projects

None yet

Milestone

4.9.0

Development

Successfully merging this pull request may close these issues.

4 participants

@loongson-zn @asmorkalov @CNClareChen @fengyuentau