[PATCH 00/12] target/riscv: Fix some RISC-V instruction corner cases

[PATCH 00/12] target/riscv: Fix some RISC-V instruction corner cases

[Anton Blanchard]

This series fixes some RISC-V instruction corner cases, specifically
illegal overlaps between mask and source registers, illegal overlaps
between source registers and illegal overlaps between source and
destination registers. These were found by looking at miscompares
between QEMU and the Tenstorrent fork of Whisper which models this
behaviour better than Spike and Sail.

Anton Blanchard (12):
  target/riscv: Source vector registers cannot overlap mask register
  target/riscv: handle vrgather mask and source overlap
  target/riscv: handle vadd.vx form mask and source overlap
  target/riscv: handle vadd.vv form mask and source overlap
  target/riscv: handle vslide1down.vx form mask and source overlap
  target/riscv: handle vzext.vf2 form mask and source overlap
  target/riscv: handle vwadd.vx form mask and source overlap
  target/riscv: handle vwadd.vv form mask and source overlap
  target/riscv: handle vwadd.wv form mask and source overlap
  target/riscv: handle vwadd.wv form vs1 and vs2 overlap
  target/riscv: Add CHECK arg to GEN_OPFVF_WIDEN_TRANS
  target/riscv: handle overlap in widening instructions with overwrite

 target/riscv/insn_trans/trans_rvv.c.inc | 139 ++++++++++++++++++------
 1 file changed, 108 insertions(+), 31 deletions(-)

-- 
2.34.1

[PATCH 01/12] target/riscv: Source vector registers cannot overlap mask register

[Anton Blanchard]

Add the relevant ISA paragraphs explaining why source (and destination)
registers cannot overlap the mask register.

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 29 ++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index b9883a5d32..20b1cb127b 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -100,10 +100,33 @@ static bool require_scale_rvfmin(DisasContext *s)
     }
 }
 
-/* Destination vector register group cannot overlap source mask register. */
-static bool require_vm(int vm, int vd)
+/*
+ * Source and destination vector register groups cannot overlap source mask
+ * register:
+ *
+ * A vector register cannot be used to provide source operands with more than
+ * one EEW for a single instruction. A mask register source is considered to
+ * have EEW=1 for this constraint. An encoding that would result in the same
+ * vector register being read with two or more different EEWs, including when
+ * the vector register appears at different positions within two or more vector
+ * register groups, is reserved.
+ * (Section 5.2)
+ *
+ * A destination vector register group can overlap a source vector
+ * register group only if one of the following holds:
+ *  1. The destination EEW equals the source EEW.
+ *  2. The destination EEW is smaller than the source EEW and the overlap
+ *     is in the lowest-numbered part of the source register group.
+ *  3. The destination EEW is greater than the source EEW, the source EMUL
+ *     is at least 1, and the overlap is in the highest-numbered part of
+ *     the destination register group.
+ * For the purpose of determining register group overlap constraints, mask
+ * elements have EEW=1.
+ * (Section 5.2)
+ */
+static bool require_vm(int vm, int v)
 {
-    return (vm != 0 || vd != 0);
+    return (vm != 0 || v != 0);
 }
 
 static bool require_nf(int vd, int nf, int lmul)
-- 
2.34.1

Re: [PATCH 01/12] target/riscv: Source vector registers cannot overlap mask register

[Max Chou]

Reviewed-by: Max Chou <max.chou@sifive.com>


On 2025/1/26 3:20 PM, Anton Blanchard wrote:
> Add the relevant ISA paragraphs explaining why source (and destination)
> registers cannot overlap the mask register.
>
> Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
> ---
>   target/riscv/insn_trans/trans_rvv.c.inc | 29 ++++++++++++++++++++++---
>   1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index b9883a5d32..20b1cb127b 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -100,10 +100,33 @@ static bool require_scale_rvfmin(DisasContext *s)
>       }
>   }
>   
> -/* Destination vector register group cannot overlap source mask register. */
> -static bool require_vm(int vm, int vd)
> +/*
> + * Source and destination vector register groups cannot overlap source mask
> + * register:
> + *
> + * A vector register cannot be used to provide source operands with more than
> + * one EEW for a single instruction. A mask register source is considered to
> + * have EEW=1 for this constraint. An encoding that would result in the same
> + * vector register being read with two or more different EEWs, including when
> + * the vector register appears at different positions within two or more vector
> + * register groups, is reserved.
> + * (Section 5.2)
> + *
> + * A destination vector register group can overlap a source vector
> + * register group only if one of the following holds:
> + *  1. The destination EEW equals the source EEW.
> + *  2. The destination EEW is smaller than the source EEW and the overlap
> + *     is in the lowest-numbered part of the source register group.
> + *  3. The destination EEW is greater than the source EEW, the source EMUL
> + *     is at least 1, and the overlap is in the highest-numbered part of
> + *     the destination register group.
> + * For the purpose of determining register group overlap constraints, mask
> + * elements have EEW=1.
> + * (Section 5.2)
> + */
> +static bool require_vm(int vm, int v)
>   {
> -    return (vm != 0 || vd != 0);
> +    return (vm != 0 || v != 0);
>   }
>   
>   static bool require_nf(int vd, int nf, int lmul)

