-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PPC32 port guidance #30323
Comments
The IBM fork of Node.js for PPC was merged into this repository years ago (around Node.js 4) but only 64-bit was supported. IBM did build 32-bit PPC binaries for a while but I don't think we supported Power5 and we definitely had reports that the fork didn't work on e500 (ibmruntimes/node#67). cc @nodejs/platform-ppc |
I'm doing a cross-build (Buildroot build for e200 target so I assume simulation is on) and currently stuck on error: static assertion failed: FIELD_SIZE(kOptionalPaddingOffset) == kHeaderPaddingSize) Just trying to get far enough to start working the other asm related build failures I'd expect based on the other tickets. |
If you're targeting a 32-bits system then you probably need this in deps/v8/src/objects/code.h: #elif V8_TARGET_ARCH_PPC32 // or whatever it's called
static constexpr int kHeaderPaddingSize =
FLAG_enable_embedded_constant_pool ? 24 : 0;
#else
// ... |
@bnoordhuis thanks for that. I still see to get the following error. I'll keep looking at my build configuration and see if some initial flag isn't getting set. My cross toolchain is attempting to build for a e200 system. I'm not doing a native build of nodejs, maybe that's the issue? |
@rc-matthew-l-weber do you configure with - -target-cpu=ppc? |
Also does anyone here know if it's ok to remove the some of the STATIC_ASSERT calls? (I'm building my own version for PPC natively on my PowerBook G4 1.67ghz 15") |
I removed some of the STATIC_ASSERT calls and now I'm having an error because of the missing information for the jump tables on PPC @bnoordhuis Do you know what the values for the jump tables on PPC are? |
./configure --prefix=/usr --dest-cpu=ppc --without-snapshot --shared-zlib --shared-cares --shared-libuv --without-dtrace --without-etw --cross-compiling --shared-nghttp2 --dest-os=linux --without-ssl --with-intl=none --without-npm |
That's unlikely to end well. :-) What jump tables are you referring to? There are quite a few places in V8 where it generates them. |
The part where it needs data that is Arch dependent, I think it was the jump size table |
This issue has been inactive for over 8 months. I'll go ahead and close it. |
The support needed for v8 is being tracked via https://chromium-review.googlesource.com/c/v8/v8/+/2083019 . The review is due for a code update, but I am working behind the scenes to get a delivery ready for our customer. As our customer is on nodejs 12.16.1, the work is going to be ported to the mainstream v8 release (and as such will also make it into nodejs). |
@woutervermeiren That never got merged. Has anything more been done since then? I'm new to the source trees of both node.js and v8, and not sure how best to even build that version of v8 with a contemporary version of node.js bolted on. I'd really appreciate it if you'd point me in the right direction. If you've moved on or don't remember anymore, though, that's fine, too. Thanks for the hard work. |
Hi @wyatt8740 , I am no longer with the company that tasked me fix this. We got to the point where internally the code was workable, but I have no view on whether (or if) the code is close to being upstreamed to have a working port for PPC32. With respect to your question about combining v8 and nodejs versions, I would advise to have a look at the dominant tree (either v8 of nodejs) in your case. And then look across and determine what version of the other tree was the most recent supported one (for you dominent version). Trying to mix-and-match different versions will probably lead to API mismatches. That said, you could get lucky if no major refactoring was done between the most recently supported version and the version you are seeking to use. |
@woutervermeiren Is it publicly available? I am interested to work on fixing V8 and Node.js for PPC, though primarily for Darwin. (I am Macports contributor, so I can bring it there.) |
@richardlau Page is dead. Do you know if the project still exists, or at least who participated? Thanks in advance. |
@barracuda156 That's a GitHub team -- team members are visible to members of the org (nodejs in this case). The team currently maintains the ppc64 ports of V8. IBM/Red Hat haven't maintained a ppc 32-bit port of V8 in years and even when they did that was for later PowerPC chips than used for Darwin. |
@richardlau Thank you! From what I gathered it looks indeed that mostly POWER8+ processors are targeted, but it seems that some success on G5 and even G4 has been reached. (After all, it should be perfectly feasible to substitute missing instructions from later ISA or disable fancy optimizations like VMX.) @jhamby Has done some work, though on Linux: |
No, I mean the stuff that is in https://github.com/ibmruntimes/v8ppc (now archived) which was way earlier. Earliest was (I think) https://github.com/ibmruntimes/v8ppc/tree/export-to-node-Aug-22-2013. When IBM started porting V8 and Node.js to PPC I think we targetted POWER 5+ as that is what we had available at the time (for comparison, G5 macs are based on POWER 4). AFAIK everything in https://github.com/ibmruntimes/v8ppc was upstreamed to V8 but I don't have the corresponding V8 CL references. This would have all been ancient versions of Node.js (0.10 and 0.12). By the time the iojs fork merged back with Node.js and Node.js 4 came out we'd already switched to maintaining PPC64 upstream and had dropped 32-bit PPC. FWIW when we were using https://github.com/ibmruntimes/node we did tag the PPC equivalent branches and tags, e.g.
with the equivalent v8ppc code tagged in the form |
@richardlau Thank you, this is very helpful! I will look through the references and think what can be done. I do not really expect to get the latest |
The patch I made is not publicly avaible unfortunately. It was finished when I left the company but it was not merged. Please contact the company for more information: [email protected]. It is best to ask for Arnout Vandecapelle. |
Here is the full patchset we had the team port against nodejs 12.18.0 |
@matthew-l-weber Thank you very much! Trying to build it now. Build system needs extra fixes for Darwin PPC (expectedly so), which is not too problematic, but I will need to see if there are any ABI incompatibilities (which could be harder to fix). |
Well, the code clock doesn't just perform static checks but also calls the function We have to look at other 32-bit targets to determine what the proper operations for tagged pointers on a 32-bit targets are. |
From what I was getting, it did not look like that – I would expect more significant mismatches, if ppc64 values were used. (But I need to check if I added something into the relevant header.) |
Looks like the function |
With the following patch: Index: nodejs-18.12.1+dfsg/deps/v8/src/objects/code.h
===================================================================
--- nodejs-18.12.1+dfsg.orig/deps/v8/src/objects/code.h
+++ nodejs-18.12.1+dfsg/deps/v8/src/objects/code.h
@@ -657,6 +657,9 @@ class Code : public HeapObject {
static constexpr int kHeaderPaddingSize = 12;
#elif V8_TARGET_ARCH_MIPS
static constexpr int kHeaderPaddingSize = 12;
+#elif V8_TARGET_ARCH_PPC
+ static constexpr int kHeaderPaddingSize =
+ FLAG_enable_embedded_constant_pool ? 8 : 0;
#elif V8_TARGET_ARCH_PPC64
static constexpr int kHeaderPaddingSize =
FLAG_enable_embedded_constant_pool ? (COMPRESS_POINTERS_BOOL ? 8 : 52)
Index: nodejs-18.12.1+dfsg/deps/v8/src/codegen/ppc/macro-assembler-ppc.h
===================================================================
--- nodejs-18.12.1+dfsg.orig/deps/v8/src/codegen/ppc/macro-assembler-ppc.h
+++ nodejs-18.12.1+dfsg/deps/v8/src/codegen/ppc/macro-assembler-ppc.h
@@ -967,6 +967,7 @@ class V8_EXPORT_PRIVATE TurboAssembler :
// ---------------------------------------------------------------------------
// Pointer compression Support
+#if !defined(V8_TARGET_ARCH_PPC)
void SmiToPtrArrayOffset(Register dst, Register src) {
#if defined(V8_COMPRESS_POINTERS) || defined(V8_31BIT_SMIS_ON_64BIT_ARCH)
STATIC_ASSERT(kSmiTag == 0 && kSmiShift < kSystemPointerSizeLog2);
@@ -976,6 +977,7 @@ class V8_EXPORT_PRIVATE TurboAssembler :
ShiftRightS64(dst, src, Operand(kSmiShift - kSystemPointerSizeLog2));
#endif
}
+#endif
// Loads a field containing a HeapObject and decompresses it if pointer
// compression is enabled.
@@ -1313,7 +1315,7 @@ class V8_EXPORT_PRIVATE MacroAssembler :
bne(not_smi_label, cr0);
}
-#if !defined(V8_COMPRESS_POINTERS) && !defined(V8_31BIT_SMIS_ON_64BIT_ARCH)
+#if !defined(V8_COMPRESS_POINTERS) && !defined(V8_31BIT_SMIS_ON_64BIT_ARCH) && !defined(V8_TARGET_ARCH_PPC)
// Ensure it is permissible to read/write int value directly from
// upper half of the smi.
STATIC_ASSERT(kSmiTag == 0); the build proceeds very far but eventually fails with:
|
Looks like this requires just a tiny fix: --- nodejs-18.12.1+dfsg.orig/deps/v8/src/runtime/runtime-utils.h
+++ nodejs-18.12.1+dfsg/deps/v8/src/runtime/runtime-utils.h
@@ -39,7 +39,7 @@ static inline ObjectPair MakePair(Object
#if defined(V8_TARGET_LITTLE_ENDIAN)
return x.ptr() | (static_cast<ObjectPair>(y.ptr()) << 32);
#elif defined(V8_TARGET_BIG_ENDIAN)
- return y->ptr() | (static_cast<ObjectPair>(x->ptr()) << 32);
+ return y.ptr() | (static_cast<ObjectPair>(x.ptr()) << 32);
#else
#error Unknown endianness
#endif |
@glaubitz This sounds inspiring! I will try this out in coming days. (Away from my PPC hardware atm, but I can try the build in Rosetta.) |
My current patch against version 18.12.1 looks like this: Index: nodejs-18.12.1+dfsg/deps/v8/src/objects/code.h
===================================================================
--- nodejs-18.12.1+dfsg.orig/deps/v8/src/objects/code.h
+++ nodejs-18.12.1+dfsg/deps/v8/src/objects/code.h
@@ -657,6 +657,9 @@ class Code : public HeapObject {
static constexpr int kHeaderPaddingSize = 12;
#elif V8_TARGET_ARCH_MIPS
static constexpr int kHeaderPaddingSize = 12;
+#elif V8_TARGET_ARCH_PPC
+ static constexpr int kHeaderPaddingSize =
+ FLAG_enable_embedded_constant_pool ? 8 : 0;
#elif V8_TARGET_ARCH_PPC64
static constexpr int kHeaderPaddingSize =
FLAG_enable_embedded_constant_pool ? (COMPRESS_POINTERS_BOOL ? 8 : 52)
Index: nodejs-18.12.1+dfsg/deps/v8/src/codegen/ppc/macro-assembler-ppc.h
===================================================================
--- nodejs-18.12.1+dfsg.orig/deps/v8/src/codegen/ppc/macro-assembler-ppc.h
+++ nodejs-18.12.1+dfsg/deps/v8/src/codegen/ppc/macro-assembler-ppc.h
@@ -967,6 +967,7 @@ class V8_EXPORT_PRIVATE TurboAssembler :
// ---------------------------------------------------------------------------
// Pointer compression Support
+#if !defined(V8_TARGET_ARCH_PPC)
void SmiToPtrArrayOffset(Register dst, Register src) {
#if defined(V8_COMPRESS_POINTERS) || defined(V8_31BIT_SMIS_ON_64BIT_ARCH)
STATIC_ASSERT(kSmiTag == 0 && kSmiShift < kSystemPointerSizeLog2);
@@ -976,6 +977,7 @@ class V8_EXPORT_PRIVATE TurboAssembler :
ShiftRightS64(dst, src, Operand(kSmiShift - kSystemPointerSizeLog2));
#endif
}
+#endif
// Loads a field containing a HeapObject and decompresses it if pointer
// compression is enabled.
@@ -1313,7 +1315,7 @@ class V8_EXPORT_PRIVATE MacroAssembler :
bne(not_smi_label, cr0);
}
-#if !defined(V8_COMPRESS_POINTERS) && !defined(V8_31BIT_SMIS_ON_64BIT_ARCH)
+#if !defined(V8_COMPRESS_POINTERS) && !defined(V8_31BIT_SMIS_ON_64BIT_ARCH) && !defined(V8_TARGET_ARCH_PPC)
// Ensure it is permissible to read/write int value directly from
// upper half of the smi.
STATIC_ASSERT(kSmiTag == 0);
Index: nodejs-18.12.1+dfsg/deps/v8/src/runtime/runtime-utils.h
===================================================================
--- nodejs-18.12.1+dfsg.orig/deps/v8/src/runtime/runtime-utils.h
+++ nodejs-18.12.1+dfsg/deps/v8/src/runtime/runtime-utils.h
@@ -39,7 +39,7 @@ static inline ObjectPair MakePair(Object
#if defined(V8_TARGET_LITTLE_ENDIAN)
return x.ptr() | (static_cast<ObjectPair>(y.ptr()) << 32);
#elif defined(V8_TARGET_BIG_ENDIAN)
- return y->ptr() | (static_cast<ObjectPair>(x->ptr()) << 32);
+ return y.ptr() | (static_cast<ObjectPair>(x.ptr()) << 32);
#else
#error Unknown endianness
#endif and gets the build very far but eventually fails with:
I will continue working on this tomorrow. |
@glaubitz Have you used anything from either #30323 (comment) or https://github.com/jhamby/void-packages/tree/master/srcpkgs/nodejs/patches ? |
No external patches. I took the Debian unstable package and tried to get it to build. The result is the above patch. Bummer that the Void people haven't upstreamed their changes. |
Wonder if
Agree. @jhamby Have you tried submitting those fixes to upstream? Given they are for Linux, it should be fairly uncontroversial. (And merging them gonna be somewhat helpful for macOS PPC too.) |
I don't think so. The Void patch which enables runtime detection for older PowerPC CPUs [1] is missing and without it, you won't have much luck on actual older machines. |
Yeah, I have added old CPUs support to |
These changes need to be upstreamed to V8. Otherwise, we keep rebasing the patches against newer upstream versions. |
I assume many improvements have not been upstreamed yet because of Google's complicated contribution guidelines [1]. I have signed Google's CLA and could upstream some of the changes though. |
I could try with upstream, but it makes a better sense to do it once Node builds locally. |
I just gave it a go with the first tiny change: https://chromium-review.googlesource.com/c/v8/v8/+/4124995 If that works, I will start upstreaming the changes which we know are correct. |
@glaubitz Did it work eventually or failing on the same spot, as of now? |
Haven't had the time to continue working on it. Will do that later this week. |
With these patches node-12 compiles well. Besides one needs edit ./lib/buffer.js and change "if (start <= 0)" to "if (start === undefined || start <= 0)". |
@glaubitz @biofreak I returned to the project and trying to build P. S. For reference, branch is here: https://github.com/barracuda156/node-ppc – but very WIP, does not yet build and likely contains errors. |
@glaubitz @biofreak @richardlau UPD. Reverted to However I am stuck at
Both space on disk and RAM should be more than sufficient. I have no idea what causes this, assuming it has been building for ppc32 for someone. |
This commit applies a set of patches from: nodejs#30323 (comment)
And, I tried to build
@matthew-l-weber How did you get around this? It should have affected Linux or AIX, I guess, just as much. |
@glaubitz UPD. Removed a screenshot of an error, since it was a result of fixing a bug wrongly, so irrelevant. (MacOS code in embedded uses |
While I assume it will not work yet, from here it should build on macOS PowerPC: https://github.com/barracuda156/v8-ppc/tree/8.3-powerpc-darwin |
@bnoordhuis @richardlau @matthew-l-weber @glaubitz It fails consistently, unrelated to V8/nodejs version (whatever I tried to build at least), both with v8 directly and with Nodejs’s build of V8, and, AFAICT, it fails unrelated to whatever patches I added to support macOS (since I bumped into that error initially and it never changed, despite multiple version of V8 itself and my Darwin-specific attempts to fix the build). I have to disable it at the moment, since I could not track the cause of the error: v8/v8@ee917c6 I suspect it could be either wrong size of bool (which is 4 bytes on Darwin ppc) or wrong size of spinlock (also 4 bytes), but do not know where to fix that. Notice, it also failed with Nodejs 12 (the earliest version I tried), which pre-dated all those breaking commits with compressed pointers. In subsequent versions, especially after 8.4.x, more stuff was broken for |
UPD. Oh, I totally forgot, maybe I should tweak magic numbers mentioned here: #30323 (comment) |
No, that does not help:
So apparently 8 is wrong (on macOS), 16 should be there, but isolate-data.h assert fail regardless. |
Something is not right with those compress pointers defines, since this results in error at runtime:
I.e., something sets |
Compare against this: https://github.com/ibmruntimes/v8ppc/blob/538162502416b1b1e7253e5cd6e87821e19df04f/src/ppc/macro-assembler-ppc.h#L1260-L1266 Upstream of V8 has broken the code with the wrong condition in v8/v8@7caea48 commit. It should be:
|
Before I get started I figured I should ask around about current state.
For this port, I'm targetting the PowerPC ISA v.2.03 using the Book III-E specification. (Freescale PowerPC e200/e500 and IBM PowerPC 405/440/460/970/Power5/Power6)
I have looked a little bit at the different changes IBM has done over the years and still need to evaluate if their fork already has this support. (https://stackoverflow.com/questions/18519899/is-it-possible-to-run-nodejs-on-powerpc-based-linux)
Is there a "zero", emulation or non-native mode that I could use instead of adding another processor ISA support?
The text was updated successfully, but these errors were encountered: