Re: [PATCH 05/17] target/i386: add 00-07, 10-17 opcodes

[Richard Henderson]

On 8/24/22 10:31, Paolo Bonzini wrote:
> +static void gen_alu_op(DisasContext *s1, int op, MemOp ot)
> +{
> +    switch(op) {
> +    case OP_ADCL:
> +        gen_compute_eflags_c(s1, s1->tmp4);
> +        if (s1->prefix & PREFIX_LOCK) {
> +            tcg_gen_add_tl(s1->T0, s1->tmp4, s1->T1);
> +            tcg_gen_atomic_add_fetch_tl(s1->T0, s1->A0, s1->T0,
> +                                        s1->mem_index, ot | MO_LE);
> +        } else {
> +            tcg_gen_add_tl(s1->T0, s1->T0, s1->T1);
> +            tcg_gen_add_tl(s1->T0, s1->T0, s1->tmp4);
> +        }
> +        gen_op_update3_cc(s1, s1->tmp4);
> +        set_cc_op(s1, CC_OP_ADCB + ot);
> +        break;
> +    case OP_SBBL:
> +        gen_compute_eflags_c(s1, s1->tmp4);
> +        if (s1->prefix & PREFIX_LOCK) {
> +            tcg_gen_add_tl(s1->T0, s1->T1, s1->tmp4);
> +            tcg_gen_neg_tl(s1->T0, s1->T0);
> +            tcg_gen_atomic_add_fetch_tl(s1->T0, s1->A0, s1->T0,
> +                                        s1->mem_index, ot | MO_LE);
> +        } else {
> +            tcg_gen_sub_tl(s1->T0, s1->T0, s1->T1);
> +            tcg_gen_sub_tl(s1->T0, s1->T0, s1->tmp4);
> +        }
> +        gen_op_update3_cc(s1, s1->tmp4);
> +        set_cc_op(s1, CC_OP_SBBB + ot);
> +        break;
> +    case OP_ADDL:
> +        if (s1->prefix & PREFIX_LOCK) {
> +            tcg_gen_atomic_add_fetch_tl(s1->T0, s1->A0, s1->T1,
> +                                        s1->mem_index, ot | MO_LE);
> +        } else {
> +            tcg_gen_add_tl(s1->T0, s1->T0, s1->T1);
> +        }
> +        gen_op_update2_cc(s1);
> +        set_cc_op(s1, CC_OP_ADDB + ot);
> +        break;
> +    case OP_SUBL:
> +        if (s1->prefix & PREFIX_LOCK) {
> +            tcg_gen_neg_tl(s1->T0, s1->T1);
> +            tcg_gen_atomic_fetch_add_tl(s1->cc_srcT, s1->A0, s1->T0,
> +                                        s1->mem_index, ot | MO_LE);
> +            tcg_gen_sub_tl(s1->T0, s1->cc_srcT, s1->T1);
> +        } else {
> +            tcg_gen_mov_tl(s1->cc_srcT, s1->T0);
> +            tcg_gen_sub_tl(s1->T0, s1->T0, s1->T1);
> +        }
> +        gen_op_update2_cc(s1);
> +        set_cc_op(s1, CC_OP_SUBB + ot);
> +        break;
> +    default:
> +    case OP_ANDL:
> +        if (s1->prefix & PREFIX_LOCK) {
> +            tcg_gen_atomic_and_fetch_tl(s1->T0, s1->A0, s1->T1,
> +                                        s1->mem_index, ot | MO_LE);
> +        } else {
> +            tcg_gen_and_tl(s1->T0, s1->T0, s1->T1);
> +        }
> +        gen_op_update1_cc(s1);
> +        set_cc_op(s1, CC_OP_LOGICB + ot);
> +        break;
> +    case OP_ORL:
> +        if (s1->prefix & PREFIX_LOCK) {
> +            tcg_gen_atomic_or_fetch_tl(s1->T0, s1->A0, s1->T1,
> +                                       s1->mem_index, ot | MO_LE);
> +        } else {
> +            tcg_gen_or_tl(s1->T0, s1->T0, s1->T1);
> +        }
> +        gen_op_update1_cc(s1);
> +        set_cc_op(s1, CC_OP_LOGICB + ot);
> +        break;
> +    case OP_XORL:
> +        if (s1->prefix & PREFIX_LOCK) {
> +            tcg_gen_atomic_xor_fetch_tl(s1->T0, s1->A0, s1->T1,
> +                                        s1->mem_index, ot | MO_LE);
> +        } else {
> +            tcg_gen_xor_tl(s1->T0, s1->T0, s1->T1);
> +        }
> +        gen_op_update1_cc(s1);
> +        set_cc_op(s1, CC_OP_LOGICB + ot);
> +        break;
> +    }
> +}

If you were sharing this with the old decoder, and thus needed to retain it, that would be 
one thing.  But there's no shared prefix or postfix in this function, so you might as well

> +
> +static void gen_ADC(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
> +{
> +    gen_alu_op(s, OP_ADCL, decode->op[0].ot);
> +}

... split each case into ...

> +
> +static void gen_ADD(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
> +{
> +    gen_alu_op(s, OP_ADDL, decode->op[0].ot);
> +}

... these functions.



r~