-
-
Notifications
You must be signed in to change notification settings - Fork 308
Fixed false positive HAVE_ARMV6_INTRIN value on old ARM platforms #1774
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1774 +/- ##
===========================================
- Coverage 83.33% 83.02% -0.31%
===========================================
Files 132 135 +3
Lines 10018 10311 +293
Branches 2687 2786 +99
===========================================
+ Hits 8348 8561 +213
- Misses 1009 1053 +44
- Partials 661 697 +36 ☔ View full report in Codecov by Sentry. |
Dead2
left a comment
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
|
Does this need to be fixed in the configure script too? |
|
It probably should be changed for all the tests in both CMake and Configure. Although the compiler should balk at the code, it is known that is will not do so if for example LTO is enabled, because at linktime the compiler can discard the code as unused and never detect/ignore that it contained code it would not be able to/allowed to compile (given the current instruction set limitations). We disable LTO during these tests in part for this reason, but there may well be other compilers/settings/conditions that will cause the same issue, now or in the future. |
f1ce625 to
cfba820
Compare
|
Added to the configure script too. Looks like it's not the only place in configure script where the test function is not called in main. |
|
My question is, why is your compiler compiling for that test okay if the instruction does not exist? |
Update zlib-ng to 2.2.1 #26113 Release: https://github.com/zlib-ng/zlib-ng/releases/tag/2.2.1 ARM diagnostics patch: zlib-ng/zlib-ng#1774 ### 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 - [ ] 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
Update zlib-ng to 2.2.1 opencv#26113 Release: https://github.com/zlib-ng/zlib-ng/releases/tag/2.2.1 ARM diagnostics patch: zlib-ng/zlib-ng#1774 ### 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 - [ ] 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
The check returns 1, even if
__uqsub16is not provided by compiler. The issue was reproduced with jetson TK1 (armv7 neon), GCC 5.4.0 and-DWITH_RUNTIME_CPU_DETECTION=ON