[PATCH v2 03/12] target/riscv: Add vext_check_input_eew to check mismatched input EEWs encoding constraint

Max Chou posted 12 patches 10 months, 2 weeks ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
There is a newer version of this series
[PATCH v2 03/12] target/riscv: Add vext_check_input_eew to check mismatched input EEWs encoding constraint
Posted by Max Chou 10 months, 2 weeks ago
According to the v spec, a vector register cannot be used to provide source
operands with more than one EEW for a single instruction.

Signed-off-by: Max Chou <max.chou@sifive.com>
---
 target/riscv/insn_trans/trans_rvv.c.inc | 29 +++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index e630f8661e1..70c19c49ae4 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -379,6 +379,35 @@ static bool vext_check_ld_index(DisasContext *s, int vd, int vs2,
     return ret;
 }
 
+/*
+ * Check whether a vector register is used to provide source operands with
+ * more than one EEW for the vector instruction.
+ * Returns true if the instruction has valid encoding
+ * Returns false if encoding violates the mismatched input EEWs constraint
+ */
+static bool vext_check_input_eew(DisasContext *s, int vs1, uint8_t eew_vs1,
+                                 int vs2, uint8_t eew_vs2, int vm)
+{
+    bool is_valid = true;
+    int8_t emul_vs1 = eew_vs1 - s->sew + s->lmul;
+    int8_t emul_vs2 = eew_vs2 - s->sew + s->lmul;
+
+    /* When vm is 0, vs1 & vs2(EEW!=1) group can't overlap v0 (EEW=1) */
+    if ((vs1 != -1 && !require_vm(vm, vs1)) ||
+        (vs2 != -1 && !require_vm(vm, vs2))) {
+        is_valid = false;
+    }
+
+    /* When eew_vs1 != eew_vs2, check whether vs1 and vs2 are overlapped */
+    if ((vs1 != -1 && vs2 != -1) && (eew_vs1 != eew_vs2) &&
+        is_overlapped(vs1, 1 << MAX(emul_vs1, 0),
+                      vs2, 1 << MAX(emul_vs2, 0))) {
+        is_valid = false;
+    }
+
+    return is_valid;
+}
+
 static bool vext_check_ss(DisasContext *s, int vd, int vs, int vm)
 {
     return require_vm(vm, vd) &&
-- 
2.43.0
Re: [PATCH v2 03/12] target/riscv: Add vext_check_input_eew to check mismatched input EEWs encoding constraint
Posted by Daniel Henrique Barboza 10 months, 1 week ago

On 3/29/25 11:44 AM, Max Chou wrote:
> According to the v spec, a vector register cannot be used to provide source
> operands with more than one EEW for a single instruction.
> 
> Signed-off-by: Max Chou <max.chou@sifive.com>
> ---
>   target/riscv/insn_trans/trans_rvv.c.inc | 29 +++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)
> 
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
> index e630f8661e1..70c19c49ae4 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -379,6 +379,35 @@ static bool vext_check_ld_index(DisasContext *s, int vd, int vs2,
>       return ret;
>   }
>   
> +/*
> + * Check whether a vector register is used to provide source operands with
> + * more than one EEW for the vector instruction.
> + * Returns true if the instruction has valid encoding
> + * Returns false if encoding violates the mismatched input EEWs constraint
> + */
> +static bool vext_check_input_eew(DisasContext *s, int vs1, uint8_t eew_vs1,
> +                                 int vs2, uint8_t eew_vs2, int vm)
> +{
> +    bool is_valid = true;
> +    int8_t emul_vs1 = eew_vs1 - s->sew + s->lmul;
> +    int8_t emul_vs2 = eew_vs2 - s->sew + s->lmul;
> +
> +    /* When vm is 0, vs1 & vs2(EEW!=1) group can't overlap v0 (EEW=1) */
> +    if ((vs1 != -1 && !require_vm(vm, vs1)) ||
> +        (vs2 != -1 && !require_vm(vm, vs2))) {
> +        is_valid = false;
> +    }
> +
> +    /* When eew_vs1 != eew_vs2, check whether vs1 and vs2 are overlapped */
> +    if ((vs1 != -1 && vs2 != -1) && (eew_vs1 != eew_vs2) &&
> +        is_overlapped(vs1, 1 << MAX(emul_vs1, 0),
> +                      vs2, 1 << MAX(emul_vs2, 0))) {
> +        is_valid = false;
> +    }
> +
> +    return is_valid;
> +}
> +

