Skip to content

build: enable Maglev for riscv64#62605

Open
JamieMagee wants to merge 1 commit into
nodejs:mainfrom
JamieMagee:riscv64-enable-maglev
Open

build: enable Maglev for riscv64#62605
JamieMagee wants to merge 1 commit into
nodejs:mainfrom
JamieMagee:riscv64-enable-maglev

Conversation

@JamieMagee

Copy link
Copy Markdown
Contributor

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:

  • configure.py didn't include riscv64 in maglev_enabled_architectures
  • tools/v8_gypfiles/v8.gyp had no GN-scraper conditions for riscv64 in the two Maglev source blocks (it had arm, arm64, x64, s390x, ppc64, just not riscv64)

This adds riscv64 to both, following the same approach as #60863 which did this for s390x.

Refs: nodejs/build#4099

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency. labels Apr 5, 2026
@Renegade334 Renegade334 added dont-land-on-v20.x dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. riscv64 Issues and PRs related to the riscv64 architecture. labels Apr 8, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@Renegade334

Copy link
Copy Markdown
Member

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?

@sxa

sxa commented Apr 9, 2026

Copy link
Copy Markdown
Member

@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.

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>
@JamieMagee JamieMagee force-pushed the riscv64-enable-maglev branch from 1114602 to 5d9447e Compare June 30, 2026 04:08
@JamieMagee

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main.

@kxxt

kxxt commented Jun 30, 2026

Copy link
Copy Markdown
Member

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?

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 kxxt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Renegade334 Renegade334 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM

@Renegade334 Renegade334 added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2026
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jun 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor
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

@richardlau richardlau removed the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Jun 30, 2026
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 30, 2026
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jun 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor
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

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@sxa

sxa commented Jun 30, 2026

Copy link
Copy Markdown
Member

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.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@sxa

sxa commented Jun 30, 2026

Copy link
Copy Markdown
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to build files or the CI. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. needs-ci PRs that need a full CI run. request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. riscv64 Issues and PRs related to the riscv64 architecture. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants