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

[Daniel Henrique Barboza <dbarboza@ventanamicro.com>]

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) &&