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.

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

riscv64: Support non 128bit vector sizes #6266

Merged
merged 5 commits into from
Apr 25, 2023

Conversation

afonso360
Copy link
Contributor

@afonso360 afonso360 commented Apr 22, 2023

👋 Hey,

This is a small follow up to #6240. This PR does a couple of things:

  • We now recognize the Zvl* family of extensions, which really just specify the minimum size of a vector register.
  • Instead of gating our vector lowerings on has_v and assuming 128 bit registers, we now allow lowering any size as long as they fit in a single register.
    • We switch the type extractor to ty_vec_fits_in_register which feels slightly neater than (if-let $true (has_v)) everywhere
  • Also iadd uses fits_in_64 which also matches small vectors, so I switched the rules to using ty_int_ref_scalar_64 which only match scalars.

This isn't really required, but it's neat that we get all of these lowerings without pretty much any effort, so I figured it would be worth it.

@afonso360 afonso360 requested a review from a team as a code owner April 22, 2023 18:38
@afonso360 afonso360 requested review from fitzgen and removed request for a team April 22, 2023 18:38
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:meta Everything related to the meta-language. labels Apr 22, 2023
@alexcrichton
Copy link
Member

Oh oops my comments on #6268 are probably more appropriate over here

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Should zvl128b imply the v extension or vice-versa?

@afonso360
Copy link
Contributor Author

afonso360 commented Apr 25, 2023

V should imply zvl128b, but not vice-versa. We don't yet have presets for v, but that shouldn't be too difficult to add. I'll add that in this PR as well.

@alexcrichton
Copy link
Member

Oh right yes, I forgot that has_* is (at least following other backends) intended to be an individual feature. Feel free to defer a v feature to a future PR, no need to have it as part of this one.

@afonso360 afonso360 added this pull request to the merge queue Apr 25, 2023
Merged via the queue into bytecodealliance:main with commit 4337ccd Apr 25, 2023
@afonso360 afonso360 deleted the riscv-vec-size branch April 25, 2023 15:28
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Apr 28, 2023
* riscv64: Add `Zvl` extensions

* riscv64: Allow lowering SIMD operations that fit in a vector register

* riscv64: Support non 128bit vector sizes

* riscv64: Add Zvl Presets

* riscv64: Precompute `min_vec_reg_size`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

2 participants