Re: [Qemu-devel] Add CMP2 instruction

[Richard Henderson <rth@twiddle.net>]

On 11/07/2014 11:14 AM, Guo, Lei wrote:
> This patch aims to add CMP2 instruction for m68k family.

Mainline target-m68k supports coldfire only.

There is an external tree for full m68k support:

   https://gitorious.org/qemu-m68k

That said, before you send this to them...

> +    if (ext & 0x8000) {
> +           reg = AREG(ext, 12);

Failure to sign-extend for opsize == OS_WORD.
You need to use signed comparisons for this case.

> +    } else {
> +           reg = DREG(ext, 12);
> +           if (opsize == OS_BYTE){
> +                    tcg_gen_andi_i32(reg, reg, 0xf);
> +           } else if (opsize == OS_WORD) {
> +                          tcg_gen_andi_i32(reg, reg, 0xff);
> +           }
> +    }

Incorrect zero-extension; you've messed up the constants.
Use tcg_gen_ext{8,16}u_i32, anyway.
You need to use unsigned comparisons for this case.

> +    l1 = gen_new_label();
> +    l2 = gen_new_label();
> +    l3 = gen_new_label();
> +    l4 = gen_new_label();
> +   
> +    tcg_gen_brcond_i32(TCG_COND_NE, reg, lower, l1);

Ew.  You'd be much better off doing this with setcond than brcond.

  gen_flush_flags(s);

  t1 = tcg_temp_new();
  t2 = tcg_temp_new();
  t3 = tcg_temp_new();
  t4 = tcg_temp_new();

  tcg_gen_setcond_i32(TCG_COND_EQ, t1, reg, upper);
  tcg_gen_setcond_i32(TCG_COND_EQ, t2, reg, lower);
  tcg_gen_setcond_i32(which_gt, t3, reg, upper);
  tcg_gen_setcond_i32(which_lt, t4, reg, lower);

  tcg_gen_or_i32(t1, t1, t2);		/* equal to either bound */
  tcg_gen_or_i32(t3, t3, t4);		/* out of bounds */

  tcg_gen_shl_i32(t1, t1, ctz32(CCF_Z));	/* shift Z into place */
  tcg_gen_shl_i32(t3, t3, ctz32(CCF_C));	/* shift C into place */
  tcg_gen_or_i32(t1, t1, t3);
  tcg_gen_andi_i32(QREG_CC_DEST, QREG_CC_DEST, ~(CCF_C | CCF_Z));
  tcg_gen_or_i32(QREG_CC_DEST, QREG_CC_DEST, t1);


r~