Skip to content

add riscv64 backend for cranelift.#4271

Merged
cfallin merged 208 commits into
bytecodealliance:mainfrom
yuyang-ok:risc-v
Sep 28, 2022
Merged

add riscv64 backend for cranelift.#4271
cfallin merged 208 commits into
bytecodealliance:mainfrom
yuyang-ok:risc-v

Conversation

@yuyang-ok
Copy link
Copy Markdown
Contributor

@yuyang-ok yuyang-ok commented Jun 15, 2022

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

yuyang-ok commented Sep 25, 2022

@cfallin I think we are ready.
maybe some part not appropriate like https://github.com/yuyang-ok/wasmtime/blob/risc-v/crates/wasmtime/src/engine.rs#L493 .
maybe you can help me a little??

@a1phyr
Copy link
Copy Markdown
Contributor

a1phyr commented Sep 26, 2022

I think you could do the same that for s390x, i.e. calling getauxval by yourself.

You can find a RISC-V example in std_detect (the implementation of is_riscv64_feature_detected!) here.

Comment thread cranelift/native/Cargo.toml Outdated
@yuyang-ok
Copy link
Copy Markdown
Contributor Author

yuyang-ok commented Sep 26, 2022

I think you could do the same that for s390x, i.e. calling getauxval by yourself.

still look a little hard to use to me. look like need use C???

@yuyang-ok
Copy link
Copy Markdown
Contributor Author

@cfallin what do you think appropriate to solve is_riscv64_feature_detected???

@cfallin
Copy link
Copy Markdown
Member

cfallin commented Sep 27, 2022

@cfallin what do you think appropriate to solve is_riscv64_feature_detected???

If we want to wait for stabilization of the feature-detection macro in the standard library, I don't see anything wrong with assuming a baseline RISC-V ISA level for now (in other words, running with no features ever detected). Does this seem reasonable or would it result in important optimizations going missing?

Copy link
Copy Markdown
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

@yuyang-ok -- I went over the rest of the code and I think this looks generally fine. At least, passing all Wasm core tests indicates that this is of sufficient quality to merge, and can be refined and improved more in-tree.

I do have one minor comment below; and there's my answer to the getauxval question above. I'm not opposed to ignoring the getauxval issue for now (and simply not detecting features), if baseline RISC-V without extra ISA features is good enough to get started. And re: the below, a comment and factoring out the logic to a helper should be enough I think. Let me know once you've done this and then we can merge.

Thanks so much for the patience and hard work as we've reviewed this; I'm really excited to see it merge soon!

Comment thread cranelift/filetests/src/test_run.rs Outdated
@yuyang-ok
Copy link
Copy Markdown
Contributor Author

yuyang-ok commented Sep 27, 2022

Does this seem reasonable or would it result in important optimizations going missing?

popcnt,ctz,rev8,brev8 will fallback to loop implementation, seen reasonable to me, I think these instructions are used rare to me.

Copy link
Copy Markdown
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Updates look good to me -- thanks!

I'm going to go ahead and merge this now. Thanks again @yuyang-ok -- this was a huge amount of work.

More improvements are always welcome of course, from you and others -- anyone who wants to talk about further improvements to the RISC-V backend, please feel free to join our project meeting or file issues!

@cfallin cfallin merged commit cdecc85 into bytecodealliance:main Sep 28, 2022
@yuyang-ok
Copy link
Copy Markdown
Contributor Author

@cfallin ok,thanks. :-)

alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Sep 28, 2022
Our submodule was accidentally reverted to an older commit as part
of bytecodealliance#4271 and while it could be updated to as it was before I went ahead
and updated it to `main`.
alexcrichton added a commit that referenced this pull request Sep 28, 2022
* Update spec test repo

Our submodule was accidentally reverted to an older commit as part
of #4271 and while it could be updated to as it was before I went ahead
and updated it to `main`.

* Update ignore directives and test multi-memory

* Update riscv ignores
@rice7th
Copy link
Copy Markdown

rice7th commented Oct 8, 2022

@cfallin ok,thanks. :-)

Congratulations!

@martasp
Copy link
Copy Markdown

martasp commented Nov 12, 2022

How can I use this feature, is there documentation?
How can I set a target risc5 and produce Linux risc5 elf binary?
I would want to use it like that:

wasmtime exampleProgram.wasm --target risc5 

@archanox
Copy link
Copy Markdown

This work is for RISC-V not risc5.
You'll likely need to use riscv64gc as per the changes in this PR.

@martasp
Copy link
Copy Markdown

martasp commented Nov 12, 2022

I see that we have binaries riscv64gc here: https://github.com/bytecodealliance/wasmtime/releases/tag/v2.0.2 therefore It should probably work on riscv64 Linux. So I tried to install wasmtime in riscv64 Linux but without success.
Probably there is a difference between riscv64 and riscv64gc but I'm kinda new to these things, where can I learn it, any good resources?

[root@localhost ~]# curl https://wasmtime.dev/install.sh -sSf | bash
Error: Sorry! Wasmtime currently only provides pre-built binaries for x86_64 (Linux, macOS, Windows), aa
rch64 (Linux, macOS), and s390x (Linux).
 
[root@localhost ~]# uname
Linux
[root@localhost ~]# uname -p
riscv64

@bjorn3
Copy link
Copy Markdown
Contributor

bjorn3 commented Nov 12, 2022

The script at https://wasmtime.dev/install.sh hasn't been updated yet for riscv64 support. You should probably directly download the binary from https://github.com/bytecodealliance/wasmtime/releases/tag/v2.0.2.

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

Labels

cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:meta Everything related to the meta-language. cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants