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: Implement SIMD icmp #6609

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

afonso360
Copy link
Contributor

@afonso360 afonso360 commented Jun 20, 2023

👋 Hey,

This PR Implements SIMD icmp for RISC-V. These rules are implemented as a combination of two steps, mask generation and mask expansion. Our comparison rules only return their results as a mask register, so we need to expand the mask into lane sized elements.

We have 20 (!) comparison instructions, nearly the full table of all IntCC codes in VV, VX and VI formats. However there are some holes in this table.

They are:

  • vmsltu.vi (Less than Unsigned (Vec-Imm))
  • vmslt.vi (Less than (Vec-Imm))
  • vmsgtu.vv (Greater than Unsigned (Vec-Vec))
  • vmsgt.vv (Greater than (Vec-Vec))
  • vmsgeu.* (Greater than or equal Unsigned (All formats))
  • vmsge.* (Greater than or equal (All formats))

Most of these can be replaced with the inverted IntCC instruction. To minimize the size of this initial PR I've only implemented rules for the opcodes that we have a direct translation.

However, in order to get all IntCC's working I've implemented some of the inverted instruction which are vmsgtu.vv, vmsgt.vv, vmsgeu.vv, vmsge.vv. These are implemented as alias to their inverted counterparts (with the inputs swapped).

I'm planning on adding a follow up commit with the rest of the VX and VI rules in both the LHS an RHS sides. We should end up with 5 rules per IntCC once this is all done.


I've split the actual mask expansion into it's own separate rule since we are going to need it for the fcmp rules as well.

The instruction selection for icmp is on a separate rule simply because the rules end up less verbose than if they were inlined directly into the icmp rule.

These are implemented as a combination of two steps, mask generation and
mask expansion. Our comparision rules only return their results as a mask
register, so we need to expand the mask into lane sized elements.

We have 20 (!) comparision instructions, nearly the full table of all IntCC codes
in VV, VX and VI formats. However there are some holes in this table.

They are:
* `vmsltu.vi`
* `vmslt.vi`
* `vmsgtu.vv`
* `vmsgt.vv`
* `vmsgeu.*`
* `vmsge.*`

Most of these can be replaces with the inverted IntCC instruction, however
this commit only implements the existing instructions without any inversion
and the inverted VV versions of `sgtu`/`sgt`/`sgeu`/`sge` since we need them
to get the full icmp functionality.

I've split the actual mask expansion into it's own separate rule since we are
going to need it for the `fcmp` rules as well.

The instruction selection for `icmp` is on a separate rule simply because the
rulse end up less verbose than if they were inlined directly into the `icmp` rule.
@afonso360 afonso360 added the cranelift:area:riscv64 Issues related to the RISC-V 64 backend. label Jun 20, 2023
@afonso360 afonso360 requested review from a team as code owners June 20, 2023 20:42
@afonso360 afonso360 requested review from cfallin and removed request for a team June 20, 2023 20:42
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.

Nice! I like the organization of the lowerings as it's quite clear to see what's going on and scan over things quickly if necessary.

@afonso360 afonso360 added this pull request to the merge queue Jun 20, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 20, 2023
@afonso360 afonso360 added this pull request to the merge queue Jun 20, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 20, 2023
@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Jun 21, 2023
@afonso360 afonso360 added this pull request to the merge queue Jun 21, 2023
Merged via the queue into bytecodealliance:main with commit b05a09c Jun 21, 2023
@afonso360 afonso360 deleted the riscv-simd-icmp branch June 21, 2023 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:riscv64 Issues related to the RISC-V 64 backend. cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants