Re: [PATCH v1 2/6] target/loongarch: Add set_vec_extctx to set LSX/LASX instructions extctx_flags

[gaosong]

在 2023/10/30 下午11:30, Richard Henderson 写道:
> On 10/29/23 20:28, gaosong wrote:
> > 在 2023/10/29 上午5:40, Richard Henderson 写道:
> > > On 10/9/23 20:36, Song Gao wrote:
> > > > Signed-off-by: Song Gao <gaosong@loongson.cn>
> > > > ---
> > > >   target/loongarch/insn_trans/trans_vec.c.inc | 12 ++++++++++++
> > > >   target/loongarch/internals.h                |  2 ++
> > > >   2 files changed, 14 insertions(+)
> > > >
> > > > diff --git a/target/loongarch/insn_trans/trans_vec.c.inc 
> > > > b/target/loongarch/insn_trans/trans_vec.c.inc
> > > > index 98f856bb29..aef16ef44a 100644
> > > > --- a/target/loongarch/insn_trans/trans_vec.c.inc
> > > > +++ b/target/loongarch/insn_trans/trans_vec.c.inc
> > > > @@ -23,8 +23,20 @@ static bool check_vec(DisasContext *ctx, 
> > > > uint32_t oprsz)
> > > >     #else
> > > >   +static void set_vec_extctx(DisasContext *ctx, uint32_t oprsz)
> > > > +{
> > > > +    if (oprsz == 16) {
> > > > +        ctx->extctx_flags |= EXTCTX_FLAGS_LSX;
> > > > +    }
> > > > +
> > > > +    if (oprsz == 32) {
> > > > +        ctx->extctx_flags |= EXTCTX_FLAGS_LASX;
> > > > +    }
> > > > +}
> > > > +
> > > >   static bool check_vec(DisasContext *ctx, uint32_t oprsz)
> > > >   {
> > > > +    set_vec_extctx(ctx, oprsz);
> > > >       return true;
> > > >   }
> > >
> > > This doesn't do anything.  Nothing copies the changed value back to 
> > > env.
> > > Anyway, I think this is the wrong way to go about it.
> > Oh, It is on patch1.
> >
> > @@ -294,6 +296,7 @@ static void 
> > loongarch_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
> >           generate_exception(ctx, EXCCODE_INE);
> >       }
> >
> > +    env->extctx_flags |= ctx->extctx_flags;
>
> Ah, well, this is also incorrect.
>
> This copy only happens at translation time, not at execution time.
>
> Anyway, I think my previous suggestion is better:
Oh,  Could you  show more details?  I think I didn't get you point.

> > > If you want to track what the program is using, you should do it 
> > > exactly like the real kernel: disable the execution unit, have the 
> > > program trap, and the enable the execution unit when the trap 
> > > occurs. At this point, CSR_EUEN enable bits contain exactly which 
> > > units have been used by the program.
we always enabled LSX/LASX exception,  This is mean that we always use 
target_lasx_context.

Thanks.
Song Gao
> r~