From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47592) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SmwL4-000683-0z for qemu-devel@nongnu.org; Thu, 05 Jul 2012 20:24:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SmwL2-0005Os-19 for qemu-devel@nongnu.org; Thu, 05 Jul 2012 20:24:01 -0400 Received: from mail-lb0-f173.google.com ([209.85.217.173]:47305) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SmwL1-0005OV-Nj for qemu-devel@nongnu.org; Thu, 05 Jul 2012 20:23:59 -0400 Received: by lbok6 with SMTP id k6so12581641lbo.4 for ; Thu, 05 Jul 2012 17:23:56 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1341110730-444-1-git-send-email-proljc@gmail.com> <1341110730-444-9-git-send-email-proljc@gmail.com> Date: Fri, 6 Jul 2012 08:23:56 +0800 Message-ID: From: Jia Liu Content-Type: text/plain; charset=ISO-8859-1 Subject: Re: [Qemu-devel] [PATCH v8 08/16] target-or32: Add instruction translation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel@nongnu.org Hi Blue, On Fri, Jul 6, 2012 at 2:45 AM, Blue Swirl wrote: > On Wed, Jul 4, 2012 at 11:59 PM, Jia Liu wrote: >> On Wed, Jul 4, 2012 at 10:23 AM, Jia Liu wrote: >>> Hi Blue, >>> >>> On Wed, Jul 4, 2012 at 2:48 AM, Blue Swirl wrote: >>>> On Sun, Jul 1, 2012 at 2:45 AM, Jia Liu wrote: >>>>> + switch (op0) { >>>>> + case 0x00: /* l.j */ >>>>> + LOG_DIS("l.j %d\n", N26); >>>>> + case 0x01: /* l.jal */ >>>>> + LOG_DIS("l.jal %d\n", N26); >>>>> + case 0x03: /* l.bnf */ >>>>> + LOG_DIS("l.bnf %d\n", N26); >>>>> + case 0x04: /* l.bf */ >>>>> + LOG_DIS("l.bf %d\n", N26); >>>>> + >>>>> + gen_jump(dc, N26, 0, op0); >>>> >>>> The cases 0x00 to 0x03 will fall through to this case, calling LOG_DIS >>>> one to four times. >>> >>> Sorry, I can't find a better path. Is this code OK? >>> >>> switch (op0) { >>> case 0x00: /* l.j */ >>> case 0x01: /* l.jal */ >>> case 0x03: /* l.bnf */ >>> case 0x04: /* l.bf */ >>> LOG_DIS("l.j/l.jal/l.bnf/l.bf %d\n", N26); >>> >>> gen_jump(dc, N26, 0, op0); >>> >> >> or is this code OK? >> >> switch (op0) { >> case 0x00: /* l.j */ >> LOG_DIS("l.j %d\n", N26); >> gen_jump(dc, N26, 0, op0); >> break; >> case 0x01: /* l.jal */ >> LOG_DIS("l.jal %d\n", N26); >> gen_jump(dc, N26, 0, op0); >> break; >> case 0x03: /* l.bnf */ >> LOG_DIS("l.bnf %d\n", N26); >> gen_jump(dc, N26, 0, op0); >> break; >> case 0x04: /* l.bf */ >> LOG_DIS("l.bf %d\n", N26); >> gen_jump(dc, N26, 0, op0); >> break; > > Yes, with N26 defined only when LOG_DIS does something (see below). > >> >> >>>>> + >>>>> + case 0x11: /* l.jr */ >>>>> + LOG_DIS("l.jr r%d\n", rb); >>>>> + case 0x12: /* l.jalr */ >>>>> + LOG_DIS("l.jalr r%d\n", rb); >>>>> + >>>>> + gen_jump(dc, 0, rb, op0); >>>> >>>> Also here. >>>> >>> >>> And, this? >>> >> case 0x11: /* l.jr */ > > Now there's no LOG_DIS() line. Oh! I was too careless! It shouldn't a LOG_DIS() here! Thank you very much! > >> gen_jump(dc, 0, rb, op0); >> break; >> case 0x12: /* l.jalr */ >> LOG_DIS("l.jr r%d\n", rb); >> gen_jump(dc, 0, rb, op0); >> break; >> >>>>> +static void dec_sys(DisasContext *dc, uint32_t insn) >>>>> +{ >>>>> + uint32_t op0; >>>>> + /*uint32_t K16;*/ >>>>> + op0 = field(insn, 16, 8); >>>>> + /*K16 = field(insn, 0, 16);*/ >>>>> + >>>>> + switch (op0) { >>>>> + case 0x000: /* l.sys */ >>>>> + /*LOG_DIS("l.sys %d\n", K16);*/ >>>> >>>> Why commented out? >>> >>> When I build QEMU, I got a error msg like "unused var K16, treat >>> warnings as errors". >>> I didn't find a better way to handle it, so, commented it. > > This can be avoided by adjusting those variable definitions which are > only used by LOG_DIS() so that they are only defined when LOG_DIS > needs them: > #ifdef OPENRISC_DISAS > int K16, N26; > #endif > Thank you very much! This is a very nice way. >>> >>>> >>>>> + tcg_gen_movi_tl(cpu_pc, dc->pc); >>>>> + gen_exception(dc, EXCP_SYSCALL); >>>>> + dc->is_jmp = DISAS_UPDATE; >>>>> + break; >>>>> + >>>>> + case 0x100: /* l.trap */ >>>>> + /*LOG_DIS("l.trap %d\n", K16);*/ >>>> >>>> Ditto >>>> >>> >>> And, commented it here. >>> >>>>> + tcg_gen_movi_tl(cpu_pc, dc->pc); >>>>> + gen_exception(dc, EXCP_TRAP); >>>>> + break; >>>>> + >>> >>> Regards, >>> Jia. >> >> >> Regards, >> Jia. Regards, Jia.