1880366 - disnative should support the RISC-V backend

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P5)

Description

[Hovav Shacham]

Attachment: rv-disas.patch

User Agent: Mozilla/5.0 (X11; CrOS x86_64 14541.0.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/121.0.0.0 Safari/537.36

Steps to reproduce:

In the JS shell when using the RISC-V backend, warmed up a function f and ran print(disnative(f))

Actual results:

The fallback codepath in js/src/jit/Disassemble.cpp fired, printing the message *** No disassembly available ***.

Expected results:

The RISC-V backend disassembler (in js/src/jit/riscv64/disasm/) could have been used to disassemble the function and print out the disassembly.

The RISC-V backend's disassembler interface closely resembles the 32-bit ARM disassembler interface. I've attached an example patch that implements RISC-V disassembly and seems to work okay in my testing. (Caveat: I was working on an older FF release and haven't checked if Bug 1875363 also affects the RISC-V disassembler.)

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::JavaScript Engine: JIT' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

This seems worth just landing -- did you want to upload the patch with moz-phab and take credit, or shall I take point on getting this in?

Comment 3

[Hovav Shacham]

There's not much patch to take credit for, but I'm happy to do whatever is easiest for y'all. I've tested the patch on a recent checkout and I think I've got moz-phab configured -- should I submit? Should I list any reviewers?

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

Go forth and submit since you're setup. You can use r?#spidermonkey-reviewers!, and we'll triage that to the right reviewer. Otherwise, you can go r?bvisness... who is who I currently would get to look at this :)

Comment 5

[Hovav Shacham]

Attachment: Bug 1880366 - [riscv] Add RISC-V support to jit::Disassemble. r?#spidermonkey-reviewers!

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:hovav, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 7

[Hovav Shacham]

Thanks for the heads up, BugBot! I think the patch over in Phabricator is ready to go, but I wasn't sure what the etiquette was around landing it, or if it would be better to wait for a specific time in the release cycle. I can go ahead whenever assuming that's the right thing to do and that I have the required permissions.

Comment 8

[Matthew Gaudet (he/him) [:mgaudet]]

Oh; Ben, typically with contributor patches they can't land them themselves, so we press the button for them :)

Comment 9

[Ben Visness [:bvisness]]

Oh, sorry about that! I've queued it up for landing now.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2d7d253c14a
[riscv] Add RISC-V support to jit::Disassemble. r=bvisness

Comment 11

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/c2d7d253c14a