Re: [PATCH 14/18] target/i386: move 60-BF opcodes to new decoder

[Richard Henderson <richard.henderson@linaro.org>]

On 10/14/23 03:01, Paolo Bonzini wrote:
> @@ -179,6 +180,9 @@
>  #define p_66_f3_f2    .valid_prefix = P_66 | P_F3 | P_F2,
>  #define p_00_66_f3_f2 .valid_prefix = P_00 | P_66 | P_F3 | P_F2,
>  
> +static X86OpEntry illegal_opcode =
> +    X86_OP_ENTRY0(illegal);

const.

> +static void gen_ARPL(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
> +{
> +    TCGLabel *label1 = gen_new_label();
> +    TCGv rpl_adj = tcg_temp_new();
> +
> +    gen_mov_eflags(s, s->tmp4);
> +    tcg_gen_andi_tl(s->tmp4, s->tmp4, ~CC_Z);
> +
> +    /* Compute dest[rpl] - src[rpl], adjust if result <0.  */
> +    tcg_gen_andi_tl(rpl_adj, s->T0, 3);
> +    tcg_gen_andi_tl(s->T1, s->T1, 3);
> +    tcg_gen_sub_tl(rpl_adj, rpl_adj, s->T1);
> +
> +    tcg_gen_brcondi_tl(TCG_COND_LT, rpl_adj, 0, label1);
> +
> +    /* Subtract dest[rpl] - src[rpl] to set dest[rpl] = src[rpl].  */
> +    tcg_gen_sub_tl(s->T0, s->T0, rpl_adj);
> +    tcg_gen_ori_tl(s->tmp4, s->tmp4, CC_Z);
> +    gen_set_label(label1);
> +
> +    decode->cc_src = s->tmp4;
> +    set_cc_op(s, CC_OP_EFLAGS);
> +}

If you're going to adjust the algorithm here,
you might as well make it branchless:

     tcg_gen_negsetcond_tl(TCG_COND_GE, mask, rpl_adj, tcg_constant_tl(0));
     tcg_gen_and_tl(rpl_adj, rpl_adj, mask);
     tcg_gen_sub_tl(t0, t0, rpl_adj);
     tcg_gen_andi_tl(mask, mask, CC_Z);
     tcg_gen_or_tl(tmp4, tmp4, mask);

> +static void gen_CBW(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
> +{
> +    switch(decode->op[0].ot) {
> +#ifdef TARGET_X86_64
> +        case MO_64:
> +            tcg_gen_ext32s_tl(s->T0, s->T0);
> +            break;
> +#endif
> +        case MO_32:
> +            tcg_gen_ext16s_tl(s->T0, s->T0);
> +            break;
> +        case MO_16:
> +            tcg_gen_ext8s_tl(s->T0, s->T0);
> +            break;
> +        default:
> +            abort();
> +    }
> +}

Reuse gen_ext_tl(t0, t0, (ot - 1) | MO_SIGN)?

> +static void gen_CWD(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
> +{
> +    int shift = 8 << decode->op[0].ot;
> +    switch (shift) {
> +    case 64:
> +        break;
> +    case 32:
> +        tcg_gen_ext32s_tl(s->T0, s->T0);
> +        break;
> +    case 16:
> +        tcg_gen_ext16s_tl(s->T0, s->T0);
> +        break;
> +    default:
> +        abort();
> +    }
> +    tcg_gen_sari_tl(s->T0, s->T0, shift - 1);

Better as

   tcg_gen_sextract_tl(t0, t0, shift - 1, 1)

to just extract and replicate the one bit you wanted.


r~