ppc: support 32bit AIX and OSX #211
Conversation
carenas
force-pushed
the
aix32
branch
2 times, most recently
from
8bd033e to
e26ec11
Compare
Since c0270dc (Support floating point arguments by more ABIs., 2017-10-21) tests had been failing in 32bit AIX, because the ABI it uses is slightly different than the one implemented. Reuse the 64bit implementation for 32bit AIX, but add the ability to skip two 32bit general registers for each 64bit floating point argument. To support a fourth argument when all three previous ones are doubles r9 had to be make accessible as a scratch register, so add r11 to replace it as TMP_REG1, with the caveat it might be clobbered by inter module calls.
|
@carenas While I am not aware if the issue affects macOS ppc, I am interested to test that. It is enough to build this and run the test suite to find out if the bug is relevant? |
|
@barracuda156: not sure I understand what you mean by "issue". This PR adds support for SLJIT to work in OSX when the CPU is a 32-bit PowerPC, without it, SLJIT will probably crash if anything that touches a FPU register is used. PCRE2 doesn't "normally" use FPU registers, but that is changing with the addition of SIMD (which most of the times reuses those registers for vector operations). That being said, I don't think the current version of PCRE2 would be affected, but that can change with any future version unless this is merged (needs rebasing first). If you have access to systems running those CPUs/OS building and runniing the tests should be enough to validate support was added correctly; a version of PCRE2 that includes this version of JIT which is widely used would be the next obvious step to confirm that the controversial ABI change (using r11 for SLJIT) was also safe for those OS. |
|
This is without your patch, from eb8ef1e commit: So yeah, failures are there. I will try your branch now (the patch does not apply cleanly onto the master). |
|
@carenas From your branch ed98de1 But from where exactly it fails, looks like OS detecting does not work, since apparently nothing sets If I change that to a standard |
|
@carenas Looks like |
|
@carenas Is this intended? The logic of macros looks a bit unusual here. |
|
Basically trying to avoid applying to Darwin. Originally it was assumed that the AIX support would overlap with Darwin/PowerPC, turned out to be not so. |
|
You can follow the development in PR #179 that lead up to this. |
|
@sevan Thank you for the reference. Is there some branch to test for ppc which has all needed patches present? I can test on 10.6 ppc and probably 10.5 ppc64 (provided it can build with cmake-bootstrap; my toolchain on 10.5 is kinda defunct, since I nuked libgcc and don't get time to rebuild everything, but Xcode compilers work, of course). |
Based on my comment in the other PR, at the time I tested with the patches from PR 211 & 214. |
|
@sevan Test results above #211 (comment) are from the latest commit from the master (OS detection and (I was just hoping to avoid rebasing patches myself, the one from this PR does not apply onto the current master.) |
|
P. S. Well, there is one thing which breaks the build: #283 |
|
I guess it is broken due to changes since October 2023, as it was passing all tests according to #179 (comment). Bisect time? |
Fixes a problem in 32bit AIX, where functions using doubles will fail to map correctly their function parameters.
Tested in AIX, Linux and OSX (thanks to @sevan)
Using
r11for TMP_REG1 might be controversial, but seem safe in practice, with the caveat that it wouldn't be preserved around inter module calls.There is also the possibility this change might be needed for other systems (ex: BSD), althought NetBSD/macppc and and older OpenBSD 6.6 seems to work (at least in 32bit) without it when tested on an QEMU emulated "mac".