Enable unaligned memory access on big endian PowerPC #1690
Conversation
ccawley2011
force-pushed
the
unaligned-big-endian
branch
from
ad3231f
to
2a72924
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1690 +/- ##
============================================
- Coverage 83.01% 32.59% -50.43%
============================================
Files 135 66 -69
Lines 10365 5513 -4852
Branches 2815 1247 -1568
============================================
- Hits 8605 1797 -6808
- Misses 1067 3447 +2380
+ Partials 693 269 -424 ☔ View full report in Codecov by Sentry. |
|
I've tested unaligned access "emulation" in qemu and it doesn't cause exception like on real hardware, but it will cause data corruption which our tests should detect. As far as I know, unaligned memory access in some PowerPC processors is handled transparently in kernel (similar to handling in SBI "firmware" for RISC-V), so user applications and libraries don't even see the exception except as performance loss. |
In that case, should I limit these changes to 64-bit PowerPC for now, since unaligned access is already enabled there in little endian builds? |
I don't have 64-bit PowerPC hardware to test, so I can't say what is the best solution... We should benchmark if there is any speed gain in enabling unaligned access, as usually switching between user and kernel code has small performance penalty. |
|
I have a g5, I can try it out tomorrow. |
|
Code doesn't currently compile - it doesn't seem to like the ALIGNED macro: I think the solution is to add zbuild.h to the inflate and deflate headers, though I'm a little bit confused why this struct isn't throwing off other architectures. Nope, that doesn't fix it either. What's even weirder is this macro is used explicitly in the VMX code. Your diff from the develop branch is just messing with the unaligned_ok macro, what on earth is causing this? |
|
Is CMake just suddenly failing the test for whether or not that compiler builtin exists and is now not defining this macro? That has to be it, somehow. Wow, yeah, somehow that's it. Adding -DHAVE_ATTRIBUTE_ALIGNED to the cflags manually allows it to compile. That's really weird, the cmakelist is somehow bypassing this test. |
|
Anyhow, assuming that cmake issue gets fixed: |
You can remove the line starting with |
|
|
Something is up with this failing this cmake test, I'm not really sure why it's suddenly failing this test. |
You should check all the relevant lines in |
See also PR #1469. This will need testing on actual hardware, but it should work fine.