From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45539) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxERi-0004wS-Rt for qemu-devel@nongnu.org; Thu, 20 Oct 2016 10:35:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxERf-0007AB-PI for qemu-devel@nongnu.org; Thu, 20 Oct 2016 10:35:50 -0400 Received: from mail-pf0-x244.google.com ([2607:f8b0:400e:c00::244]:35030) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1bxERf-00079w-Ia for qemu-devel@nongnu.org; Thu, 20 Oct 2016 10:35:47 -0400 Received: by mail-pf0-x244.google.com with SMTP id s8so5770272pfj.2 for ; Thu, 20 Oct 2016 07:35:47 -0700 (PDT) Sender: Richard Henderson References: <20161019032935.6717-1-marex@denx.de> <20161020134401.3703-1-marex@denx.de> From: Richard Henderson Message-ID: Date: Thu, 20 Oct 2016 07:35:43 -0700 MIME-Version: 1.0 In-Reply-To: <20161020134401.3703-1-marex@denx.de> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V5 2/7] nios2: Add architecture emulation support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marek Vasut , qemu-devel@nongnu.org Cc: Chris Wulff , Jeff Da Silva , Ley Foon Tan , Sandra Loosemore , Yves Vandervennet On 10/20/2016 06:44 AM, Marek Vasut wrote: > +typedef struct Nios2Instruction { > + void (*handler)(DisasContext *dc, uint32_t code, TCGMemOp flags); > + uint32_t flags; > +} Nios2Instruction; I gave you some bad advice wrt the type of flags beforehand. I had failed to divine that it was also used for EXCP_* and TCG_COND_*. I think you were right the first time with unsigned. My bad, sorry. > +/* Load instructions */ > +static void gen_ldx(DisasContext *dc, uint32_t code, TCGMemOp flags) > +{ > + I_TYPE(instr, code); > + > + TCGv addr = tcg_temp_new(); > + TCGv data = tcg_temp_new(); > + tcg_gen_addi_tl(addr, load_gpr(dc, instr.a), instr.imm16s); > + tcg_gen_qemu_ld_tl(data, addr, dc->mem_idx, flags); > + > + /* > + * WARNING: Loads into R_ZERO are ignored, but we must generate the > + * memory access itself to emulate the CPU precisely. Load > + * from a protected page to R_ZERO will cause SIGSEGV on > + * the Nios2 CPU. > + */ > + if (likely(instr.b != R_ZERO)) { > + tcg_gen_mov_tl(dc->cpu_R[instr.b], data); > + } Consider TCGv data; if (unlikely(instr.b == R_ZERO)) { /* The writeback to R_ZERO is ignored, but we must generate the * memory access itself to emulate the CPU precisely. Load from * a protected page to R_ZERO will cause SIGSEGV on the Nios2 CPU. */ data = tcg_temp_new(); } else { data = dc->cpu_R[instr.b]; } tcg_gen_qemu_ld_tl(data, addr, dc->mem_idx, flags); if (unlikely(instr.b == R_ZERO)) { tcg_temp_free(data); } so that you don't require the mov opcode. That's really what I do on Alpha with dest_gpr. > +#define gen_r_div(fname, insn) \ > +static void (fname)(DisasContext *dc, uint32_t code, TCGMemOp flags) \ > +{ \ > + R_TYPE(instr, (code)); \ > + if (likely(instr.c != R_ZERO)) { \ > + TCGv val = tcg_const_tl(0); \ > + tcg_gen_setcond_tl(TCG_COND_EQ, val, load_gpr((dc), instr.b), val); \ > + tcg_gen_or_tl(val, val, load_gpr((dc), instr.b)); \ > + tcg_gen_##insn((dc)->cpu_R[instr.c], load_gpr((dc), instr.a), val); \ > + tcg_temp_free(val); \ > + } \ > +} > + > +gen_r_div(divs, div_tl) For signed division, you have to protect against 0x80000000 / -1 as well, which raises an overflow exception on the x86 host. > + /* Set up instruction counts */ > + num_insns = 0; > + max_insns = tb->cflags & CF_COUNT_MASK; > + if (max_insns == 0) { > + max_insns = CF_COUNT_MASK; > + } > + if (max_insns > TCG_MAX_INSNS) { > + max_insns = TCG_MAX_INSNS; > + } > + next_page_start = (tb->pc & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; ... > + } while (!dc->is_jmp && > + !tcg_op_buf_full() && > + !cs->singlestep_enabled && > + !singlestep && > + dc->pc < next_page_start && > + num_insns < max_insns); Consider if (cs->singlestep_enabled || singlestep) { max_insns = 1; } else { int page_insns = (TARGET_PAGE_SIZE - (tb->pc & TARGET_PAGE_MASK)) / 4; max_insns = tb->cflags & CF_COUNT_MASK; if (max_insns == 0) { max_insns = CF_COUNT_MASK; } if (max_insns > page_insns) { max_insns = page_insns; } if (max_insns > TCG_MAX_INSNS) { max_insns = TCG_MAX_INSNS; } } so that we collapse the last 4 loop conditions into: num_insns < max_insns. > + /* End off the block */ > + gen_tb_end(tb, num_insns); > + > + /* Mark instruction starts for the final generated instruction */ > + tb->size = dc->pc - tb->pc; > + tb->icount = num_insns; No CPU_LOG_TB_IN_ASM disassembly? I thought patch 1 added a nios2 disassembler. r~