Implement support for ppc64 ELFv2 #1
Conversation
src/osfiber_asm_ppc64.S
Outdated
| std 5, MARL_REG_CCR(3) | ||
|
|
||
| // Store non-volatile floating point registers | ||
| stfd 14, MARL_REG_FPRS+0x10(3) |
There was a problem hiding this comment.
It is not immediately clear to me why the first offset here is 0x10. 0x98 is 19 x 8, and fprs is an array of 18 elements, so it looks to me like we're overrunning the fprs array by two elements. Please excuse me if I'm just being stupid.
In the other platforms I've purposefully tried to dodge pointer arithmetic for an explicit list of addresses (unrolling if required) and verifying that each offset is as expected with the static_asserts.
There was a problem hiding this comment.
Thank you for catching that, you are absolutely correct that this is overflowing the fprs array. I chose to handle fprs and vmx as an array since the non-volatile registers in these classes are contiguous, though I now see how that can lead to issues like this.
For now I'll correct the offsets, but if you would like me to I can unroll the fprs array. The vmx array can also be unrolled, though since it doesn't have displacement-based load/store instructions, I had to stagger load/store and addi instructions which seems less error prone than manually calculating the offsets like with fprs.
There was a problem hiding this comment.
I would prefer them unrolled (less cognitive brain power required, and I feel this is more robust for changes on these files), but if you feel strongly I won't block this change just for this.
There was a problem hiding this comment.
I'll go ahead and unroll the fprs array for sure then. If it's okay with you though, I still think that leaving the vmx array as-is is the cleanest way to handle it, especially since vmx registers are 128-bit and representing the registers individually in C would require adding a padding u64 for each register, or creating a separate struct with two u64s to represent a VMX register.
There was a problem hiding this comment.
I still think that leaving the vmx array as-is is the cleanest way to handle it
Yeah, that's fine. I saw they required a bit more special casing.
SGTM, thanks!
shawnanastasio
force-pushed
the
ppc64-support
branch
from
da63cc4
to
c948b69
Compare
This commit adds Yarn support for powerpc64 machines using the ELFv2 ABI. The changes have been tested on a POWER9 machine running in Little Endian mode and all unit tests pass.
shawnanastasio
force-pushed
the
ppc64-support
branch
from
c948b69
to
d40d56e
Compare
|
Just pushed an update with the fprs array unrolled and the .c and .h files run through clang-format |
This commit adds Yarn support for powerpc64 machines
using the ELFv2 ABI. The changes have been tested on
a POWER9 machine running in Little Endian mode and all
unit tests pass.