add riscv64 backend for cranelift.

[yuyang-ok]

I am been trying to add riscv64 backend for cranelift these days. right now I have pass all run test in filetests.

some features not implemented right now. i128 mul div rem, all simd type and compare overflow.

some test need platform support. like bitrev need qemu-riscv64 support bitmanip and zbkb extension (don't know how to enable it.).

[yuyang-ok]

ci did not pass because I used is_riscv_feature_detected which is in std library.

[cfallin]

Hi @yuyang-ok -- thanks for this! I will take a detailed look next week when I'm back in the office. At a high level, my initial feedback is that new backends need to use ISLE for all instruction lowerings; we're working on migrating the others, and we don't want to create more work for ourselves by adopting a half-migrated backend. So, the lower_inst.rs file should be essentially empty (right now it has a bunch of handwritten lowerings). But now that you have a working backend, hopefully doing this is a bit easier than starting from scratch!

[yuyang-ok]

ok I will improve this.

[yuyang-ok]

@cfallin I didnot using isle to write a very much logic , I think isle is very good at match.turns out lower_inst.rs and isle.rs is quit large now!

[yuyang-ok]

@cfallin are you reviewing this?

[cfallin]

hi @yuyang-ok -- no, not yet; I just got back this morning. I would expect this to take some time, especially for a 21k-line PR.

At a high level, though, two questions:

• My question about ISLE is I think still unanswered: are you planning to update this PR to use only ISLE lowering patterns, and not handwritten code? That is our standard for all new backend code and we cannot merge a new backend that uses the legacy approach.

• Grepping for "todo" in this code, I still see a bunch of comments -- picking one example, "//todo what is this??" We unfortunately can't merge things like that (they are fine for draft PRs but not for production code): please make a pass through the PR and resolve all TODOs or uncertainties.

I think it would also be good to talk about ongoing expectations once a new backend is merged. I mentioned this in some earlier feedback as well, but: we want to make sure that every backend has some person or people who specialize in the architecture actively involved in the project. This is part of why I'm asking about your future plans: this PR is the start, but shouldn't be the only step. To this end, would you be willing to attend the Cranelift biweekly meeting at some point so we can talk about your new backend and plans?

Thanks for all your hard work on this so far!

[yuyang-ok]

@cfallin I think it is fine to use ISLE to lowering inst instead of handwritten code, I think maybe the way I am using ISLE is not very good right now,I should doing this part of work after the review.

yes,software is never an one time job,software will evolve from time to time,I think at least I can maintain riscv64 backend, I am happy attend the Cranelift biweekly meeting at some point.

[yuyang-ok]

@cfallin I think need provides an extractor.

(decl value_inst(Inst)Value)
(extern extractor value_inst value_inst)

look like I will need them in "trapff" ... lowering. So I can get compare parameters.

[yuyang-ok]

@cfallin still waiting for your review. :)

[cfallin]

@yuyang-ok unfortunately, I still have not gotten to it, sorry.

It looks like you still have code in legacy handwritten lowerings, though you're slowly moving it over to ISLE -- that's good, please keep doing that. I think it makes the most sense to review this PR once that is done.

LIkewise, I still see large blocks of commented-out code. As a random example, I see

        // println!("xxxx : {}", buffer.disasm.unwrap());
        let code = buffer.buffer.data();
        // write_to_a_file("/home/yuyang/tmp/code.bin", code);
        //0:   000015b7                lui     a1,0x1
        //4:   23458593                addi    a1,a1,564 # 0x1234
        //8:   00b5053b                addw    a0,a0,a1
        //c:   00008067  

Please carefully go over your code and remove anything you would not want merged into the main branch.

Regarding your continued pings: unfortunately, reviewing a 21,000-line PR is not a small task. It will probably take me a few days to a week to read through all of your code in detail. I have an enormous amount of work on my plate, with varying levels of priority. I also spent all last week with COVID and am still working through some backlog from that time away. So, I haven't gotten to this yet, and pinging me will not make the TODO list go faster.

While we very much appreciate the contribution -- having a RISC-V backend will be very valuable for Cranelift -- this process will require patience as well. Thanks!

[yuyang-ok]

@cfallin ok ,thanks,take care of yourself.

[yuyang-ok]

@cfallin ok.

[dunxen]

Hi @yuyang-ok, it's great that you've started the work on this, and thanks @cfallin for the mention. I do indeed have an interest in seeing this through.

I'll be able to give a thorough pass of this later today and throughout the rest of the week.

[yuyang-ok]

abi document I found it at https://github.com/riscv-non-isa/riscv-elf-psabi-doc

[yuyang-ok]

some part of wasmtime cannot compiler on target riscv64. @cfallin @afonso360 @bjorn3 @bjorn3

[cfallin]

@yuyang-ok a port of wasmtime's CPU-dependent bits -- the fiber library, and the trap handler details, at least -- would probably not be too much work beyond a pure Cranelift-only port, and is needed in any case for the port to be useful: otherwise, all we can do is exercise it with filetests. Enabling wasmtime on the platform allows us to run the Wasm test suite as well. So, yes, you've run into that missing support; would you be willing to build it? Or if not, perhaps another contributor could help?

[yuyang-ok]

@cfallin I like to give it a try.

[yuyang-ok]

can Someone help to port https://github.com/bytecodealliance/wasmtime/tree/main/crates/fiber to riscv64, I am not very understand of this lib. thanks.

[yuyang-ok]

@cfallin is this project rely on v8,there are still some part not compile.

[bjorn3]

If v8 doesn't support riscv, it should be fine to disable the fuzzers that check wasmtime against v8 I think.

[yuyang-ok]

@cfallin @bjorn3 @afonso360 @afonso360 I think it's the time to review.I think I fix most of the problems.

