From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52861)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ZWdwD-0007OL-VQ
	for qemu-devel@nongnu.org; Tue, 01 Sep 2015 01:16:54 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ZWdw9-000551-V2
	for qemu-devel@nongnu.org; Tue, 01 Sep 2015 01:16:53 -0400
Received: from mail-pa0-x232.google.com ([2607:f8b0:400e:c03::232]:32811)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1ZWdw9-00054v-Nx
	for qemu-devel@nongnu.org; Tue, 01 Sep 2015 01:16:49 -0400
Received: by paczk9 with SMTP id zk9so15604393pac.0
	for <qemu-devel@nongnu.org>; Mon, 31 Aug 2015 22:16:48 -0700 (PDT)
Sender: Richard Henderson <rth7680@gmail.com>
References: <1440433079-14458-1-git-send-email-rth@twiddle.net>
	<1440433079-14458-23-git-send-email-rth@twiddle.net>
	<CAFEAcA9M+JVOBrwnP3Hf46-+J1ZQNHq7y5z9y9rMSt3-Dhv9_g@mail.gmail.com>
From: Richard Henderson <rth@twiddle.net>
Message-ID: <55E534BD.5030704@twiddle.net>
Date: Mon, 31 Aug 2015 22:16:45 -0700
MIME-Version: 1.0
In-Reply-To: <CAFEAcA9M+JVOBrwnP3Hf46-+J1ZQNHq7y5z9y9rMSt3-Dhv9_g@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v14 22/33] target-tilegx: Implement system
 and memory management 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: Peter Maydell <peter.maydell@linaro.org>
Cc: "walt@tilera.com" <walt@tilera.com>, Chris Metcalf <cmetcalf@ezchip.com>, QEMU Developers <qemu-devel@nongnu.org>, Chen Gang <xili_gchen_5257@hotmail.com>

On 08/29/2015 02:21 PM, Peter Maydell wrote:
>> +    case OE_RR_X1(SWINT3):
>> +        ret = TILEGX_EXCP_OPCODE_UNKNOWN;
>> +        mnemonic = "swint3";
>> +        goto done0;
>> +    done0:
>
> goto to immediately following label ?

Sometimes nicer than fallthrough when future patches might interpose extra code.

>> @@ -284,8 +339,12 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext,
>>           dc->jmp.cond = TCG_COND_ALWAYS;
>>           dc->jmp.dest = tcg_temp_new();
>>           tcg_gen_andi_tl(dc->jmp.dest, load_gr(dc, srca), ~7);
>
> Should this really fall through into the added check ?
>
>> +    done1:
>> +        if (dest && ret == TILEGX_EXCP_NONE) {
>> +            ret = TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
>> +        }
...
>>       case OE_RR_X1(LNK):
>>       case OE_RR_Y1(LNK):
>>           if (srca) {
>> -            return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
>> +            ret = TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
>
> This code change seems unrelated to the rest of the patch?
> Also we'll end up printing the disassembly as "lnk" rather
> than whatever we should print for undefined instructions.

I'm not really sure what to do with reserved operand fields on this 
architecture.  Raising an exception is what Chen started with, but the encoding 
for brk suggests that they're ignored.

Suggestions?


r~