From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Myp6S-0001yU-7E for qemu-devel@nongnu.org; Fri, 16 Oct 2009 11:52:28 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Myp6R-0001y1-M0 for qemu-devel@nongnu.org; Fri, 16 Oct 2009 11:52:27 -0400 Received: from [199.232.76.173] (port=47229 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Myp6R-0001xs-HR for qemu-devel@nongnu.org; Fri, 16 Oct 2009 11:52:27 -0400 Received: from hall.aurel32.net ([88.191.82.174]:55592) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Myp6Q-0004UH-Ve for qemu-devel@nongnu.org; Fri, 16 Oct 2009 11:52:27 -0400 Date: Fri, 16 Oct 2009 17:52:21 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH 1/9] TCG "sync" op Message-ID: <20091016155221.GF4127@hall.aurel32.net> References: <1255696735-21396-1-git-send-email-uli@suse.de> <1255696735-21396-2-git-send-email-uli@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1255696735-21396-2-git-send-email-uli@suse.de> Sender: Aurelien Jarno List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ulrich Hecht Cc: riku.voipio@iki.fi, qemu-devel@nongnu.org, agraf@suse.de On Fri, Oct 16, 2009 at 02:38:47PM +0200, Ulrich Hecht wrote: > sync allows concurrent accesses to locations in memory through different TCG > variables. This comes in handy when you are emulating CPU registers that can > be used as either 32 or 64 bit, as TCG doesn't know anything about aliases. > See the s390x target for an example. > > Fixed sync_i64 build failure on 32-bit targets. It don't really see the point of such a new op, especially the way it is used in the S390 target. If a global is "synced" before each load/store, it will be load/stored from/to memory each time it is used. This is exactly what tcg_gen_ld/st do, except it's only one op. The benefit of globals in TCG is to hold them as long as possible in host register and avoid costly memory load/store. tcg_gen_ld/st would probably be even more efficient, as it is one op instead of two, and also because mapping more globals means more time spent in the code looping over all globals. IMHO, the correct way to do it is to use the following code, assuming you want to use 64-bit TCG regs to hold 32-bit values (that's something that is not really clear in your next patch): - for register load: | static TCGv load_reg(int reg) | { | TCGv r = tcg_temp_new_i64(); | tcg_gen_ext32u_i64(r, tcgregs[reg]); | return r; | } | | static void store_reg32(int reg, TCGv v) | { | tcg_gen_ext32u_i64(v, v); /* may be optional */ | tcg_gen_andi_i64(tcgregs[reg], tcgregs[reg], 0xffffffff00000000ULL); | tcg_gen_or_i64(tcgregs[reg], tcgregs[reg], v); | } If you want to do the same using 32-bit TCG regs: | static TCGv_i32 load_reg(int reg) | { | TCGv_i32 r = tcg_temp_new_i32(); | tcg_gen_extu_i32_i64(r, tcgregs[reg]); | return r; | } | | static void store_reg32(int reg, TCGv_i32 v) | { | TCGv tmp = tcg_temp_new(); | tcg_gen_extu_i32_i64(tmp, v); | tcg_gen_andi_i64(tcgregs[reg], tcgregs[reg], 0xffffffff00000000ULL); | tcg_gen_or_i64(tcgregs[reg], tcgregs[reg], tmp); | tcg_temp_free(tmp); | } Regards, Aurelien > Signed-off-by: Ulrich Hecht > --- > tcg/tcg-op.h | 12 ++++++++++++ > tcg/tcg-opc.h | 2 ++ > tcg/tcg.c | 6 ++++++ > 3 files changed, 20 insertions(+), 0 deletions(-) > > diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h > index faf2e8b..c1b4710 100644 > --- a/tcg/tcg-op.h > +++ b/tcg/tcg-op.h > @@ -316,6 +316,18 @@ static inline void tcg_gen_br(int label) > tcg_gen_op1i(INDEX_op_br, label); > } > > +static inline void tcg_gen_sync_i32(TCGv_i32 arg) > +{ > + tcg_gen_op1_i32(INDEX_op_sync_i32, arg); > +} > + > +#if TCG_TARGET_REG_BITS == 64 > +static inline void tcg_gen_sync_i64(TCGv_i64 arg) > +{ > + tcg_gen_op1_i64(INDEX_op_sync_i64, arg); > +} > +#endif > + > static inline void tcg_gen_mov_i32(TCGv_i32 ret, TCGv_i32 arg) > { > if (!TCGV_EQUAL_I32(ret, arg)) > diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h > index b7f3fd7..5dcdeba 100644 > --- a/tcg/tcg-opc.h > +++ b/tcg/tcg-opc.h > @@ -40,6 +40,7 @@ DEF2(call, 0, 1, 2, TCG_OPF_SIDE_EFFECTS) /* variable number of parameters */ > DEF2(jmp, 0, 1, 0, TCG_OPF_BB_END | TCG_OPF_SIDE_EFFECTS) > DEF2(br, 0, 0, 1, TCG_OPF_BB_END | TCG_OPF_SIDE_EFFECTS) > > +DEF2(sync_i32, 0, 1, 0, 0) > DEF2(mov_i32, 1, 1, 0, 0) > DEF2(movi_i32, 1, 0, 1, 0) > /* load/store */ > @@ -109,6 +110,7 @@ DEF2(neg_i32, 1, 1, 0, 0) > #endif > > #if TCG_TARGET_REG_BITS == 64 > +DEF2(sync_i64, 0, 1, 0, 0) > DEF2(mov_i64, 1, 1, 0, 0) > DEF2(movi_i64, 1, 0, 1, 0) > /* load/store */ > diff --git a/tcg/tcg.c b/tcg/tcg.c > index 3c0e296..8eb60f8 100644 > --- a/tcg/tcg.c > +++ b/tcg/tcg.c > @@ -1930,6 +1930,12 @@ static inline int tcg_gen_code_common(TCGContext *s, uint8_t *gen_code_buf, > // dump_regs(s); > #endif > switch(opc) { > + case INDEX_op_sync_i32: > +#if TCG_TARGET_REG_BITS == 64 > + case INDEX_op_sync_i64: > +#endif > + temp_save(s, args[0], s->reserved_regs); > + break; > case INDEX_op_mov_i32: > #if TCG_TARGET_REG_BITS == 64 > case INDEX_op_mov_i64: > -- > 1.6.2.1 > > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net