On Fri, Mar 15, 2024 at 3:58 AM Daniel Henrique Barboza <dbarb...@ventanamicro.com> 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 <dbarb...@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> Alistair > --- > target/riscv/insn_trans/trans_rvv.c.inc | 3 --- > target/riscv/vector_helper.c | 5 +++++ > 2 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/target/riscv/insn_trans/trans_rvv.c.inc > b/target/riscv/insn_trans/trans_rvv.c.inc > index 8c16a9f5b3..52c26a7834 100644 > --- a/target/riscv/insn_trans/trans_rvv.c.inc > +++ b/target/riscv/insn_trans/trans_rvv.c.inc > @@ -3664,12 +3664,9 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME * > a) \ > vreg_ofs(s, a->rs2), maxsz, maxsz); \ > mark_vs_dirty(s); \ > } else { \ > - TCGLabel *over = gen_new_label(); \ > - tcg_gen_brcondi_tl(TCG_COND_GEU, cpu_vstart, maxsz, over); \ > tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \ > tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); > \ > mark_vs_dirty(s); \ > - gen_set_label(over); \ > } \ > return true; \ > } \ > diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c > index 34ac4aa808..bcc553c0e2 100644 > --- a/target/riscv/vector_helper.c > +++ b/target/riscv/vector_helper.c > @@ -5075,6 +5075,11 @@ void HELPER(vmvr_v)(void *vd, void *vs2, CPURISCVState > *env, uint32_t desc) > uint32_t startb = env->vstart * sewb; > uint32_t i = startb; > > + if (startb >= maxsz) { > + env->vstart = 0; > + return; > + } > + > if (HOST_BIG_ENDIAN && i % 8 != 0) { > uint32_t j = ROUND_UP(i, 8); > memcpy((uint8_t *)vd + H1(j - 1), > -- > 2.44.0 > >