Re: [Qemu-devel] [PATCH v2 1/3] target-m68k: add abcd/sbcd/nbcd

[Richard Henderson]

On 11/02/2016 03:15 PM, Laurent Vivier wrote:
> +static void bcd_sub(TCGv dest, TCGv src)
> +{
> +    TCGv t0, t1, t2;
> +
> +    /*  dest10 = dest10 - src10 - X
> +     *         = bcd_add(dest + 1 - X, 0xf99 - src)
> +     */
> +
> +    /* t0 = 0xfff - src */
> +
> +    t0 = tcg_temp_new();
> +    tcg_gen_neg_i32(t0, src);
> +    tcg_gen_addi_i32(t0, t0, 0xfff);

tcg_gen_subfi_i32(t0, 0xfff, src);

And how did we get 0xfff from 0xf99?  Also, see below...

> +
> +    /* t1 = t0 + dest + 1 - X*/
> +
> +    t1 = tcg_temp_new();
> +    tcg_gen_add_i32(t1, t0, dest);
> +    tcg_gen_addi_i32(t1, t1, 1);
> +    tcg_gen_sub_i32(t1, t1, QREG_CC_X);
> +
> +    /* t2 = t0 ^ dest ^ 1 ^ X */
> +
> +    t2 = tcg_temp_new();
> +    tcg_gen_xor_i32(t2, t0, dest);
> +    tcg_gen_xori_i32(t2, t2, 1);
> +    tcg_gen_xor_i32(t2, t2, QREG_CC_X);

Since you only care about bits 0x110, you can drop the ^ 1 ^ X; they won't 
affect the result.  (Similarly wrt bcd_add, dropping ^ X).

> +    /* t2 = ~t0 & 0x110 */
> +
> +    tcg_gen_not_i32(t2, t0);
> +    tcg_gen_andi_i32(t2, t2, 0x110);
> +
> +    /* t0 = (t2 >> 2) | (t2 >> 3) */
> +
> +    tcg_gen_shri_i32(t0, t2, 2);
> +    tcg_gen_shri_i32(t2, t2, 3);
> +    tcg_gen_or_i32(t0, t0, t2);

For the benefit of hosts that have 8-bit immediate AND insns (e.g. arm32), it 
would be better to rearrange this a little:

    t2 = t0 >> 3;
    t3 = ~t2 & 0x22;
    t4 = t3 + t3;
    t5 = t3 + t4;

(Similarly in bcd_add).

> +
> +    /* return t1 - t0 */
> +
> +    tcg_gen_sub_i32(dest, t1, t0);
> +}
> +
> +static void bcd_flags(TCGv val)
> +{
> +    tcg_gen_andi_i32(QREG_CC_C, val, 0x0ff);
> +    tcg_gen_or_i32(QREG_CC_Z, QREG_CC_Z, QREG_CC_C);
> +
> +    tcg_gen_movi_i32(QREG_CC_X, 0);
> +    tcg_gen_andi_i32(val, val, 0xf00);
> +    tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_C, val, QREG_CC_X);

Surely 0x100 is the carry.  I don't see how you could produce 0x2xx from 
addition.  For subtraction, I think we got things other than 0/1 in the third 
nibble simply because we started with 0xf99 instead of 0x199.


r~