Re: [PATCH 2/2] target/mips: Correct check for CABS instructions

[Jiaxun Yang]

在 2022/11/7 22:35, Philippe Mathieu-Daudé 写道:
> On 2/11/22 17:57, Jiaxun Yang wrote:
> > Accroading to "MIPS Architecture for Programmers Volume IV-c:
> > The MIPS-3D Application-Specific Extension to the MIPS64 Architecture"
> > (MD00099). CABS.cond.fmt belongs to MIPS-3D ASE, and it has nothing 
> > to do
> > with COP1X opcode.
> >
> > Remove all unnecessary COP1X checks and check for MIPS3D availability
> > in decoding code path.
> >
> > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > ---
> >   target/mips/tcg/translate.c | 9 +--------
> >   1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
> > index e49d2a25a8..23e575ad95 100644
> > --- a/target/mips/tcg/translate.c
> > +++ b/target/mips/tcg/translate.c
> > @@ -1788,16 +1788,8 @@ static inline void gen_cmp ## type ## _ ## 
> > fmt(DisasContext *ctx, int n,      \
> > check_ps(ctx); \
> > break; \
> >       case FMT_D: \
> > -        if (abs) 
> > {                                                            \
> > - check_cop1x(ctx); \
> > - } \
> >           check_cp1_registers(ctx, fs | 
> > ft);                                    \
> > break; \
> > -    case FMT_S: \
> > -        if (abs) 
> > {                                                            \
> > - check_cop1x(ctx); \
> > - } \
> > - break; \
>
> I'm not sure we want to remove this check on all opcodes handled by
> the FOP_CONDS() macro, and for all architecture variants. Maybe we
> need to special-case CABS.cond.fmt?

Hmm if I read the code correctly COP1X check is only ran when abs is
set, and the only case abs is set is for CABS.cond.fmt.

> > } \
> >       gen_ldcmp_fpr##bits(ctx, fp0, 
> > fs);                                        \
> >       gen_ldcmp_fpr##bits(ctx, fp1, 
> > ft);                                        \
> > @@ -10424,6 +10416,7 @@ static void gen_farith(DisasContext *ctx, 
> > enum fopcode op1,
> >       case OPC_CMP_NGT_S:
> >           check_insn_opc_removed(ctx, ISA_MIPS_R6);
> >           if (ctx->opcode & (1 << 6)) {
> > +            check_insn(ctx, ASE_MIPS3D);
>
> You somehow revert commit b8aa4598e2 ("MIPS COP1X (and related)
> instructions") which is in use since 15 years.

Still no idea about why it is here in first place....
CABS.cond.fmt is even not mentioned in MIPS IV manual. So it's unlikely 
to have
anything to do with COP1X.

Thanks
- Jiaxun

> >               gen_cmpabs_s(ctx, func - 48, ft, fs, cc);
> >           } else {
> >               gen_cmp_s(ctx, func - 48, ft, fs, cc);