From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LtHuq-0004YR-1v for qemu-devel@nongnu.org; Mon, 13 Apr 2009 04:53:20 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LtHup-0004YD-7J for qemu-devel@nongnu.org; Mon, 13 Apr 2009 04:53:19 -0400 Received: from [199.232.76.173] (port=44512 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LtHup-0004YA-1i for qemu-devel@nongnu.org; Mon, 13 Apr 2009 04:53:19 -0400 Received: from hall.aurel32.net ([88.191.82.174]:54307) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LtHuo-0008CO-Eo for qemu-devel@nongnu.org; Mon, 13 Apr 2009 04:53:18 -0400 Date: Mon, 13 Apr 2009 10:53:10 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [7042] target-mips: optimize decode_opc() Message-ID: <20090413085310.GA13745@volta.aurel32.net> References: <49E24BC0.1020109@mail.berlios.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <49E24BC0.1020109@mail.berlios.de> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: qemu-devel@nongnu.org On Sun, Apr 12, 2009 at 10:14:56PM +0200, Stefan Weil wrote: > Aurelien Jarno schrieb: > > Revision: 7042 > > http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=7042 > > Author: aurel32 > > Date: 2009-04-08 21:47:55 +0000 (Wed, 08 Apr 2009) > > Log Message: > > ----------- > > target-mips: optimize decode_opc() > > > > Signed-off-by: Aurelien Jarno > > > > Modified Paths: > > -------------- > > trunk/target-mips/translate.c > > > > Modified: trunk/target-mips/translate.c > > =================================================================== > > --- trunk/target-mips/translate.c 2009-04-08 21:47:44 UTC (rev 7041) > > +++ trunk/target-mips/translate.c 2009-04-08 21:47:55 UTC (rev 7042) > > @@ -7527,7 +7527,6 @@ > > case OPC_MOVCI: > > check_insn(env, ctx, ISA_MIPS4 | ISA_MIPS32); > > if (env->CP0_Config1 & (1 << CP0C1_FP)) { > > - save_cpu_state(ctx, 1); > > check_cp1_enabled(ctx); > > gen_movci(ctx, rd, rs, (ctx->opcode >> 18) & 0x7, > > (ctx->opcode >> 16) & 1); > > @@ -7623,28 +7622,33 @@ > > case OPC_RDHWR: > > check_insn(env, ctx, ISA_MIPS32R2); > > { > > - TCGv t0 = tcg_temp_local_new(); > > + TCGv t0 = tcg_temp_new(); > > > > switch (rd) { > > case 0: > > save_cpu_state(ctx, 1); > > gen_helper_rdhwr_cpunum(t0); > > + gen_store_gpr(t0, rt); > > break; > > case 1: > > save_cpu_state(ctx, 1); > > gen_helper_rdhwr_synci_step(t0); > > + gen_store_gpr(t0, rt); > > break; > > case 2: > > save_cpu_state(ctx, 1); > > gen_helper_rdhwr_cc(t0); > > + gen_store_gpr(t0, rt); > > break; > > case 3: > > save_cpu_state(ctx, 1); > > gen_helper_rdhwr_ccres(t0); > > + gen_store_gpr(t0, rt); > > break; > > case 29: > > #if defined(CONFIG_USER_ONLY) > > tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUState, tls_value)); > > + gen_store_gpr(t0, rt); > > break; > > #else > > /* XXX: Some CPUs implement this in hardware. > > @@ -7655,15 +7659,14 @@ > > generate_exception(ctx, EXCP_RI); > > break; > > } > > - gen_store_gpr(t0, rt); > > tcg_temp_free(t0); > > } > > break; > > case OPC_FORK: > > check_insn(env, ctx, ASE_MT); > > { > > - TCGv t0 = tcg_temp_local_new(); > > - TCGv t1 = tcg_temp_local_new(); > > + TCGv t0 = tcg_temp_new(); > > + TCGv t1 = tcg_temp_new(); > > > > gen_load_gpr(t0, rt); > > gen_load_gpr(t1, rs); > > @@ -7675,8 +7678,9 @@ > > case OPC_YIELD: > > check_insn(env, ctx, ASE_MT); > > { > > - TCGv t0 = tcg_temp_local_new(); > > + TCGv t0 = tcg_temp_new(); > > > > + save_cpu_state(ctx, 1); > > gen_load_gpr(t0, rs); > > gen_helper_yield(t0, t0); > > gen_store_gpr(t0, rd); > > @@ -7748,37 +7752,41 @@ > > case OPC_MFMC0: > > #ifndef CONFIG_USER_ONLY > > { > > - TCGv t0 = tcg_temp_local_new(); > > + TCGv t0 = tcg_temp_new(); > > > > op2 = MASK_MFMC0(ctx->opcode); > > switch (op2) { > > case OPC_DMT: > > check_insn(env, ctx, ASE_MT); > > gen_helper_dmt(t0, t0); > > + gen_store_gpr(t0, rt); > > break; > > case OPC_EMT: > > check_insn(env, ctx, ASE_MT); > > gen_helper_emt(t0, t0); > > + gen_store_gpr(t0, rt); > > break; > > case OPC_DVPE: > > check_insn(env, ctx, ASE_MT); > > gen_helper_dvpe(t0, t0); > > + gen_store_gpr(t0, rt); > > break; > > case OPC_EVPE: > > check_insn(env, ctx, ASE_MT); > > gen_helper_evpe(t0, t0); > > + gen_store_gpr(t0, rt); > > break; > > case OPC_DI: > > check_insn(env, ctx, ISA_MIPS32R2); > > - save_cpu_state(ctx, 1); > save_cpu_state needed? > > gen_helper_di(t0); > > + gen_store_gpr(t0, rt); > > /* Stop translation as we may have switched the execution mode */ > > ctx->bstate = BS_STOP; > > break; > > case OPC_EI: > > check_insn(env, ctx, ISA_MIPS32R2); > > - save_cpu_state(ctx, 1); > save_cpu_state needed? > > gen_helper_ei(t0); > > + gen_store_gpr(t0, rt); > > /* Stop translation as we may have switched the execution mode */ > > ctx->bstate = BS_STOP; > > break; > > @@ -7787,7 +7795,6 @@ > > generate_exception(ctx, EXCP_RI); > > break; > > } > > - gen_store_gpr(t0, rt); > > tcg_temp_free(t0); > > } > > #endif /* !CONFIG_USER_ONLY */ > > @@ -7839,7 +7846,6 @@ > > case OPC_SWC1: > > case OPC_SDC1: > > if (env->CP0_Config1 & (1 << CP0C1_FP)) { > > - save_cpu_state(ctx, 1); > > check_cp1_enabled(ctx); > > gen_flt_ldst(ctx, op, rt, rs, imm); > > } else { > > @@ -7849,7 +7855,6 @@ > > > > case OPC_CP1: > > if (env->CP0_Config1 & (1 << CP0C1_FP)) { > > - save_cpu_state(ctx, 1); > > check_cp1_enabled(ctx); > > op1 = MASK_CP1(ctx->opcode); > > switch (op1) { > > @@ -7908,7 +7913,6 @@ > > > > case OPC_CP3: > > if (env->CP0_Config1 & (1 << CP0C1_FP)) { > > - save_cpu_state(ctx, 1); > > check_cp1_enabled(ctx); > > op1 = MASK_CP3(ctx->opcode); > > switch (op1) { > > > > This patch breaks Linux boot (fatal kernel error during boot, > wrong kernel mode?). I'm sorry that I cannot give more > details at the moment because I have trouble with > my test hardware. > > Tested with QEMU trunk, mips little endian, malta machine. > > Re-adding the two removed calls of save_cpu_state (for > OPC_DI, OPC_EI) lets the kernel boot again. > Maybe there is a better fix. > Reverting those parts is actually the correct fix. I have remove the calls to save_cpu_states() for helpers that can't trigger an exception, but I forget to consider the case where those helpers are rising an interrupt, calling in fine cpu_unlink_tb(). Strangely my test image was still working here, probably because I only tried with a kernel without R2 support. That should be fixed in the SVN now. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net