[cfallin]

@cfallin @bjorn3 @afonso360 @afonso360 I think it's the time to review.I think I fix most of the problems.

@yuyang-ok I scanned through the diff at places where I had left comments before, and it looks like almost none of my comments have been addressed. Some of them are fairly small things, like formatting and typo fixes; others are more substantial, like "no commented-out code, please" or a general indication that some code needs to be cleaned up. Could you please address all of these comments, and then let us know when that is done?

Also, above (in this comment) I see @dunxen expressed interest in making a clean-up pass over this PR. @dunxen, are you still willing to do this? If so, it seems that either after @yuyang-ok makes a pass or beforehand (coordinating with them as necessary) might be a good time for this. Thanks (and no worries if no longer able)!

I'm happy to see the qemu CI job added (does this mean Wasmtime fully works now?); I look forward to seeing this merged eventually, once it is cleaned up a bit!

[yuyang-ok]

@cfallin ok , I will do a full check.

[yuyang-ok]

wasmtime is not fully workd,because rusty_v8 cannot be compiler on riscv64.

[cfallin]

wasmtime is not fully workd,because rusty_v8 cannot be compiler on riscv64.

That's fine, as @bjorn3 mentions above V8 is only needed for one fuzzing target. You can disable it on riscv64.

[afonso360]

Running cargo test --target riscv64gc-unknown-linux-gnu from the root of the repository it looks like we are also missing some other stuff for wasmtime, such as trampolines and trap handling.

Output:

error: unsupported architecture
  --> crates/runtime/src/trampolines.rs:55:9
   |
55 |         compile_error!("unsupported architecture");
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: unsupported architecture
  --> crates/runtime/src/traphandlers/backtrace.rs:46:9
   |
46 |         compile_error!("unsupported architecture");
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: unsupported platform
   --> crates/runtime/src/traphandlers/unix.rs:232:13
    |
232 |             compile_error!("unsupported platform");
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: cannot find macro `wasm_to_libcall_trampoline` in this scope
   --> crates/runtime/src/libcalls.rs:100:17
    |
100 |                 wasm_to_libcall_trampoline!($name ; [<impl_ $name>]);
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
...
130 |     wasmtime_environ::foreach_builtin_function!(libcall);
    |     ---------------------------------------------------- in this macro invocation
    |
    = note: this error originates in the macro `libcall` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0433]: failed to resolve: use of undeclared crate or module `arch`
   --> crates/runtime/src/traphandlers/backtrace.rs:227:9
    |
227 |         arch::assert_entry_sp_is_aligned(first_wasm_sp);
    |         ^^^^ use of undeclared crate or module `arch`

error[E0433]: failed to resolve: use of undeclared crate or module `arch`
   --> crates/runtime/src/traphandlers/backtrace.rs:230:13
    |
230 |             arch::assert_fp_is_aligned(fp);
    |             ^^^^ use of undeclared crate or module `arch`

error[E0433]: failed to resolve: use of undeclared crate or module `arch`
   --> crates/runtime/src/traphandlers/backtrace.rs:242:16
    |
242 |             if arch::reached_entry_sp(fp, first_wasm_sp) {
    |                ^^^^ use of undeclared crate or module `arch`

error[E0433]: failed to resolve: use of undeclared crate or module `arch`
   --> crates/runtime/src/traphandlers/backtrace.rs:252:18
    |
252 |             pc = arch::get_next_older_pc_from_fp(fp);
    |                  ^^^^ use of undeclared crate or module `arch`

error[E0433]: failed to resolve: use of undeclared crate or module `arch`
   --> crates/runtime/src/traphandlers/backtrace.rs:258:24
    |
258 |             assert_eq!(arch::NEXT_OLDER_FP_FROM_FP_OFFSET, 0);
    |                        ^^^^ use of undeclared crate or module `arch`

error[E0433]: failed to resolve: use of undeclared crate or module `arch`
   --> crates/runtime/src/traphandlers/backtrace.rs:260:57
    |
260 |             let next_older_fp = *(fp as *mut usize).add(arch::NEXT_OLDER_FP_FROM_FP_OFFSET);
    |                                                         ^^^^ use of undeclared crate or module `arch`

error[E0308]: mismatched types
   --> crates/runtime/src/traphandlers/unix.rs:167:73
    |
167 | unsafe fn get_pc_and_fp(cx: *mut libc::c_void, _signum: libc::c_int) -> (*const u8, usize) {
    |           -------------                                                 ^^^^^^^^^^^^^^^^^^ expected tuple, found `()`
    |           |
    |           implicitly returns `()` as its body has no tail or `return` expression
    |
    = note:  expected tuple `(*const u8, usize)`
            found unit type `()`

Some errors have detailed explanations: E0308, E0433.
For more information about an error, try `rustc --explain E0308`.
error: could not compile `wasmtime-runtime` due to 11 previous errors
warning: build failed, waiting for other jobs to finish...

[yuyang-ok]

@afonso360 I will give a try to fix these.

[yuyang-ok]

a little update. cranelift can pass test on riscv64. @cfallin I think I resolved most of modify that you request.

wasmtime test test_successful_compile not pass. since I am not familiar with wasmtime's code . maybe still need some time to fix.

[yuyang-ok]

hey @cfallin @bjorn3 maybe need a little help here. https://bytecodealliance.zulipchat.com/#narrow/stream/206238-general/topic/as.20help.20about.20utf8_to_latin1.2E/near/293465163

thanks. :-)

[yuyang-ok]

https://bytecodealliance.zulipchat.com/#narrow/stream/206238-general/topic/cranelift.20opcode.20get_return_address.2E should load return address from stack if frame is set up?