build: enable Maglev for riscv64#62605
Conversation
|
Review requested:
|
|
Started CI to satisfy the PR requirements although there is no riscv runner – perhaps someone from @nodejs/platform-riscv64 can verify the build passes the node/V8 test suites? |
|
@Renegade334 Yep I'm looking at what I can do - although we've got quite a lot of RISC-V PRs around at the moment so it won't be quick. |
9b0d257 to
1114602
Compare
V8's Maglev compiler has supported riscv64 since V8 14.0 (with full source files in deps/v8/src/maglev/riscv/), but Node.js never wired it up: - configure.py excluded riscv64 from maglev_enabled_architectures - tools/v8_gypfiles/v8.gyp lacked GN-scraper conditions for riscv64 Maglev sources in both the v8_internal_headers and v8_base_without_compiler blocks This adds riscv64 to both, following the same pattern used for s390x in nodejs#60863 and matching V8's own BUILD.gn which already lists riscv64 alongside arm, arm64, x64, s390x, and ppc64 as Maglev-enabled architectures. Refs: nodejs/build#4099 Signed-off-by: Jamie Magee <jamie.magee@gmail.com>
1114602 to
5d9447e
Compare
|
Rebased onto latest main. |
I am testing this one in my CI to see if any tests regress: https://github.com/riscv-forks/node-riscv/actions/runs/28420684700?pr=5 |
kxxt
left a comment
There was a problem hiding this comment.
I can confirm that there are no regressions with this change. Test results:
[19:21|% 100|+ 5701|- 2]: Done
Failed tests:
out/Release/node /home/kxxt/node-riscv/test/parallel/test-snapshot-reproducible.js
out/Release/node /home/kxxt/node-riscv/test/wpt/test-compression.js
The first one is a known failure and the second one is flaky, which should be fixed by #64027.
Failed to start CI- Validating Jenkins credentials ✔ Jenkins credentials valid - Querying data for job/node-test-pull-request/72555/ [SyntaxError: Unexpected token '<', ..." https://github.com/nodejs/node/actions/runs/28438109518 |
Failed to start CI- Validating Jenkins credentials ✔ Jenkins credentials valid - Querying data for job/node-test-pull-request/72555/ [SyntaxError: Unexpected token '<', ..." https://github.com/nodejs/node/actions/runs/28444126538 |
|
Kicked off https://ci.nodejs.org/job/node-test-pull-request/74455/ manually as the failure message from an hour ago is referencing node-test-pull-request/72555/ which no longer exists on the CI. |
|
Win32 failures in latest run are due to #64216 so while I'd probably be ok to land it if you want to rebase so we can get a clean CI that would be good. |
V8's BUILD.gn has listed riscv64 as a Maglev-supported architecture since V8 14.0, and the source files exist in
deps/v8/src/maglev/riscv/. But Node.js never wired it up on its side:This adds riscv64 to both, following the same approach as #60863 which did this for s390x.
Refs: nodejs/build#4099