[PATCH 02/12] target/riscv: handle vrgather mask and source overlap

[Anton Blanchard]

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 20b1cb127b..c66cd95bdb 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3453,7 +3453,9 @@ static bool vrgather_vv_check(DisasContext *s, arg_rmrr *a)
            require_align(a->rs1, s->lmul) &&
            require_align(a->rs2, s->lmul) &&
            (a->rd != a->rs2 && a->rd != a->rs1) &&
-           require_vm(a->vm, a->rd);
+           require_vm(a->vm, a->rd) &&
+           require_vm(a->vm, a->rs1) &&
+           require_vm(a->vm, a->rs2);
 }
 
 static bool vrgatherei16_vv_check(DisasContext *s, arg_rmrr *a)
@@ -3470,7 +3472,9 @@ static bool vrgatherei16_vv_check(DisasContext *s, arg_rmrr *a)
                           a->rs1, 1 << MAX(emul, 0)) &&
            !is_overlapped(a->rd, 1 << MAX(s->lmul, 0),
                           a->rs2, 1 << MAX(s->lmul, 0)) &&
-           require_vm(a->vm, a->rd);
+           require_vm(a->vm, a->rd) &&
+           require_vm(a->vm, a->rs1) &&
+           require_vm(a->vm, a->rs2);
 }
 
 GEN_OPIVV_TRANS(vrgather_vv, vrgather_vv_check)
@@ -3483,7 +3487,8 @@ static bool vrgather_vx_check(DisasContext *s, arg_rmrr *a)
            require_align(a->rd, s->lmul) &&
            require_align(a->rs2, s->lmul) &&
            (a->rd != a->rs2) &&
-           require_vm(a->vm, a->rd);
+           require_vm(a->vm, a->rd) &&
+           require_vm(a->vm, a->rs2);
 }
 
 /* vrgather.vx vd, vs2, rs1, vm # vd[i] = (x[rs1] >= VLMAX) ? 0 : vs2[rs1] */
-- 
2.34.1

Re: [PATCH 02/12] target/riscv: handle vrgather mask and source overlap

[Max Chou]

Hi Anton,

You might need to extend this patch or provide a new patch to handle
the different EEWs source operands checking for the vrgatherei16.vv
instruction (when SEW is not 16).

Thanks,
Max


On 2025/1/26 3:20 PM, Anton Blanchard wrote:
> Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
> ---
>   target/riscv/insn_trans/trans_rvv.c.inc | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index 20b1cb127b..c66cd95bdb 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -3453,7 +3453,9 @@ static bool vrgather_vv_check(DisasContext *s, arg_rmrr *a)
>              require_align(a->rs1, s->lmul) &&
>              require_align(a->rs2, s->lmul) &&
>              (a->rd != a->rs2 && a->rd != a->rs1) &&
> -           require_vm(a->vm, a->rd);
> +           require_vm(a->vm, a->rd) &&
> +           require_vm(a->vm, a->rs1) &&
> +           require_vm(a->vm, a->rs2);
>   }
>   
>   static bool vrgatherei16_vv_check(DisasContext *s, arg_rmrr *a)
> @@ -3470,7 +3472,9 @@ static bool vrgatherei16_vv_check(DisasContext *s, arg_rmrr *a)
>                             a->rs1, 1 << MAX(emul, 0)) &&
>              !is_overlapped(a->rd, 1 << MAX(s->lmul, 0),
>                             a->rs2, 1 << MAX(s->lmul, 0)) &&
> -           require_vm(a->vm, a->rd);
> +           require_vm(a->vm, a->rd) &&
> +           require_vm(a->vm, a->rs1) &&
> +           require_vm(a->vm, a->rs2);
>   }
>   
>   GEN_OPIVV_TRANS(vrgather_vv, vrgather_vv_check)
> @@ -3483,7 +3487,8 @@ static bool vrgather_vx_check(DisasContext *s, arg_rmrr *a)
>              require_align(a->rd, s->lmul) &&
>              require_align(a->rs2, s->lmul) &&
>              (a->rd != a->rs2) &&
> -           require_vm(a->vm, a->rd);
> +           require_vm(a->vm, a->rd) &&
> +           require_vm(a->vm, a->rs2);
>   }
>   
>   /* vrgather.vx vd, vs2, rs1, vm # vd[i] = (x[rs1] >= VLMAX) ? 0 : vs2[rs1] */

