Enable !LJ_GC64 interpreter on PPC64. #140
Conversation
|
Rebased on top of current v2.1. Ping. |
|
Rebased again. Ping? |
This also adds a bit of plumbing to recognize ELF v2 ABI (which is NYI). Since P64 no longer implies TOC, make TOC (and TOCENV) an independent define passed to DynAsm from lj_arch.h.
The ABI mandates using CR save space in the caller's frame - we cannot use the slot in our own frame, since some function we call could overwrite it.
|
Hi, what's the status for this pull request? |
|
Note: PR has conflicting files |
|
Any hope of resurrecting this @Koriakin? I see you rebased in 2016 and never heard back on that. :/ |
|
Hi there, I am interesting to have this @Koriakin work integrated on luajit, could someone look at it ? |
|
For what it's worth, I can cook up a new rebased version, but it doesn't seem to be a good use of resources if it's just going to end up being ignored (again). So, is there any hope of getting some comments on this patchset / getting it merged? |
|
@Koriakin Sorry no one from the official team has been responding to you, and thank so much for putting in the time to do this. As a user who really wants to use this, would you mind keeping it up-to-date anyway? I for one intend to use this, and based on the other responses it sounds like there are others too. Even in its current forked state, it's definitely useful to have this. The Summit supercomputer is coming up soon, which is Power9 based, so at least in the HPC community we're going to be stuck with this architecture for a while. |
|
@elliottslaughter This issue was opened as a financial bounty. It's unfair to Marcin to request that he maintain the branch with no progress. If Summit and the HPC community wants LuaJIT for PPC64LE Linux, they should hire Marcin to maintain the fork or include a forked version in the deliverables for the Summit contract. There are a few use cases (including users of nginx), but it doesn't make sense if the LuaJIT community won't respond. I think that users of LuaJIT should switch to another language with a more responsive and productive community. |
| |.endmacro | ||
| | | ||
| |->vm_exit_handler: | ||
| |.if JIT | ||
| | addi sp, sp, -(16+32*8+32*4) | ||
| | stmw r2, 16+32*8+2*4(sp) | ||
| | addi sp, TMP0, sp, -(EXIT_OFFSET+32*8+32*PSIZE) |
There was a problem hiding this comment.
FWIW, Debian is using the changes from this PR and the powerpc build fails because this addi instruction is incorrect:
make[4]: Entering directory '/<<PKGBUILDDIR>>/src'
HOSTCC host/minilua.o
HOSTLINK host/minilua
DYNASM host/buildvm_arch.h
vm_ppc.dasc:2777: error: wrong number of parameters for `addi':
| addi sp, TMP0, sp, -(EXIT_OFFSET+32*8+32*PSIZE)
vm_ppc.dasc:*: info: 1 error in input file -- no output file generated.
make[4]: *** [Makefile:648: host/buildvm_arch.h] Error 1
There was a problem hiding this comment.
Ah, I missed that you already notified the author about this issue.
Create a patch for PPC64 support based on LuaJIT#140. https://bugzilla.redhat.com/show_bug.cgi?id=1591701 This patch has been rebased to match FPU support Author: Guy Menanteau <menantea@fr.ibm.com> Signed-Off-By: Marcin Kościelnicki <koriakin@0x04.net Signed-Off-By: Siddhesh Poyarekar <siddhesh@gotplt.org>
|
Has this been PR tested on 32-bit PowerPC (big-endian)? I see that ever since Debian has imported this patch to the Could this be fixed before the merge? As far as I know, 32-bit PowerPC is still officially supported and we're still building packages for this target in both Debian and openSUSE (I happen to be developer for both). Full log in: https://buildd.debian.org/status/fetch.php?pkg=luajit&arch=powerpc&ver=2.1.0%7Ebeta3-2&stamp=1502015864&raw=0 |
|
@siddhesh if you still need a test system, you can also get one from http://openpower.ic.unicamp.br/minicloud/ . Feel free to use me [clnperez - at - us - dot - ibm - dot -com] as a reference when requesting. This will get you a linux VM you can use, since this is an open source project. Do you have a fork for luajit2 already? |
|
@clnperez Does the minicloud have BE systems as this bug affects 32-bit big-endian? |
|
@glaubitz it does not. Sorry, I had read that but forgotten when I made that update. Thanks for the reminder. |
|
Sorry, got busy in $dayjob.
Last time I checked there weren't any ppc64 BE machines online. I checked again and indeed the one listed BE machine is offline.
Thanks, I'll send you my pubkey.
It will be significant given the current architecture. It might be easier (about a month or so) to get a port without JIT. |
The luajit2 project is responsive, so my fork of luajit2 is basically just to post PRs. The only hurdle for the luajit2 fork being a proper replacement to this project is that it has a few openresty-specific language extensions that should go behind an optional flag. I'm working on that (and getting the ports I put in my fork, including this one) and hopefully by the end of it Debian should be able to simply replace luajit with luajit2 to get more regular updates; I intend to do the same for Fedora. |
|
I've pushed the following fixes into master of my fork to fix this, thankfully it wasn't as bad as I thought it would be and the bugs mostly seem to be minor oversights: moonjit@4121ead |
|
@siddhesh Awesome, thanks a lot! |
|
@siddhesh I am also interested in luajit for PPC64(LE), do you have an insight as to how much work (if any) remains on that front? and if there is anything I can help with? I assume that you are planning to get the support for this into luajit2 ultimately? |
|
Hi @seth-priya, there is still work to be done; incidentally someone just noticed that the JIT is not enabled for ppc64le[1]. You could work on fixing up the JIT compiler to emit correct code for ppc64le. And yes, I do intend to get all of this stuff into luajit2. [1] https://www.freelists.org/post/luajit/Luajit-build-for-ppc64le |
|
Thanks @siddhesh, based on what you have seen while working on this, do you have a feel of how much effort might be involved in fixing the JIT compiler for ppc64le? |
|
One more thing, I was able to build the code both from https://github.com/siddhesh/LuaJIT and integrating the Power patch (locally) to https://github.com/openresty/luajit2, there is one test case failure when I run the test suite at https://github.com/openresty/luajit2-test-suite (on both branches), as below Not sure if I missed something, but I am checking further (this is on ppc64le / RHEL 7.6) |
|
That test is categorised as unportable in https://github.com/LuaJIT/LuaJIT-test-cleanup and I believe the luajit2 testsuite is based on an old version of this. I haven't looked at it closely, but there may be issues with the test itself. It's worth some investigation. Please make a new issue/PR to discuss this though, since it seems like a distinct issue. An issue on my fork would be great but if you prefer using this fork it's fine too since I'm watching issues here too. I only cannot pull PRs from here and change issue/PR state, just like everyone else :) |
|
@siddhesh sure, can't see an issues tab on https://github.com/siddhesh/LuaJIT though, looks like something you may need to enable explicitly for the fork? thanks! |
|
Ahh, thanks for pointing out, I have enabled issues now. |
|
@siddhesh I created an issue moonjit#2, and noted the observations there. On your earlier comment, And yes, I do intend to get all of this stuff into luajit2. [1] https://www.freelists.org/post/luajit/Luajit-build-for-ppc64le_ since the JIT support may take a while to implement and is not necessarily required for all use cases, do you think we can move ahead with the current changes for Power and get them into luajit2? similar to what we already have for Z? |
|
@siddhesh now that the test failure for ppc64le is resolved, any thoughts on the above comments? Can we push forward with the upstreaming of the Power changes (without JIT support)? There is an increasing number of open source packages that depend on openresty, currently the build for openresty fails on Power because luajit2 fails to build, having these changes up-stream would help cross the first hurdle … thoughts? |
|
There you go @seth-priya: |
Create a patch for PPC64 support based on LuaJIT#140. https://bugzilla.redhat.com/show_bug.cgi?id=1591701 This patch has been rebased to match FPU support Author: Guy Menanteau <menantea@fr.ibm.com> Signed-Off-By: Marcin Kościelnicki <koriakin@0x04.net Signed-Off-By: Siddhesh Poyarekar <siddhesh@gotplt.org>
Create a patch for PPC64 support based on LuaJIT#140. https://bugzilla.redhat.com/show_bug.cgi?id=1591701 This patch has been rebased to match FPU support Author: Guy Menanteau <menantea@fr.ibm.com> Signed-Off-By: Marcin Kościelnicki <koriakin@0x04.net Signed-Off-By: Siddhesh Poyarekar <siddhesh@gotplt.org>
Create a patch for PPC64 support based on LuaJIT#140. https://bugzilla.redhat.com/show_bug.cgi?id=1591701 This patch has been rebased to match FPU support Author: Guy Menanteau <menantea@fr.ibm.com> Signed-Off-By: Marcin Kościelnicki <koriakin@0x04.net Signed-Off-By: Siddhesh Poyarekar <siddhesh@gotplt.org>
Create a patch for PPC64 support based on LuaJIT/LuaJIT#140. https://bugzilla.redhat.com/show_bug.cgi?id=1591701 This patch has been rebased to match FPU support Author: Guy Menanteau <menantea@fr.ibm.com> Signed-Off-By: Marcin Kościelnicki <koriakin@0x04.net Signed-Off-By: Siddhesh Poyarekar <siddhesh@gotplt.org> [ppc] Fix access beyond list in ipairs The load into TMP2 was incorrectly put into ENDIAN_LE, which made the subsequent check invalid. [ppc] Fix typo [ppc] Load BASEP4 as much as possible BASEP4 doesn't seem to get initialized all the time, especially when BASE is updated because of which programs can crash at random on ppc32. Err on the conservative side and set BASEP4 every time BASE_LO (or BASE_HI for LE) are accessed. This eventually needs to be tuned optimally. [ppc] Revert LE code for assert [ppc] Fix off by one in assert It ended up reading the first argument twice. Fix BC_POW on ppc64le
|
@mwkmwkmwk Could I ask for the annual (or even less) rebase of this patch on the top of the current LuaJIT, please? There seems to be some live in LuaJIT now, so perhaps we can finally try to persuade @MikePall that it could be a good idea to merge. However, it certainly won't be easy if this PR still has conflict. |
|
@mcepl For what it's worth, I've been using https://github.com/moonjit/moonjit successfully on PPC64le. It has patches that (to the best of my knowledge) are only on that fork, and is the only fork I've been able to use successfully on PPC64le. The maintainer stepped back a while ago, so it's technically unmaintained, but the code still works, and LuaJIT (unlike PUC Lua) actually maintains backwards compatibility, so feature-wise it's not falling behind. Not a great situation but hopefully survivable until at least the end of the current hardware cycle. If someone did want to look at this, it would definitely be important to make sure this fork (or whichever one we move forward with) is up to date with all the relevant patches. |
|
@elliottslaughter I know about moonjit. I am the lead maintainer of Lua packages for openSUSE, a Linux distribution. We have package for moonjit available, but it is not available for normal users, because it is difficult to keep two LuaJIT interpreters side-by-side (yes, it is possible to somehow make it more configurable and have only one switched on as the LuaJIT interpreter for the distribution, but it is too much work than I can provide; patches welcome!). Yes, we can live without PPC64 support for some time (a long time), but I would much prefer if the efforts could be harmonized into one project. |
|
| @@ -245,7 +241,7 @@ void emit_asm(BuildCtx *ctx) | |||
| int i, rel; | |||
|
|
|||
| fprintf(ctx->fp, "\t.file \"buildvm_%s.dasc\"\n", ctx->dasm_arch); | |||
| #if LJ_ARCH_PPC64 | |||
| #if LJ_ARCH_PPC_ELFV2 | |||
| @@ -86,10 +86,23 @@ typedef union FPRArg { | |||
| #elif LJ_TARGET_PPC | |||
|
|
|||
| #define CCALL_NARG_GPR 8 | |||
| #if LJ_ARCH_BITS == 64 | |||
| #define CCALL_NARG_FPR 13 | |||
There was a problem hiding this comment.
Is this is a number of arguments to be passed in FPR, it should be 14, AFAIK, and not restricted to ppc64: 32-bit ppc also has 14 FPRs available:
The registers that can be used to pass arguments to called functions are the general-purpose registers GPR3 through GPR10, the floating-point registers FPR1 through FPR13, and the vector registers V2 through V13 (see "Register Preservation" (page 23) for details).
http://personal.denison.edu/~bressoud/cs281-s07/MacOSXLowLevelABI.pdf
This patchset gets the existing ppc interpreter working on powerpc64-linux (with no JIT and no FFI). My end goal is to get the interpreter+FFI+JIT working on powerpc64le-linux, and this is the first patchset in the series (the next patches will enable little-endian support, ELFv2 ABI, then do the same for FFI and JIT).
What do you think?