From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:60995)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1XmhgX-00060U-8i
	for qemu-devel@nongnu.org; Fri, 07 Nov 2014 06:26:38 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1XmhgR-0002la-KQ
	for qemu-devel@nongnu.org; Fri, 07 Nov 2014 06:26:33 -0500
Received: from mail-wi0-x22a.google.com ([2a00:1450:400c:c05::22a]:52702)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1XmhgR-0002lN-C9
	for qemu-devel@nongnu.org; Fri, 07 Nov 2014 06:26:27 -0500
Received: by mail-wi0-f170.google.com with SMTP id r20so4056947wiv.5
	for <qemu-devel@nongnu.org>; Fri, 07 Nov 2014 03:26:26 -0800 (PST)
Sender: Richard Henderson <rth7680@gmail.com>
Message-ID: <545CAC5E.2030607@twiddle.net>
Date: Fri, 07 Nov 2014 12:26:22 +0100
From: Richard Henderson <rth@twiddle.net>
MIME-Version: 1.0
References: <36D1AB4E1AAC4541A1C88167C7231D0B74BAAD@G08CNEXMBPEKD02.g08.fujitsu.local>
In-Reply-To: <36D1AB4E1AAC4541A1C88167C7231D0B74BAAD@G08CNEXMBPEKD02.g08.fujitsu.local>
Content-Type: text/plain; charset=gbk
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] Add CMP2 instruction
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Guo, Lei" <guol-fnst@cn.fujitsu.com>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Cc: "Wang, Chunping" <wangcp@cn.fujitsu.com>

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~