[PATCH 03/12] target/riscv: handle vadd.vx form mask and source overlap

[Anton Blanchard]

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index c66cd95bdb..bc2780497e 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -382,6 +382,7 @@ static bool vext_check_ld_index(DisasContext *s, int vd, int vs2,
 static bool vext_check_ss(DisasContext *s, int vd, int vs, int vm)
 {
     return require_vm(vm, vd) &&
+           require_vm(vm, vs) &&
            require_align(vd, s->lmul) &&
            require_align(vs, s->lmul);
 }
-- 
2.34.1

Re: [PATCH 03/12] target/riscv: handle vadd.vx form mask and source overlap

[Max Chou]

Hi Anton,

I think that the commit message could be improved for better clarity.
The vext_check_ss function affects more RVV instructions than the 
vadd.vx instruction alone.
(PS:perhaps using the category (OPIVX/OPFVF/etc.) to describe the 
affected RVV instructions would be more helpful.)
Additionally, the patch 04/07/08/09/10 also have the same issue.

Thanks,
Max


On 2025/1/26 3:20 PM, Anton Blanchard wrote:
> Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
> ---
>   target/riscv/insn_trans/trans_rvv.c.inc | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index c66cd95bdb..bc2780497e 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -382,6 +382,7 @@ static bool vext_check_ld_index(DisasContext *s, int vd, int vs2,
>   static bool vext_check_ss(DisasContext *s, int vd, int vs, int vm)
>   {
>       return require_vm(vm, vd) &&
> +           require_vm(vm, vs) &&
>              require_align(vd, s->lmul) &&
>              require_align(vs, s->lmul);
>   }

[PATCH 04/12] target/riscv: handle vadd.vv form mask and source overlap

[Anton Blanchard]

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index bc2780497e..f5ba1c4280 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -403,6 +403,7 @@ static bool vext_check_ss(DisasContext *s, int vd, int vs, int vm)
 static bool vext_check_sss(DisasContext *s, int vd, int vs1, int vs2, int vm)
 {
     return vext_check_ss(s, vd, vs2, vm) &&
+           require_vm(vm, vs1) &&
            require_align(vs1, s->lmul);
 }
 
-- 
2.34.1

[PATCH 05/12] target/riscv: handle vslide1down.vx form mask and source overlap

[Anton Blanchard]

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index f5ba1c4280..a873536eea 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -609,6 +609,7 @@ static bool vext_check_slide(DisasContext *s, int vd, int vs2,
 {
     bool ret = require_align(vs2, s->lmul) &&
                require_align(vd, s->lmul) &&
+               require_vm(vm, vs2) &&
                require_vm(vm, vd);
     if (is_over) {
         ret &= (vd != vs2);
-- 
2.34.1

Re: [PATCH 05/12] target/riscv: handle vslide1down.vx form mask and source overlap

[Max Chou]

Hi Anton,

The vext_check_slide function affects the 
vslide[up|down].v[x|i]/vfslide1[up|down].vf/vslide1[up|down].vx
instructions than the vslide1down.vx instruction alone.
Therefore, it would be more appropriate to update the commit message to 
provide a clearer information.
(PS:perhaps, using the “vector slide instructions” to replace the 
specified vslide1down.vx instruction would be better.)
The patch 06 also has the same issue.

Thanks,
Max


On 2025/1/26 3:20 PM, Anton Blanchard wrote:
> Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
> ---
>   target/riscv/insn_trans/trans_rvv.c.inc | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index f5ba1c4280..a873536eea 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -609,6 +609,7 @@ static bool vext_check_slide(DisasContext *s, int vd, int vs2,
>   {
>       bool ret = require_align(vs2, s->lmul) &&
>                  require_align(vd, s->lmul) &&
> +               require_vm(vm, vs2) &&
>                  require_vm(vm, vd);
>       if (is_over) {
>           ret &= (vd != vs2);

[PATCH 06/12] target/riscv: handle vzext.vf2 form mask and source overlap

[Anton Blanchard]

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index a873536eea..0952bcbe2c 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3631,6 +3631,7 @@ static bool int_ext_check(DisasContext *s, arg_rmr *a, uint8_t div)
         require_align(a->rd, s->lmul) &&
         require_align(a->rs2, s->lmul - div) &&
         require_vm(a->vm, a->rd) &&
+        require_vm(a->vm, a->rs2) &&
         require_noover(a->rd, s->lmul, a->rs2, s->lmul - div);
     return ret;
 }
-- 
2.34.1

[PATCH 07/12] target/riscv: handle vwadd.vx form mask and source overlap

[Anton Blanchard]

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 0952bcbe2c..bc22b42801 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -458,13 +458,14 @@ static bool vext_check_mss(DisasContext *s, int vd, int vs1, int vs2)
  *      instruction cannot overlap the source mask register (v0).
  *      (Section 5.3)
  */
-static bool vext_wide_check_common(DisasContext *s, int vd, int vm)
+static bool vext_wide_check_common(DisasContext *s, int vd, int vs, int vm)
 {
     return (s->lmul <= 2) &&
            (s->sew < MO_64) &&
            ((s->sew + 1) <= (s->cfg_ptr->elen >> 4)) &&
            require_align(vd, s->lmul + 1) &&
-           require_vm(vm, vd);
+           require_vm(vm, vd) &&
+           require_vm(vm, vs);
 }
 
 /*
@@ -498,14 +499,14 @@ static bool vext_narrow_check_common(DisasContext *s, int vd, int vs2,
 
 static bool vext_check_ds(DisasContext *s, int vd, int vs, int vm)
 {
-    return vext_wide_check_common(s, vd, vm) &&
+    return vext_wide_check_common(s, vd, vs, vm) &&
            require_align(vs, s->lmul) &&
            require_noover(vd, s->lmul + 1, vs, s->lmul);
 }
 
 static bool vext_check_dd(DisasContext *s, int vd, int vs, int vm)
 {
-    return vext_wide_check_common(s, vd, vm) &&
+    return vext_wide_check_common(s, vd, vs, vm) &&
            require_align(vs, s->lmul + 1);
 }
 
-- 
2.34.1

[PATCH 08/12] target/riscv: handle vwadd.vv form mask and source overlap

[Anton Blanchard]

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index bc22b42801..45b2868c54 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -525,6 +525,7 @@ static bool vext_check_dd(DisasContext *s, int vd, int vs, int vm)
 static bool vext_check_dss(DisasContext *s, int vd, int vs1, int vs2, int vm)
 {
     return vext_check_ds(s, vd, vs2, vm) &&
+           require_vm(vm, vs1) &&
            require_align(vs1, s->lmul) &&
            require_noover(vd, s->lmul + 1, vs1, s->lmul);
 }
-- 
2.34.1

[PATCH 09/12] target/riscv: handle vwadd.wv form mask and source overlap

[Anton Blanchard]

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 45b2868c54..2309d9abd0 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -548,6 +548,7 @@ static bool vext_check_dss(DisasContext *s, int vd, int vs1, int vs2, int vm)
 static bool vext_check_dds(DisasContext *s, int vd, int vs1, int vs2, int vm)
 {
     return vext_check_ds(s, vd, vs1, vm) &&
+           require_vm(vm, vs2) &&
            require_align(vs2, s->lmul + 1);
 }
 
-- 
2.34.1

[PATCH 10/12] target/riscv: handle vwadd.wv form vs1 and vs2 overlap

[Anton Blanchard]

for 2*SEW = 2*SEW op SEW instructions vs2 and vs1 cannot overlap
because it would mean a register is read with two different SEW
settings.

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 2309d9abd0..312d8b1b81 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -549,7 +549,8 @@ static bool vext_check_dds(DisasContext *s, int vd, int vs1, int vs2, int vm)
 {
     return vext_check_ds(s, vd, vs1, vm) &&
            require_vm(vm, vs2) &&
-           require_align(vs2, s->lmul + 1);
+           require_align(vs2, s->lmul + 1) &&
+           !is_overlapped(vs2, 1 << MAX(s->lmul+1, 0), vs1, 1 << MAX(s->lmul, 0));
 }
 
 static bool vext_check_sd(DisasContext *s, int vd, int vs, int vm)
-- 
2.34.1

Re: [PATCH 10/12] target/riscv: handle vwadd.wv form vs1 and vs2 overlap

[Max Chou]

Hi Anton,

This patch violates some coding style rules of QEMU.
You can verify the coding style by running the checkpatch.pl script in 
the QEMU repository.
(ref: 
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#use-the-qemu-coding-style)
The patch 12 also has the same issue.

Thanks,
Max


On 2025/1/26 3:20 PM, Anton Blanchard wrote:
> for 2*SEW = 2*SEW op SEW instructions vs2 and vs1 cannot overlap
> because it would mean a register is read with two different SEW
> settings.
>
> Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
> ---
>   target/riscv/insn_trans/trans_rvv.c.inc | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index 2309d9abd0..312d8b1b81 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -549,7 +549,8 @@ static bool vext_check_dds(DisasContext *s, int vd, int vs1, int vs2, int vm)
>   {
>       return vext_check_ds(s, vd, vs1, vm) &&
>              require_vm(vm, vs2) &&
> -           require_align(vs2, s->lmul + 1);
> +           require_align(vs2, s->lmul + 1) &&
> +           !is_overlapped(vs2, 1 << MAX(s->lmul+1, 0), vs1, 1 << MAX(s->lmul, 0));
>   }
>   
>   static bool vext_check_sd(DisasContext *s, int vd, int vs, int vm)

[PATCH 11/12] target/riscv: Add CHECK arg to GEN_OPFVF_WIDEN_TRANS

[Anton Blanchard]

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 312d8b1b81..2741f8bd8e 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2410,10 +2410,10 @@ static bool opfvf_widen_check(DisasContext *s, arg_rmrr *a)
 }
 
 /* OPFVF with WIDEN */
-#define GEN_OPFVF_WIDEN_TRANS(NAME)                              \
+#define GEN_OPFVF_WIDEN_TRANS(NAME, CHECK)                       \
 static bool trans_##NAME(DisasContext *s, arg_rmrr *a)           \
 {                                                                \
-    if (opfvf_widen_check(s, a)) {                               \
+    if (CHECK(s, a)) {                                           \
         uint32_t data = 0;                                       \
         static gen_helper_opfvf *const fns[2] = {                \
             gen_helper_##NAME##_h, gen_helper_##NAME##_w,        \
@@ -2429,8 +2429,8 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)           \
     return false;                                                \
 }
 
-GEN_OPFVF_WIDEN_TRANS(vfwadd_vf)
-GEN_OPFVF_WIDEN_TRANS(vfwsub_vf)
+GEN_OPFVF_WIDEN_TRANS(vfwadd_vf, opfvf_widen_check)
+GEN_OPFVF_WIDEN_TRANS(vfwsub_vf, opfvf_widen_check)
 
 static bool opfwv_widen_check(DisasContext *s, arg_rmrr *a)
 {
@@ -2512,7 +2512,7 @@ GEN_OPFVF_TRANS(vfrdiv_vf,  opfvf_check)
 
 /* Vector Widening Floating-Point Multiply */
 GEN_OPFVV_WIDEN_TRANS(vfwmul_vv, opfvv_widen_check)
-GEN_OPFVF_WIDEN_TRANS(vfwmul_vf)
+GEN_OPFVF_WIDEN_TRANS(vfwmul_vf, opfvf_widen_check)
 
 /* Vector Single-Width Floating-Point Fused Multiply-Add Instructions */
 GEN_OPFVV_TRANS(vfmacc_vv, opfvv_check)
@@ -2537,10 +2537,10 @@ GEN_OPFVV_WIDEN_TRANS(vfwmacc_vv, opfvv_widen_check)
 GEN_OPFVV_WIDEN_TRANS(vfwnmacc_vv, opfvv_widen_check)
 GEN_OPFVV_WIDEN_TRANS(vfwmsac_vv, opfvv_widen_check)
 GEN_OPFVV_WIDEN_TRANS(vfwnmsac_vv, opfvv_widen_check)
-GEN_OPFVF_WIDEN_TRANS(vfwmacc_vf)
-GEN_OPFVF_WIDEN_TRANS(vfwnmacc_vf)
-GEN_OPFVF_WIDEN_TRANS(vfwmsac_vf)
-GEN_OPFVF_WIDEN_TRANS(vfwnmsac_vf)
+GEN_OPFVF_WIDEN_TRANS(vfwmacc_vf, opfvf_widen_check)
+GEN_OPFVF_WIDEN_TRANS(vfwnmacc_vf, opfvf_widen_check)
+GEN_OPFVF_WIDEN_TRANS(vfwmsac_vf, opfvf_widen_check)
+GEN_OPFVF_WIDEN_TRANS(vfwnmsac_vf, opfvf_widen_check)
 
 /* Vector Floating-Point Square-Root Instruction */
 
-- 
2.34.1

Re: [PATCH 11/12] target/riscv: Add CHECK arg to GEN_OPFVF_WIDEN_TRANS

[Max Chou]

Reviewed-by: Max Chou <max.chou@sifive.com>


On 2025/1/26 3:20 PM, Anton Blanchard wrote:
> Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
> ---
>   target/riscv/insn_trans/trans_rvv.c.inc | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index 312d8b1b81..2741f8bd8e 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -2410,10 +2410,10 @@ static bool opfvf_widen_check(DisasContext *s, arg_rmrr *a)
>   }
>   
>   /* OPFVF with WIDEN */
> -#define GEN_OPFVF_WIDEN_TRANS(NAME)                              \
> +#define GEN_OPFVF_WIDEN_TRANS(NAME, CHECK)                       \
>   static bool trans_##NAME(DisasContext *s, arg_rmrr *a)           \
>   {                                                                \
> -    if (opfvf_widen_check(s, a)) {                               \
> +    if (CHECK(s, a)) {                                           \
>           uint32_t data = 0;                                       \
>           static gen_helper_opfvf *const fns[2] = {                \
>               gen_helper_##NAME##_h, gen_helper_##NAME##_w,        \
> @@ -2429,8 +2429,8 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)           \
>       return false;                                                \
>   }
>   
> -GEN_OPFVF_WIDEN_TRANS(vfwadd_vf)
> -GEN_OPFVF_WIDEN_TRANS(vfwsub_vf)
> +GEN_OPFVF_WIDEN_TRANS(vfwadd_vf, opfvf_widen_check)
> +GEN_OPFVF_WIDEN_TRANS(vfwsub_vf, opfvf_widen_check)
>   
>   static bool opfwv_widen_check(DisasContext *s, arg_rmrr *a)
>   {
> @@ -2512,7 +2512,7 @@ GEN_OPFVF_TRANS(vfrdiv_vf,  opfvf_check)
>   
>   /* Vector Widening Floating-Point Multiply */
>   GEN_OPFVV_WIDEN_TRANS(vfwmul_vv, opfvv_widen_check)
> -GEN_OPFVF_WIDEN_TRANS(vfwmul_vf)
> +GEN_OPFVF_WIDEN_TRANS(vfwmul_vf, opfvf_widen_check)
>   
>   /* Vector Single-Width Floating-Point Fused Multiply-Add Instructions */
>   GEN_OPFVV_TRANS(vfmacc_vv, opfvv_check)
> @@ -2537,10 +2537,10 @@ GEN_OPFVV_WIDEN_TRANS(vfwmacc_vv, opfvv_widen_check)
>   GEN_OPFVV_WIDEN_TRANS(vfwnmacc_vv, opfvv_widen_check)
>   GEN_OPFVV_WIDEN_TRANS(vfwmsac_vv, opfvv_widen_check)
>   GEN_OPFVV_WIDEN_TRANS(vfwnmsac_vv, opfvv_widen_check)
> -GEN_OPFVF_WIDEN_TRANS(vfwmacc_vf)
> -GEN_OPFVF_WIDEN_TRANS(vfwnmacc_vf)
> -GEN_OPFVF_WIDEN_TRANS(vfwmsac_vf)
> -GEN_OPFVF_WIDEN_TRANS(vfwnmsac_vf)
> +GEN_OPFVF_WIDEN_TRANS(vfwmacc_vf, opfvf_widen_check)
> +GEN_OPFVF_WIDEN_TRANS(vfwnmacc_vf, opfvf_widen_check)
> +GEN_OPFVF_WIDEN_TRANS(vfwmsac_vf, opfvf_widen_check)
> +GEN_OPFVF_WIDEN_TRANS(vfwnmsac_vf, opfvf_widen_check)
>   
>   /* Vector Floating-Point Square-Root Instruction */
>   

[PATCH 12/12] target/riscv: handle overlap in widening instructions with overwrite

[Anton Blanchard]

In these instructions vd is considered a source, so no overlap
is allowed between vd and vs1/vs2. See:

https://github.com/riscv/riscv-isa-manual/issues/1789

Signed-off-by: Anton Blanchard <antonb@tenstorrent.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 71 +++++++++++++++++++------
 1 file changed, 56 insertions(+), 15 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 2741f8bd8e..715008db79 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -1505,6 +1505,16 @@ static bool opivv_widen_check(DisasContext *s, arg_rmrr *a)
            vext_check_dss(s, a->rd, a->rs1, a->rs2, a->vm);
 }
 
+/* OPIVV with overwrite and WIDEN */
+static bool opivv_overwrite_widen_check(DisasContext *s, arg_rmrr *a)
+{
+    return require_rvv(s) &&
+           vext_check_isa_ill(s) &&
+           vext_check_dss(s, a->rd, a->rs1, a->rs2, a->vm) &&
+           !is_overlapped(a->rd, 1 << MAX(s->lmul+1, 0), a->rs1, 1 << MAX(s->lmul, 0)) &&
+           !is_overlapped(a->rd, 1 << MAX(s->lmul+1, 0), a->rs2, 1 << MAX(s->lmul, 0));
+}
+
 static bool do_opivv_widen(DisasContext *s, arg_rmrr *a,
                            gen_helper_gvec_4_ptr *fn,
                            bool (*checkfn)(DisasContext *, arg_rmrr *))
@@ -1552,6 +1562,15 @@ static bool opivx_widen_check(DisasContext *s, arg_rmrr *a)
            vext_check_ds(s, a->rd, a->rs2, a->vm);
 }
 
+/* OPIVX with overwrite and WIDEN */
+static bool opivx_overwrite_widen_check(DisasContext *s, arg_rmrr *a)
+{
+    return require_rvv(s) &&
+           vext_check_isa_ill(s) &&
+           vext_check_ds(s, a->rd, a->rs2, a->vm) &&
+           !is_overlapped(a->rd, 1 << MAX(s->lmul+1, 0), a->rs2, 1 << MAX(s->lmul, 0));
+}
+
 #define GEN_OPIVX_WIDEN_TRANS(NAME, CHECK) \
 static bool trans_##NAME(DisasContext *s, arg_rmrr *a)                    \
 {                                                                         \
@@ -2023,13 +2042,13 @@ GEN_OPIVX_TRANS(vmadd_vx, opivx_check)
 GEN_OPIVX_TRANS(vnmsub_vx, opivx_check)
 
 /* Vector Widening Integer Multiply-Add Instructions */
-GEN_OPIVV_WIDEN_TRANS(vwmaccu_vv, opivv_widen_check)
-GEN_OPIVV_WIDEN_TRANS(vwmacc_vv, opivv_widen_check)
-GEN_OPIVV_WIDEN_TRANS(vwmaccsu_vv, opivv_widen_check)
-GEN_OPIVX_WIDEN_TRANS(vwmaccu_vx, opivx_widen_check)
-GEN_OPIVX_WIDEN_TRANS(vwmacc_vx, opivx_widen_check)
-GEN_OPIVX_WIDEN_TRANS(vwmaccsu_vx, opivx_widen_check)
-GEN_OPIVX_WIDEN_TRANS(vwmaccus_vx, opivx_widen_check)
+GEN_OPIVV_WIDEN_TRANS(vwmaccu_vv, opivv_overwrite_widen_check)
+GEN_OPIVV_WIDEN_TRANS(vwmacc_vv, opivv_overwrite_widen_check)
+GEN_OPIVV_WIDEN_TRANS(vwmaccsu_vv, opivv_overwrite_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwmaccu_vx, opivx_overwrite_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwmacc_vx, opivx_overwrite_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwmaccsu_vx, opivx_overwrite_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwmaccus_vx, opivx_overwrite_widen_check)
 
 /* Vector Integer Merge and Move Instructions */
 static bool trans_vmv_v_v(DisasContext *s, arg_vmv_v_v *a)
@@ -2370,6 +2389,18 @@ static bool opfvv_widen_check(DisasContext *s, arg_rmrr *a)
            vext_check_dss(s, a->rd, a->rs1, a->rs2, a->vm);
 }
 
+/* Vector Widening Floating-Point Add/Subtract Instructions with overwrite */
+static bool opfvv_overwrite_widen_check(DisasContext *s, arg_rmrr *a)
+{
+    return require_rvv(s) &&
+           require_rvf(s) &&
+           require_scale_rvf(s) &&
+           vext_check_isa_ill(s) &&
+           vext_check_dss(s, a->rd, a->rs1, a->rs2, a->vm) &&
+           !is_overlapped(a->rd, 1 << MAX(s->lmul+1, 0), a->rs1, 1 << MAX(s->lmul, 0)) &&
+           !is_overlapped(a->rd, 1 << MAX(s->lmul+1, 0), a->rs2, 1 << MAX(s->lmul, 0));
+}
+
 /* OPFVV with WIDEN */
 #define GEN_OPFVV_WIDEN_TRANS(NAME, CHECK)                       \
 static bool trans_##NAME(DisasContext *s, arg_rmrr *a)           \
@@ -2409,6 +2440,16 @@ static bool opfvf_widen_check(DisasContext *s, arg_rmrr *a)
            vext_check_ds(s, a->rd, a->rs2, a->vm);
 }
 
+static bool opfvf_overwrite_widen_check(DisasContext *s, arg_rmrr *a)
+{
+    return require_rvv(s) &&
+           require_rvf(s) &&
+           require_scale_rvf(s) &&
+           vext_check_isa_ill(s) &&
+           vext_check_ds(s, a->rd, a->rs2, a->vm) &&
+           !is_overlapped(a->rd, 1 << MAX(s->lmul+1, 0), a->rs2, 1 << MAX(s->lmul, 0));
+}
+
 /* OPFVF with WIDEN */
 #define GEN_OPFVF_WIDEN_TRANS(NAME, CHECK)                       \
 static bool trans_##NAME(DisasContext *s, arg_rmrr *a)           \
@@ -2533,14 +2574,14 @@ GEN_OPFVF_TRANS(vfmsub_vf, opfvf_check)
 GEN_OPFVF_TRANS(vfnmsub_vf, opfvf_check)
 
 /* Vector Widening Floating-Point Fused Multiply-Add Instructions */
-GEN_OPFVV_WIDEN_TRANS(vfwmacc_vv, opfvv_widen_check)
-GEN_OPFVV_WIDEN_TRANS(vfwnmacc_vv, opfvv_widen_check)
-GEN_OPFVV_WIDEN_TRANS(vfwmsac_vv, opfvv_widen_check)
-GEN_OPFVV_WIDEN_TRANS(vfwnmsac_vv, opfvv_widen_check)
-GEN_OPFVF_WIDEN_TRANS(vfwmacc_vf, opfvf_widen_check)
-GEN_OPFVF_WIDEN_TRANS(vfwnmacc_vf, opfvf_widen_check)
-GEN_OPFVF_WIDEN_TRANS(vfwmsac_vf, opfvf_widen_check)
-GEN_OPFVF_WIDEN_TRANS(vfwnmsac_vf, opfvf_widen_check)
+GEN_OPFVV_WIDEN_TRANS(vfwmacc_vv, opfvv_overwrite_widen_check)
+GEN_OPFVV_WIDEN_TRANS(vfwnmacc_vv, opfvv_overwrite_widen_check)
+GEN_OPFVV_WIDEN_TRANS(vfwmsac_vv, opfvv_overwrite_widen_check)
+GEN_OPFVV_WIDEN_TRANS(vfwnmsac_vv, opfvv_overwrite_widen_check)
+GEN_OPFVF_WIDEN_TRANS(vfwmacc_vf, opfvf_overwrite_widen_check)
+GEN_OPFVF_WIDEN_TRANS(vfwnmacc_vf, opfvf_overwrite_widen_check)
+GEN_OPFVF_WIDEN_TRANS(vfwmsac_vf, opfvf_overwrite_widen_check)
+GEN_OPFVF_WIDEN_TRANS(vfwnmsac_vf, opfvf_overwrite_widen_check)
 
 /* Vector Floating-Point Square-Root Instruction */
 
-- 
2.34.1

Re: [PATCH 00/12] target/riscv: Fix some RISC-V instruction corner cases

[Max Chou]

Hi Anton,

I hope you’re doing well.

While reviewing this patchset, I noticed a few missing parts related to 
the mismatched input EEWs encoding constraint.
I also found a few other rvv encoding issues and planned to submit an 
upstream patchset to address them.
However, I think it would be better to merge these fixes into this 
patchset to maintain the series’ cohesion and keep up to date.

If you agree with this approach, I can integrate the fixes and submit a 
v2 of the patchset.
Please let me know your thoughts, and we can discuss the details further.

Thanks,
Max

On 2025/1/26 3:20 PM, Anton Blanchard wrote:
> This series fixes some RISC-V instruction corner cases, specifically
> illegal overlaps between mask and source registers, illegal overlaps
> between source registers and illegal overlaps between source and
> destination registers. These were found by looking at miscompares
> between QEMU and the Tenstorrent fork of Whisper which models this
> behaviour better than Spike and Sail.
>
> Anton Blanchard (12):
>    target/riscv: Source vector registers cannot overlap mask register
>    target/riscv: handle vrgather mask and source overlap
>    target/riscv: handle vadd.vx form mask and source overlap
>    target/riscv: handle vadd.vv form mask and source overlap
>    target/riscv: handle vslide1down.vx form mask and source overlap
>    target/riscv: handle vzext.vf2 form mask and source overlap
>    target/riscv: handle vwadd.vx form mask and source overlap
>    target/riscv: handle vwadd.vv form mask and source overlap
>    target/riscv: handle vwadd.wv form mask and source overlap
>    target/riscv: handle vwadd.wv form vs1 and vs2 overlap
>    target/riscv: Add CHECK arg to GEN_OPFVF_WIDEN_TRANS
>    target/riscv: handle overlap in widening instructions with overwrite
>
>   target/riscv/insn_trans/trans_rvv.c.inc | 139 ++++++++++++++++++------
>   1 file changed, 108 insertions(+), 31 deletions(-)
>

Re: [CAUTION - External Sender] Re: [PATCH 00/12] target/riscv: Fix some RISC-V instruction corner cases

[Anton Blanchard]

Hi Max,

On Fri, Feb 28, 2025 at 1:47 AM Max Chou <max.chou@sifive.com> wrote:
> While reviewing this patchset, I noticed a few missing parts related to
> the mismatched input EEWs encoding constraint.
> I also found a few other rvv encoding issues and planned to submit an
> upstream patchset to address them.
> However, I think it would be better to merge these fixes into this
> patchset to maintain the series’ cohesion and keep up to date.
>
> If you agree with this approach, I can integrate the fixes and submit a
> v2 of the patchset.
> Please let me know your thoughts, and we can discuss the details further.

Sorry for not getting back to you earlier but that would be great.

Thanks,
Anton