Re: [PATCH 10/13] target/riscv: Adjust scalar reg in vector with ol

[LIU Zhiwei]

On 2021/11/2 上午12:33, Richard Henderson wrote:
> On 11/1/21 6:01 AM, LIU Zhiwei wrote:
> > @@ -2677,6 +2677,7 @@ static bool trans_vmv_s_x(DisasContext *s, 
> > arg_vmv_s_x *a)
> >           /* This instruction ignores LMUL and vector register groups */
> >           int maxsz = s->vlen >> 3;
> >           TCGv_i64 t1;
> > +        TCGv src1 = get_gpr(s, a->rs1, EXT_ZERO);
> >           TCGLabel *over = gen_new_label();
> >             tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
> > @@ -2686,7 +2687,7 @@ static bool trans_vmv_s_x(DisasContext *s, 
> > arg_vmv_s_x *a)
> >           }
> >             t1 = tcg_temp_new_i64();
> > -        tcg_gen_extu_tl_i64(t1, cpu_gpr[a->rs1]);
> > +        tcg_gen_extu_tl_i64(t1, src1);
> >           vec_element_storei(s, a->rd, 0, t1);
> >           tcg_temp_free_i64(t1);
> >       done:
>
> This isn't actually correct.  Or, may have been correct for the 0.7.1 
> revision,
Yes.
> but the rvv 1.0 revision has a sign-extend here.
>
> This probably shouldn't be touched until the rvv 1.0 patch set comes in.

I try to make all sub extensions upstream are right.

Or we have to make an assert that the uxl32 is not ready for RVV.

> > diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> > index 451688c328..5bdbbf7c71 100644
> > --- a/target/riscv/vector_helper.c
> > +++ b/target/riscv/vector_helper.c
> > @@ -4763,6 +4763,7 @@ void HELPER(NAME)(void *vd, void *v0, 
> > target_ulong s1, void *vs2,         \
> >       uint32_t mlen = 
> > vext_mlen(desc);                                      \
> >       uint32_t vlmax = env_archcpu(env)->cfg.vlen / 
> > mlen;                   \
> >       uint32_t vm = 
> > vext_vm(desc);                                          \
> > +    uint32_t olen = 16 << 
> > vext_ol(desc);                                  \
> >       uint32_t vl = 
> > env->vl;                                                \
> >       uint32_t 
> > i;                                                           \
> > \
> > @@ -4771,7 +4772,7 @@ void HELPER(NAME)(void *vd, void *v0, 
> > target_ulong s1, void *vs2,         \
> > continue;                                                     \
> > } \
> >           if (i == 0) 
> > {                                                     \
> > -            *((ETYPE *)vd + H(i)) = 
> > s1;                                   \
> > +            *((ETYPE *)vd + H(i)) = adjust_addr(s1, 
> > olen);                \
> >           } else 
> > {                                                          \
> >               *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i - 
> > 1));           \
> > } \
> > @@ -4792,6 +4793,7 @@ void HELPER(NAME)(void *vd, void *v0, 
> > target_ulong s1, void *vs2,         \
> >       uint32_t mlen = 
> > vext_mlen(desc);                                      \
> >       uint32_t vlmax = env_archcpu(env)->cfg.vlen / 
> > mlen;                   \
> >       uint32_t vm = 
> > vext_vm(desc);                                          \
> > +    uint32_t olen = 16 << 
> > vext_ol(desc);                                  \
> >       uint32_t vl = 
> > env->vl;                                                \
> >       uint32_t 
> > i;                                                           \
> > \
> > @@ -4800,7 +4802,7 @@ void HELPER(NAME)(void *vd, void *v0, 
> > target_ulong s1, void *vs2,         \
> > continue;                                                     \
> > } \
> >           if (i == vl - 1) 
> > {                                                \
> > -            *((ETYPE *)vd + H(i)) = 
> > s1;                                   \
> > +            *((ETYPE *)vd + H(i)) = adjust_addr(s1, 
> > olen);                \
> >           } else 
> > {                                                          \
> >               *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i + 
> > 1));           \
> > } \
>
> What in the world is this?  S1 is not an address, it's just a value 
> from X[RS1].

It's name is bad. Maybe I should just s1 & UINT32_MAX.

Zhiwei

> r~