Re: [PATCH for 9.0 v15 04/10] target/riscv: always clear vstart in whole vec move insns

[Richard Henderson <richard.henderson@linaro.org>]

On 3/14/24 07:56, Daniel Henrique Barboza wrote:
> These insns have 2 paths: we'll either have vstart already cleared if
> vstart_eq_zero or we'll do a brcond to check if vstart >= maxsz to call
> the 'vmvr_v' helper. The helper will clear vstart if it executes until
> the end, or if vstart >= vl.
> 
> For starters, the check itself is wrong: we're checking vstart >= maxsz,
> when in fact we should use vstart in bytes, or 'startb' like 'vmvr_v' is
> calling, to do the comparison. But even after fixing the comparison we'll
> still need to clear vstart in the end, which isn't happening too.
> 
> We want to make the helpers responsible to manage vstart, including
> these corner cases, precisely to avoid these situations:
> 
> - remove the wrong vstart >= maxsz cond from the translation;
> - add a 'startb >= maxsz' cond in 'vmvr_v', and clear vstart if that
>    happens.
> 
> This way we're now sure that vstart is being cleared in the end of the
> execution, regardless of the path taken.
> 
> Fixes: f714361ed7 ("target/riscv: rvv-1.0: implement vstart CSR")
> Signed-off-by: Daniel Henrique Barboza<dbarboza@ventanamicro.com>
> ---
>   target/riscv/insn_trans/trans_rvv.c.inc | 3 ---
>   target/riscv/vector_helper.c            | 5 +++++
>   2 files changed, 5 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~