From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40529) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXVor-00005j-97 for qemu-devel@nongnu.org; Thu, 03 Sep 2015 10:48:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZXVoo-0005VG-3b for qemu-devel@nongnu.org; Thu, 03 Sep 2015 10:48:53 -0400 Received: from mail.uni-paderborn.de ([131.234.142.9]:46673) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXVon-0005V9-Tw for qemu-devel@nongnu.org; Thu, 03 Sep 2015 10:48:50 -0400 References: <1441239463-18981-1-git-send-email-rth@twiddle.net> <1441239463-18981-4-git-send-email-rth@twiddle.net> From: Bastian Koppelmann Message-ID: <55E85DCE.9050305@mail.uni-paderborn.de> Date: Thu, 3 Sep 2015 16:48:46 +0200 MIME-Version: 1.0 In-Reply-To: <1441239463-18981-4-git-send-email-rth@twiddle.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 03/17] target-openrisc: Invert the decoding in dec_calc List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, proljc@gmail.com On 09/03/2015 02:17 AM, Richard Henderson wrote: > Decoding the opcodes in the right order reduces by 100+ lines. > Also, it happens to put the opcodes in the same order as Chapter 17. > > Signed-off-by: Richard Henderson > --- > target-openrisc/translate.c | 300 ++++++++++++++------------------------------ > 1 file changed, 94 insertions(+), 206 deletions(-) > > diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c > index 9a8f886..c9e3198 100644 > --- a/target-openrisc/translate.c > +++ b/target-openrisc/translate.c > @@ -419,267 +419,155 @@ static void dec_calc(DisasContext *dc, uint32_t insn) > rb = extract32(insn, 11, 5); > rd = extract32(insn, 21, 5); > > - switch (op0) { I think it's worth while to group all instructions with op0 = 0 into a separate decoding function. > - > - case 0x000c: > - switch (op1) { > - case 0x00: > + case 0xc: > switch (op2) { > - case 0x00: /* l.exths */ > + case 0: /* l.exths */ > LOG_DIS("l.exths r%d, r%d\n", rd, ra); > tcg_gen_ext16s_tl(cpu_R[rd], cpu_R[ra]); Nitpick: l.exths comes before l.add in chapter 17 of my manual [1] However those are mostly minor issues, so Reviewed-by: Bastian Koppelmann Cheers, Bastian [1] https://github.com/openrisc/doc/blob/master/openrisc-arch-1.1-rev0.pdf?raw=true