Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

riscv64: Improve scalar_to_vector codegen for constants #7189

Closed
afonso360 opened this issue Oct 8, 2023 · 2 comments · Fixed by #7284
Closed

riscv64: Improve scalar_to_vector codegen for constants #7189

afonso360 opened this issue Oct 8, 2023 · 2 comments · Fixed by #7284
Labels
cranelift:area:riscv64 Issues related to the RISC-V 64 backend. cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers!

Comments

@afonso360
Copy link
Contributor

afonso360 commented Oct 8, 2023 •
edited
Loading

👋 Hey,

This is a fairly simple improvement. Cranelift's scalar_to_vector instruction grabs a scalar value and places it at the bottom of a vector register.

We translate this into the vmerge.vxm instruction in the RISC-V backend.

This is all fairly ok, but we don't have a rule to match the vmerge.vim instruction that can place an immediate value instead of a register.

Implementing this should be fairly simple, we can create a rule similar to the existing ones but using a imm5_from_value extractror:

(rule 2 (lower (has_type (ty_vec_fits_in_register ty) (scalar_to_vector (imm5_from_value x))))

This imm5_from_value extractor doesn't exist yet, but we already have an equivalent in the replicated_imm5 extractor. It should be fairly easy to extract that into it's own separate extractor for use in this new rule.

See the existing uimm5_from_value extractor for help.

If anyone needs help working on this issue let me know!
@afonso360 afonso360 added cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers! cranelift:area:riscv64 Issues related to the RISC-V 64 backend. labels Oct 8, 2023
@afonso360 afonso360 changed the title riscv64: Improve scalar_to_vector codegen riscv64: Improve scalar_to_vector codegen for constants Oct 8, 2023
@BieVic
Copy link
Contributor

BieVic commented Oct 12, 2023

Hey! I'm fairly new to Cranelift but would like to give it a try.

@afonso360
Copy link
Contributor Author

Awesome 🎉 Let me know if you need any help getting started!

BieVic added a commit to BieVic/wasmtime that referenced this issue Oct 17, 2023
@BieVic
riscv: Add rule for vmerge.vim
2855a07 
Additionally add an imm5_from_value extractor. Closes bytecodealliance#7189
BieVic added a commit to BieVic/wasmtime that referenced this issue Oct 18, 2023
@BieVic
riscv: Add tests for vmerge.vim
0ae1ec8 
Closes bytecodealliance#7189
BieVic added a commit to BieVic/wasmtime that referenced this issue Oct 18, 2023
@BieVic
riscv: Add rule for vmerge.vim
3cb0a2a 
Additionally add an imm5_from_value extractor. Closes bytecodealliance#7189
BieVic added a commit to BieVic/wasmtime that referenced this issue Oct 18, 2023
@BieVic
riscv: Add tests for vmerge.vim
0f2fe3f 
Closes bytecodealliance#7189
@BieVic BieVic mentioned this issue Oct 18, 2023
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 19, 2023
@BieVic
riscv64: Improve scalar_to_vector for constants (#7284)
70039d8 
* riscv: Add rule for vmerge.vim

Additionally add an imm5_from_value extractor. Closes #7189

* riscv: Add tests for vmerge.vim

Closes #7189

* riscv: format tests

* riscv: format scalar_to_vector rule

* riscv: add runtests and rm duplicate definition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cranelift:area:riscv64 Issues related to the RISC-V 64 backend. cranelift:E-easy Issues suitable for newcomers to investigate, including Rust newcomers!
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

riscv64: Improve scalar_to_vector for constants BieVic/wasmtime
2 participants
@afonso360 @BieVic