Stream: git-wasmtime

Topic: wasmtime / issue #7481 riscv64: Use `vadd.vi` when subtra...

Wasmtime GitHub notifications bot (Nov 04 2023 at 11:29):

afonso360 added the cranelift label to Issue #7481.

Wasmtime GitHub notifications bot (Nov 04 2023 at 11:29):

afonso360 added the cranelift:E-easy label to Issue #7481.

Wasmtime GitHub notifications bot (Nov 04 2023 at 11:29):

afonso360 added the cranelift:area:riscv64 label to Issue #7481.

Wasmtime GitHub notifications bot (Nov 04 2023 at 11:29):

afonso360 opened issue #7481:

:wave: Hey,

The RISC-V vector backend has no vsub.vi instruction. That instruction would subtract a vector register with an immediate. It does have a vrsub.vi that computes imm - vector.

Instead of emitting a vsub.vi we can negate the immediate and emit a vadd.vi that does exist.

Example testcase

Here's an example test case that currently does not compile to an optimal instruction sequence:

function %isub_imm(i64x2) -> i64x2 {
block0(v0: i64x2):
    v1 = vconst.i64x2 [1 1]
    v2 = isub v0, v1
    return v2
}

This emits:

  vle64.v v13,[const(0)] #avl=2, #vtype=(e64, m1, ta, ma)
  vsub.vv v13,v9,v13 #avl=2, #vtype=(e64, m1, ta, ma)

It loads the constant and then emits the .vv version of the instruction. But we could just emit vadd.vi v13, v9, -1.

Implementation

Recognizing constants is currently done using the replicated_imm5 constructor. We probably need to build an equivalent constructor that matches with an equivalent negated Imm5

Afterwards we should only need to add a rule that matches an isub with an imm5 on the right hand side. We should only need one since for subs with constants in the left hand side, we already have a vrsub instruction.

I've recently implemented a similar set of rules for scalar sub in https://github.com/bytecodealliance/wasmtime/pull/7480/commits/6bc69c095a1ada4b4ee7190baaf45aaf7d85c5f2.

Wasmtime GitHub notifications bot (Nov 04 2023 at 11:29):

afonso360 added the good first issue label to Issue #7481.

Wasmtime GitHub notifications bot (Nov 04 2023 at 13:32):

BieVic commented on issue #7481:

I would like to do it, but I can wait in case someone else wants to get started with Cranelift :)

Wasmtime GitHub notifications bot (Nov 07 2023 at 20:02):

pyroMechanical commented on issue #7481:

I'd like to take this on as a first project! I have prior rust and compiler experience, but haven't worked with cranelift before.

Wasmtime GitHub notifications bot (Nov 07 2023 at 20:29):

BieVic commented on issue #7481:

Awesome! Let me or @afonso360 know if you need any help

Wasmtime GitHub notifications bot (Nov 09 2023 at 01:12):

pyroMechanical commented on issue #7481:

I think i've gotten an okay start, but I'm not sure how to use the example test case to see if my changes are producing the desired result. I've also tried using cargo's testing but I'm not sure how to test for a different ISA.

Wasmtime GitHub notifications bot (Nov 09 2023 at 09:02):

BieVic commented on issue #7481:

For testing you can use clif-util (e.g. wasmtime/target/debug/clif-util), with which you can also target different ISAs. I'm also fairly new and this doc about testing helped me wrap my head around

Wasmtime GitHub notifications bot (Nov 23 2023 at 14:58):

afonso360 closed issue #7481:

:wave: Hey,

The RISC-V vector backend has no vsub.vi instruction. That instruction would subtract a vector register with an immediate. It does have a vrsub.vi that computes imm - vector.

Instead of emitting a vsub.vi we can negate the immediate and emit a vadd.vi that does exist.

Example testcase

Here's an example test case that currently does not compile to an optimal instruction sequence:

function %isub_imm(i64x2) -> i64x2 {
block0(v0: i64x2):
    v1 = vconst.i64x2 [1 1]
    v2 = isub v0, v1
    return v2
}

This emits:

  vle64.v v13,[const(0)] #avl=2, #vtype=(e64, m1, ta, ma)
  vsub.vv v13,v9,v13 #avl=2, #vtype=(e64, m1, ta, ma)

It loads the constant and then emits the .vv version of the instruction. But we could just emit vadd.vi v13, v9, -1.

Implementation

Recognizing constants is currently done using the replicated_imm5 constructor. We probably need to build an equivalent constructor that matches with an equivalent negated Imm5

Afterwards we should only need to add a rule that matches an isub with an imm5 on the right hand side. We should only need one since for subs with constants in the left hand side, we already have a vrsub instruction.

I've recently implemented a similar set of rules for scalar sub in https://github.com/bytecodealliance/wasmtime/pull/7480/commits/6bc69c095a1ada4b4ee7190baaf45aaf7d85c5f2.

Last updated: Oct 23 2024 at 20:03 UTC