Hi,

On Tue, Apr 15, 2025 at 8:25 PM Philippe Mathieu-Daudé
<phi...@linaro.org> wrote:
>
> Hi,
>
> On 15/4/25 14:06, WANG Rui wrote:
> > Previously, some instructions could be executed regardless of CPU mode or
> > feature support. This patch enforces proper checks so that instructions are
> > only allowed when the required CPU features are enabled.
> >
> > Signed-off-by: WANG Rui <wang...@loongson.cn>
> > ---
> >   target/loongarch/cpu.c                        |  4 +--
> >   target/loongarch/cpu.h                        |  2 +-
> >   .../tcg/insn_trans/trans_atomic.c.inc         | 36 +++++++++----------
> >   .../tcg/insn_trans/trans_branch.c.inc         |  4 +--
> >   .../tcg/insn_trans/trans_extra.c.inc          | 20 ++++++-----
> >   .../tcg/insn_trans/trans_privileged.c.inc     |  4 +--
> >   .../tcg/insn_trans/trans_shift.c.inc          |  4 +--
> >   .../loongarch/tcg/insn_trans/trans_vec.c.inc  | 16 ++++-----
> >   target/loongarch/translate.h                  |  6 ++++
> >   9 files changed, 53 insertions(+), 43 deletions(-)
>
>
> > diff --git a/target/loongarch/translate.h b/target/loongarch/translate.h
> > index 195f53573a..cf6eecd7ab 100644
> > --- a/target/loongarch/translate.h
> > +++ b/target/loongarch/translate.h
> > @@ -14,6 +14,11 @@
> >       static bool trans_##NAME(DisasContext *ctx, arg_##NAME * a) \
> >       { return avail_##AVAIL(ctx) && FUNC(ctx, a, __VA_ARGS__); }
> >
> > +#define TRANS2(NAME, AVAIL1, AVAIL2, FUNC, ...) \
> > +    static bool trans_##NAME(DisasContext *ctx, arg_##NAME * a) \
> > +    { return avail_##AVAIL1(ctx) && avail_##AVAIL2(ctx) && \
> > +             FUNC(ctx, a, __VA_ARGS__); }
> > +
>
> What about simply:
>
>    #define TRANS64(NAME, AVAIL, FUNC, ...) \
>        static bool trans_##NAME(DisasContext *ctx, arg_##NAME * a) \
>        { return avail_64(ctx) \
>                 && avail_##AVAIL(ctx) \
>                 && FUNC(ctx, a, __VA_ARGS__); \
>        }
>
> ?

For the current case, I believe TRANS64 is a great option. I
introduced TRANS2 mainly for added flexibility -- it allows combining
any two feature checks. I'm happy to send a v3 if you'd prefer that.

>
> >   #define avail_ALL(C)   true
> >   #define avail_64(C)    (FIELD_EX32((C)->cpucfg1, CPUCFG1, ARCH) == \
> >                           CPUCFG1_ARCH_LA64)


Reply via email to