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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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?

>       }                                                                         \
>       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.

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