[arm] Disable ACLE and UNALIGNED64_OK on armv7 and earlier when using cmake. #1030
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
mtl1979
commented
Jul 21, 2021
mtl1979
commented
- ACLE was introduced in armv8
- Support for unaligned 64-bit reads was added in armv8
- Add status text in cmake output if unaligned 64-bit reads are enabled or disabled.
Codecov Report
@@ Coverage Diff @@
## develop #1030 +/- ##
===========================================
- Coverage 77.91% 77.86% -0.05%
===========================================
Files 81 81
Lines 8389 8380 -9
Branches 1371 1364 -7
===========================================
- Hits 6536 6525 -11
- Misses 1320 1325 +5
+ Partials 533 530 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Configure log |
|
@hlewin It clearly shows WITH_NEON is disabled... the configuration step doesn't show all the expected output so |
|
Observations: adler32.c only checks for __ARM_NEON, which IS set by the compiler The build script (with this patch) deactivates NEON alltogether and hence does not build |
|
@hlewin my machine doesn't even have those lines. |
|
@mtl1979 I am sorry. I was on the wrong branch shame |
|
@hlewin People make mistakes, but in the end we're here to help people even if the error is between computer and the chair. |
|
However, I still get the bus error... |
|
armv7 should support unaligned reads that are 16 or 32 bits (2 or 4 bytes)... But your processor might be exception... |
|
I am really not familiar with arms. There are too much variations. |
|
@hlewin Me neither... I need to figure out when |
|
I have tried with both shorts and ints and do not get a bus error. |
…ier. * armv7 has partial support for unaligned reads, but compiler might use instructions that do not support unaligned accesses
|
@hlewin When you allocate from stack, it's always aligned. When you offset it, it knows that the access is unaligned. The problem is when the pointer is passed as function parameter. compiler might not know where the variable was allocated from, heap or stack. |
|
Running this with |
|
Running this with a debug-build, though, gives a working executable. Maybe the compiler knows more than you think.... |
|
@hlewin Debug build disables optimizations, which means compiler doesn't try to combine multiple loads and stores as single wider access... For some platforms using |
|
|
@hlewin |
|
Finally, I got the remote debugger to work properly. You are right. Seems like 2 reads were aggregated: |
|
The latest version should not define |
|
Though it likely fixes my problem, it does not seem sufficient to disallow this on armv7. There is a separate UNALIGNED64_OK flag which gets kinda useless if UNALIGNED_OK leaves the possibility for the compiler to do such stuff. This might happen on any platform. |
|
@hlewin Combining memory accesses happens on most platforms, but only few don't support unaligned accesses... Older ARM processors and big-endian PowerPC are the one that I know... |
|
Okay, this is your decision. I just wanted to point out that UNALIGNED_OK and UNALIGNED64_OK become synonymous under these circumstances. |
|
@hlewin I did read about suggestion to add |
|
There are some attributes for variables that might help. I dunno if these would really work in this case, though. |
|
@hlewin |
|
Yes - if the compiler knows that a variable is not on a properly aligned address, it will not generate an illegal load instruction. |
|
@hlewin But it would break the code on other platforms unless the attribute is wrapped inside preprocessor macro that gets expanded only on required platforms... It would make the code quite hard to maintain. |
|
The code already is full of |
|
@hlewin We did do speed test with memcpy() and it was actually quite a lot slower. |
|
That again, would be an argument for the attributes if you mind armv7 targets. For my work, setting UNALIGNED_OK and getting the slow memcpy code-path even if the 32-bit path could work, is kinda okay as nobody uses armv7 androids anymore... PS: But this is all speculation. Again, thank you for your quick support on this one. |
|
@hlewin Officially we (zlib-ng) support as old as armv2 (with all optimized code paths disabled) but that doesn't mean the generated code is optimal for anything other than armv8 or newer. |
Dead2
added a commit
that referenced
this pull request
Dec 13, 2021
- Fix hangs on macOS #1031 - Fix minideflate write buffers being overwritten #1060 - Fix build problems when building outside of source dir #1049 - Fix build problems on arm2-7 #1030 - Fixed some compile warnings #1020 #1036 #1037 #1048 - Improved posix memalign support #888 - Improvements to testing #637 #1026 #1035 #1051 #1056 #1063 #1067 - Improvements for integration into other projects #1022 #1042 - Code style fixes #637 #1040 #1050
Merged
Dead2
added a commit
that referenced
this pull request
Dec 20, 2021
- Fix hangs on macOS #1031 - Fix minideflate write buffers being overwritten #1060 - Fix deflateBound and compressBound returning too small size estimates #1071 - Fix build problems when building outside of source dir #1049 - Fix build problems on arm2-7 #1030 - Fixed some compile warnings #1020 #1036 #1037 #1048 - Improved posix memalign support #888 - Improvements to testing #637 #1026 #1032 #1035 #1051 #1056 #1063 #1067 - Improvements for integration into other projects #1022 #1042 - Code style fixes #637 #1040 #1050 #1075
Dead2
added a commit
that referenced
this pull request
Dec 20, 2021
- Fix hangs on macOS #1031 - Fix minideflate write buffers being overwritten #1060 - Fix deflateBound and compressBound returning too small size estimates #1071 - Fix build problems when building outside of source dir #1049 - Fix build problems on arm2-7 #1030 - Fixed some compile warnings #1020 #1036 #1037 #1048 - Improved posix memalign support #888 - Improvements to testing #637 #1026 #1032 #1035 #1051 #1056 #1063 #1067 - Improvements for integration into other projects #1022 #1042 - Code style fixes #637 #1040 #1050 #1075
Dead2
added a commit
that referenced
this pull request
Dec 20, 2021
- Fix hangs on macOS #1031 - Fix minideflate write buffers being overwritten #1060 - Fix deflateBound and compressBound returning too small size estimates #1071 - Fix build problems when building outside of source dir #1049 - Fix build problems on arm2-7 #1030 - Fixed some compile warnings #1020 #1036 #1037 #1048 - Improved posix memalign support #888 - Improvements to testing #637 #1026 #1032 #1035 #1051 #1056 #1063 #1067 - Improvements for integration into other projects #1022 #1042 - Code style fixes #637 #1040 #1050 #1075
Dead2
added a commit
that referenced
this pull request
Dec 20, 2021
- Fix hangs on macOS #1031 - Fix minideflate write buffers being overwritten #1060 - Fix deflateBound and compressBound returning too small size estimates #1049 #1071 - Fix incorrect function declaration warning #1080 - Fix build problems when building outside of source dir #1049 - Fix build problems on arm2-7 #1030 - Fixed some compile warnings #1020 #1036 #1037 #1048 - Improved posix memalign support #888 - Improvements to testing #637 #1026 #1032 #1035 #1049 #1051 #1056 #1063 #1067 - Improvements for integration into other projects #1022 #1042 - Code style fixes #637 #1040 #1050 #1075
Dead2
added a commit
that referenced
this pull request
Dec 20, 2021
- Fix hangs on macOS #1031 - Fix minideflate write buffers being overwritten #1060 - Fix deflateBound and compressBound returning too small size estimates #1049 #1071 - Fix incorrect function declaration warning #1080 - Fix build problems when building outside of source dir #1049 - Fix build problems on arm2-7 #1030 - Fixed some compile warnings #1020 #1036 #1037 #1048 - Improved posix memalign support #888 - Improvements to testing #637 #1026 #1032 #1035 #1049 #1051 #1056 #1063 #1067 #1079 - Improvements for integration into other projects #1022 #1042 - Code style fixes #637 #1040 #1050 #1075
Dead2
added a commit
that referenced
this pull request
Dec 24, 2021
- Fix hangs on macOS #1031 - Fix minideflate write buffers being overwritten #1060 - Fix deflateBound and compressBound returning too small size estimates #1049 #1071 - Fix incorrect function declaration warning #1080 - Fix build problems when building outside of source dir #1049 - Fix build problems on arm2-7 #1030 - Fixed some compile warnings #1020 #1036 #1037 #1048 - Improved posix memalign support #888 - Improvements to testing #637 #1026 #1032 #1035 #1049 #1051 #1056 #1063 #1067 #1079 - Improvements for integration into other projects #1022 #1042 - Code style fixes #637 #1040 #1050 #1075
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.