From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42685)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <James.Hogan@imgtec.com>) id 1XXUt1-0005wv-TE
	for qemu-devel@nongnu.org; Fri, 26 Sep 2014 08:44:40 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <James.Hogan@imgtec.com>) id 1XXUsw-0001pU-Qo
	for qemu-devel@nongnu.org; Fri, 26 Sep 2014 08:44:35 -0400
Received: from mailapp01.imgtec.com ([195.59.15.196]:28008)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <James.Hogan@imgtec.com>) id 1XXUsw-0001o9-Kh
	for qemu-devel@nongnu.org; Fri, 26 Sep 2014 08:44:30 -0400
Message-ID: <54255FA6.3020400@imgtec.com>
Date: Fri, 26 Sep 2014 13:44:22 +0100
From: James Hogan <james.hogan@imgtec.com>
MIME-Version: 1.0
References: <1403882530-47821-1-git-send-email-leon.alrae@imgtec.com>
	<1403882530-47821-5-git-send-email-leon.alrae@imgtec.com>
In-Reply-To: <1403882530-47821-5-git-send-email-leon.alrae@imgtec.com>
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v3 04/21] target-mips: move LL and SC
	instructions
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: Leon Alrae <leon.alrae@imgtec.com>, qemu-devel@nongnu.org
Cc: yongbok.kim@imgtec.com, cristian.cuna@imgtec.com, aurelien@aurel32.net, rth@twiddle.net

Hi Leon,

On 27/06/14 16:21, Leon Alrae wrote:
> @@ -1215,6 +1217,8 @@ const struct mips_opcode mips_builtin_opcodes[] =
>     them first.  The assemblers uses a hash table based on the
>     instruction name anyhow.  */
>  /* name,    args,	match,	    mask,	pinfo,          	membership */
> +{"ll",      "t,o(b)",   0x7c000036, 0xfc00003f, LDD|RD_b|WR_t,        0, I32R6},
> +{"sc",      "t,o(b)",   0x7c000026, 0xfc00003f, LDD|RD_b|WR_t,        0, I32R6},

Doesn't bit 6 need to be 0 too for these, so mask should be 0xfc00007f?

Again, do these strictly have to be at the beginning? I know sc aliases
dmod.g, but that's right at the end of the table.

> @@ -15121,7 +15144,8 @@ static void decode_opc (CPUMIPSState *env, DisasContext *ctx)
>              break;
>          case OPC_DDIV_G_2E ... OPC_DDIVU_G_2E:
>          case OPC_DMULT_G_2E ... OPC_DMULTU_G_2E:
> -        case OPC_DMOD_G_2E ... OPC_DMODU_G_2E:
> +        case OPC_DMODU_G_2E:
> +            check_insn_opc_removed(ctx, ISA_MIPS32R6);

AFAICT you remove this check_insn_opc_removed line again in patch 6, so
I don't think you need to add it here.

Otherwise
Reviewed-by: James Hogan <james.hogan@imgtec.com>

Cheers
James