From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Kb6ZE-0002ZR-VG for qemu-devel@nongnu.org; Thu, 04 Sep 2008 00:35:37 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Kb6ZE-0002Z3-67 for qemu-devel@nongnu.org; Thu, 04 Sep 2008 00:35:36 -0400 Received: from [199.232.76.173] (port=41539 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Kb6ZD-0002Yy-WC for qemu-devel@nongnu.org; Thu, 04 Sep 2008 00:35:36 -0400 Received: from hall.aurel32.net ([91.121.138.14]:40616) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Kb6ZD-0005tJ-Qc for qemu-devel@nongnu.org; Thu, 04 Sep 2008 00:35:36 -0400 Received: from 151.red-80-38-163.staticip.rima-tde.net ([80.38.163.151] helo=volta.aurel32.net) by hall.aurel32.net with esmtpsa (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.63) (envelope-from ) id 1Kb6ZB-0003JQ-NW for qemu-devel@nongnu.org; Thu, 04 Sep 2008 06:35:34 +0200 Received: from aurel32 by volta.aurel32.net with local (Exim 4.69) (envelope-from ) id 1Kb6Z5-0002WX-Da for qemu-devel@nongnu.org; Thu, 04 Sep 2008 06:35:27 +0200 Date: Thu, 4 Sep 2008 06:35:27 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH v2] convert of few alpha insn to TCG Message-ID: <20080904043527.GA3416@volta.aurel32.net> References: <20080903084633.GD21732@volta.aurel32.net> <548C69A4-B4D1-4F86-87AB-EDD663895A53@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <548C69A4-B4D1-4F86-87AB-EDD663895A53@adacore.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Wed, Sep 03, 2008 at 02:44:48PM +0200, Tristan Gingold wrote: > Hi, > > all the issues have been fixed in this second version. > > Tristan. > > Signed-off-by: Tristan Gingold > I have applied a modified version of this patch, as I don't want to spend to much time doing mail ping pong. Please see the details of the change below. Also please watch the indentation, target-alpha/ is indented with space, I am not sure we want to mix that. > Index: target-alpha/op.c > =================================================================== > --- target-alpha/op.c (revision 5119) > +++ target-alpha/op.c (working copy) > @@ -131,12 +131,6 @@ > RETURN(); > } > > -void OPPROTO op_tb_flush (void) > -{ > - helper_tb_flush(); > - RETURN(); > -} > - > /* Load and stores */ > #define MEMSUFFIX _raw > #include "op_mem.h" > @@ -685,27 +679,6 @@ > } > #endif > > -#if 0 // Qemu does not know how to do this... > -void OPPROTO op_update_pc (void) > -{ > - env->pc = PARAM(1); > - RETURN(); > -} > -#else > -void OPPROTO op_update_pc (void) > -{ > - env->pc = ((uint64_t)PARAM(1) << 32) | (uint64_t)PARAM(2); > - RETURN(); > -} > -#endif > - > -/* Optimization for 32 bits hosts architectures */ > -void OPPROTO op_update_pc32 (void) > -{ > - env->pc = (uint64_t)PARAM(1); > - RETURN(); > -} > - > /* IEEE floating point arithmetic */ > /* S floating (single) */ > void OPPROTO op_adds (void) > Index: target-alpha/helper.h > =================================================================== > --- target-alpha/helper.h (revision 0) > +++ target-alpha/helper.h (revision 0) > @@ -0,0 +1,5 @@ > +#ifndef DEF_HELPER > +#define DEF_HELPER(ret, name, params) ret name params; > +#endif > + > +DEF_HELPER(void, helper_tb_flush, (void)) > Index: target-alpha/translate.c > =================================================================== > --- target-alpha/translate.c (revision 5119) > +++ target-alpha/translate.c (working copy) > @@ -25,6 +25,7 @@ > #include "cpu.h" > #include "exec-all.h" > #include "disas.h" > +#include "helper.h" > #include "tcg-op.h" > #include "qemu-common.h" > > @@ -44,15 +45,34 @@ > }; > > static TCGv cpu_env; > +static TCGv cpu_ir[31]; > +static TCGv cpu_pc; > > #include "gen-icount.h" > > static void alpha_translate_init(void) > { > static int done_init = 0; > + static char cpu_reg_names[4*31]; > + char *p; > + int i; > + > if (done_init) > - return; > + return; > + > cpu_env = tcg_global_reg_new(TCG_TYPE_PTR, TCG_AREG0, "env"); > + > + p = cpu_reg_names; > + for (i = 0; i < 31; i++) { > + sprintf(p, "r%d", i); ir would be better to match the name of the variable. > + cpu_ir[i] = tcg_global_mem_new(TCG_TYPE_I64, TCG_AREG0, > + offsetof(CPUState, ir[i]), p); > + p += 4; > + } > + > + cpu_pc = tcg_global_mem_new(TCG_TYPE_I64, TCG_AREG0, > + offsetof(CPUState, pc), "pc"); > + > done_init = 1; > } You should also register the helpers here. > @@ -126,6 +146,20 @@ > } > } > > +static inline void get_ir (TCGv t, int reg) > +{ > + if (reg == 31) > + tcg_gen_movi_i64(t, 0); > + else > + tcg_gen_mov_i64(t, cpu_ir[reg]); > +} > + > +static inline void set_ir (TCGv t, int reg) > +{ > + if (reg != 31) > + tcg_gen_mov_i64(cpu_ir[reg], t); > +} > + Even if the register are now mapped directly in TCG, this way of doing will not really help to find some optimisations later: the ir registers can't directly be a target register of a TCG instruction, the only way to read or write those register is through a move. I'll fix that in another patch. > /* FIR moves */ > /* Special hacks for fir31 */ > #define gen_op_load_FT0_fir31 gen_op_reset_FT0 > @@ -356,15 +390,9 @@ > > static always_inline void gen_update_pc (DisasContext *ctx) > { > - if (!(ctx->pc >> 32)) { > - gen_op_update_pc32(ctx->pc); > - } else { > -#if 0 // Qemu does not know how to do this... > - gen_op_update_pc(ctx->pc); > -#else > - gen_op_update_pc(ctx->pc >> 32, ctx->pc); > -#endif > - } > + TCGv v = tcg_const_i64(ctx->pc); > + tcg_gen_mov_i64(cpu_pc, v); > + tcg_temp_free(v); > } There is no need to use a TCG const, you can use tcg_gen_movi_i64 instead. > > static always_inline void _gen_op_bcond (DisasContext *ctx) > @@ -700,17 +728,23 @@ > goto invalid_opc; > case 0x08: > /* LDA */ > - gen_load_ir(ctx, rb, 0); > - gen_set_sT1(ctx, disp16); > - gen_op_addq(); > - gen_store_ir(ctx, ra, 0); > + { > + TCGv v = tcg_const_i64(disp16); > + if (rb != 31) > + tcg_gen_add_i64(v, cpu_ir[rb], v); > + set_ir(v, ra); > + tcg_temp_free(v); > + } > break; > case 0x09: > /* LDAH */ > - gen_load_ir(ctx, rb, 0); > - gen_set_sT1(ctx, disp16 << 16); > - gen_op_addq(); > - gen_store_ir(ctx, ra, 0); > + { > + TCGv v = tcg_const_i64(disp16 << 16); > + if (rb != 31) > + tcg_gen_add_i64(v, cpu_ir[rb], v); > + set_ir(v, ra); > + tcg_temp_free(v); > + } > break; > case 0x0A: > /* LDBU */ > @@ -1871,13 +1905,16 @@ > break; > case 0x30: > /* BR */ > - gen_set_uT0(ctx, ctx->pc); > - gen_store_ir(ctx, ra, 0); > - if (disp21 != 0) { > - gen_set_sT1(ctx, disp21 << 2); > - gen_op_addq(); > + if (ra != 31) { > + TCGv t = tcg_const_i64(ctx->pc); > + set_ir(t, ra); > + tcg_temp_free(t); > + } > + { > + TCGv pc = tcg_const_i64(ctx->pc + (disp21 << 2)); > + tcg_gen_mov_i64(cpu_pc, pc); > + tcg_temp_free(pc); > } Same here, tcg_gen_movi_i64 could be use instead. > - gen_op_branch(); > ret = 1; > break; > case 0x31: > @@ -2059,7 +2096,7 @@ > gen_update_pc(&ctx); > } > #if defined (DO_TB_FLUSH) > - gen_op_tb_flush(); > + tcg_gen_helper_0_0(helper_tb_flush); > #endif > if (tb->cflags & CF_LAST_IO) > gen_io_end(); > -- .''`. Aurelien Jarno | GPG: 1024D/F1BCDB73 : :' : Debian developer | Electrical Engineer `. `' aurel32@debian.org | aurelien@aurel32.net `- people.debian.org/~aurel32 | www.aurel32.net