Code LGTM but the patch won't compile on its own because there's no callers for
it:


In file included from ../target/riscv/translate.c:1182:
../target/riscv/insn_trans/trans_rvv.c.inc:388:13: error: ‘vext_check_input_eew’ defined but not used [-Werror=unused-function]
   388 | static bool vext_check_input_eew(DisasContext *s, int vs1, uint8_t eew_vs1,
       |             ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.


We want each patch to be "buildable" and with test passing to make our lives easier
when doing bisects.

You can merge this patch with patch 4 to introduce the new function and add its first
callers. Thanks,


Daniel



>   static bool vext_check_ss(DisasContext *s, int vd, int vs, int vm)
>   {
>       return require_vm(vm, vd) &&


Re: [PATCH v2 03/12] target/riscv: Add vext_check_input_eew to check mismatched input EEWs encoding constraint
Posted by Max Chou 10 months, 1 week ago
On 2025/4/5 5:09 PM, Daniel Henrique Barboza wrote:
>
>
> On 3/29/25 11:44 AM, Max Chou wrote:
>> According to the v spec, a vector register cannot be used to provide 
>> source
>> operands with more than one EEW for a single instruction.
>>
>> Signed-off-by: Max Chou <max.chou@sifive.com>
>> ---
>>   target/riscv/insn_trans/trans_rvv.c.inc | 29 +++++++++++++++++++++++++
>>   1 file changed, 29 insertions(+)
>>
>> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
>> b/target/riscv/insn_trans/trans_rvv.c.inc
>> index e630f8661e1..70c19c49ae4 100644
>> --- a/target/riscv/insn_trans/trans_rvv.c.inc
>> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
>> @@ -379,6 +379,35 @@ static bool vext_check_ld_index(DisasContext *s, 
>> int vd, int vs2,
>>       return ret;
>>   }
>>   +/*
>> + * Check whether a vector register is used to provide source 
>> operands with
>> + * more than one EEW for the vector instruction.
>> + * Returns true if the instruction has valid encoding
>> + * Returns false if encoding violates the mismatched input EEWs 
>> constraint
>> + */
>> +static bool vext_check_input_eew(DisasContext *s, int vs1, uint8_t 
>> eew_vs1,
>> +                                 int vs2, uint8_t eew_vs2, int vm)
>> +{
>> +    bool is_valid = true;
>> +    int8_t emul_vs1 = eew_vs1 - s->sew + s->lmul;
>> +    int8_t emul_vs2 = eew_vs2 - s->sew + s->lmul;
>> +
>> +    /* When vm is 0, vs1 & vs2(EEW!=1) group can't overlap v0 
>> (EEW=1) */
>> +    if ((vs1 != -1 && !require_vm(vm, vs1)) ||
>> +        (vs2 != -1 && !require_vm(vm, vs2))) {
>> +        is_valid = false;
>> +    }
>> +
>> +    /* When eew_vs1 != eew_vs2, check whether vs1 and vs2 are 
>> overlapped */
>> +    if ((vs1 != -1 && vs2 != -1) && (eew_vs1 != eew_vs2) &&
>> +        is_overlapped(vs1, 1 << MAX(emul_vs1, 0),
>> +                      vs2, 1 << MAX(emul_vs2, 0))) {
>> +        is_valid = false;
>> +    }
>> +
>> +    return is_valid;
>> +}
>> +
>
> Code LGTM but the patch won't compile on its own because there's no 
> callers for
> it:
>
>
> In file included from ../target/riscv/translate.c:1182:
> ../target/riscv/insn_trans/trans_rvv.c.inc:388:13: error: 
> ‘vext_check_input_eew’ defined but not used [-Werror=unused-function]
>   388 | static bool vext_check_input_eew(DisasContext *s, int vs1, 
> uint8_t eew_vs1,
>       |             ^~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> ninja: build stopped: subcommand failed.
>
>
> We want each patch to be "buildable" and with test passing to make our 
> lives easier
> when doing bisects.
>
> You can merge this patch with patch 4 to introduce the new function 
> and add its first
> callers. Thanks,
>
>
> Daniel
Thanks for the suggestion. Will fix this issue at v3.

Max
>
>
>
>>   static bool vext_check_ss(DisasContext *s, int vd, int vs, int vm)
>>   {
>>       return require_vm(vm, vd) &&
>