From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41037) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbdrR-0004l3-2A for qemu-devel@nongnu.org; Mon, 04 Jun 2012 16:26:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SbdrO-0002nR-JA for qemu-devel@nongnu.org; Mon, 04 Jun 2012 16:26:44 -0400 Received: from v220110690675601.yourvserver.net ([78.47.199.172]:44455) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SbdrO-0002n4-93 for qemu-devel@nongnu.org; Mon, 04 Jun 2012 16:26:42 -0400 Message-ID: <4FCD19FE.5020406@weilnetz.de> Date: Mon, 04 Jun 2012 22:26:38 +0200 From: Stefan Weil MIME-Version: 1.0 References: <1338521008-12740-1-git-send-email-peter.crosthwaite@petalogix.com> In-Reply-To: <1338521008-12740-1-git-send-email-peter.crosthwaite@petalogix.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4] target-microblaze: lwx/swx: first implementation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Peter A. G. Crosthwaite" Cc: edgar.iglesias@gmail.com, david.holsgrove@petalogix.com, qemu-devel@nongnu.org, john.williams@petalogix.com Hi Peter, this patch (which was applied to QEMU master) breaks debug builds (configure --enable-debug). see my comment below. Regards, Stefan W. Am 01.06.2012 05:23, schrieb Peter A. G. Crosthwaite: > Signed-off-by: Peter A. G. Crosthwaite > --- > changed from v3: > simplified tcg local variable usage aqcross branch > changed from v2: > fixed tcg local variable usage across branch > reworked carry logic (with new write_carryi() function) > made LOG_DIS show lwx swx properly > changed from v1: > implemented reservation address checking > created new cpu state variable specifically for reservation address/flag state > > target-microblaze/cpu.c | 1 + > target-microblaze/cpu.h | 4 ++ > target-microblaze/helper.c | 2 + > target-microblaze/translate.c | 62 +++++++++++++++++++++++++++++++++++++--- > 4 files changed, 64 insertions(+), 5 deletions(-) > ... > diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c > index a362938..f0ebd59 100644 > --- a/target-microblaze/translate.c > +++ b/target-microblaze/translate.c > @@ -162,6 +162,14 @@ static void write_carry(DisasContext *dc, TCGv v) > tcg_temp_free(t0); > } > > +static void write_carryi(DisasContext *dc, int carry) > +{ > + TCGv t0 = tcg_temp_new(); > + tcg_gen_movi_tl(t0, carry ? 1 : 0); > + write_carry(dc, t0); > + tcg_temp_free(t0); > +} > + > /* True if ALU operand b is a small immediate that may deserve > faster treatment. */ > static inline int dec_alu_op_b_is_small_imm(DisasContext *dc) > @@ -948,12 +956,13 @@ static inline void dec_byteswap(DisasContext *dc, TCGv dst, TCGv src, int size) > static void dec_load(DisasContext *dc) > { > TCGv t, *addr; > - unsigned int size, rev = 0; > + unsigned int size, rev = 0, ex = 0; > > size = 1<< (dc->opcode& 3); > > if (!dc->type_b) { > rev = (dc->ir>> 9)& 1; > + ex = (dc->ir>> 10)& 1; > } > > if (size> 4&& (dc->tb_flags& MSR_EE_FLAG) > @@ -963,7 +972,8 @@ static void dec_load(DisasContext *dc) > return; > } > > - LOG_DIS("l%d%s%s\n", size, dc->type_b ? "i" : "", rev ? "r" : ""); > + LOG_DIS("l%d%s%s%s\n", size, dc->type_b ? "i" : "", rev ? "r" : "", > + ex ? "x" : ""); > > t_sync_flags(dc); > addr = compute_ldst_addr(dc,&t); > @@ -1019,6 +1029,17 @@ static void dec_load(DisasContext *dc) > } > } > > + /* lwx does not throw unaligned access errors, so force alignment */ > + if (ex) { > + /* Force addr into the temp. */ > + if (addr !=&t) { > + t = tcg_temp_new(); > + tcg_gen_mov_tl(t, *addr); > + addr =&t; > + } > + tcg_gen_andi_tl(t, t, ~3); > + } > + > /* If we get a fault on a dslot, the jmpstate better be in sync. */ > sync_jmpstate(dc); > > @@ -1057,6 +1078,12 @@ static void dec_load(DisasContext *dc) > } > } > > + if (ex) { /* lwx */ > + /* no support for for AXI exclusive so always clear C */ > + write_carryi(dc, 0); > + tcg_gen_st_tl(*addr, cpu_env, offsetof(CPUMBState, res_addr)); > + } > + > if (addr ==&t) > tcg_temp_free(t); > } > @@ -1078,12 +1105,14 @@ static void gen_store(DisasContext *dc, TCGv addr, TCGv val, > > static void dec_store(DisasContext *dc) > { > - TCGv t, *addr; > - unsigned int size, rev = 0; > + TCGv t, *addr, swx_addr, r_check = 0; With --enable-debug, TCGv is a struct (not an int), therefore r_check = 0 does not work. swx_addr and r_check are used in the same context. Can their declarations be moved to the block where they are first used? If that is not possible, either none or both of these two variables should be initialised. > + int swx_skip = 0; > + unsigned int size, rev = 0, ex = 0; > > size = 1<< (dc->opcode& 3); > if (!dc->type_b) { > rev = (dc->ir>> 9)& 1; > + ex = (dc->ir>> 10)& 1; > } > > if (size> 4&& (dc->tb_flags& MSR_EE_FLAG) > @@ -1093,12 +1122,30 @@ static void dec_store(DisasContext *dc) > return; > } > > - LOG_DIS("s%d%s%s\n", size, dc->type_b ? "i" : "", rev ? "r" : ""); > + LOG_DIS("s%d%s%s%s\n", size, dc->type_b ? "i" : "", rev ? "r" : "", > + ex ? "x" : ""); > t_sync_flags(dc); > /* If we get a fault on a dslot, the jmpstate better be in sync. */ > sync_jmpstate(dc); > addr = compute_ldst_addr(dc,&t); > > + if (ex) { /* swx */ > + r_check = tcg_temp_new(); > + swx_addr = tcg_temp_local_new(); TCGv r_check = ...; TCGv swx_addr = ...; > + > + /* Force addr into the swx_addr. */ > + tcg_gen_mov_tl(swx_addr, *addr); > + addr =&swx_addr; > + /* swx does not throw unaligned access errors, so force alignment */ > + tcg_gen_andi_tl(swx_addr, swx_addr, ~3); > + > + tcg_gen_ld_tl(r_check, cpu_env, offsetof(CPUMBState, res_addr)); > + write_carryi(dc, 1); > + swx_skip = gen_new_label(); > + tcg_gen_brcond_tl(TCG_COND_NE, r_check, swx_addr, swx_skip); > + write_carryi(dc, 0); Add this code from block at end of function? gen_set_label(swx_skip); tcg_temp_free(swx_addr); tcg_temp_free(r_check); > + } > + > if (rev&& size != 4) { > /* Endian reverse the address. t is addr. */ > switch (size) { > @@ -1174,6 +1221,11 @@ static void dec_store(DisasContext *dc) > gen_helper_memalign(*addr, tcg_const_tl(dc->rd), > tcg_const_tl(1), tcg_const_tl(size - 1)); > } Move the following conditional block up? > + if (ex) { > + gen_set_label(swx_skip); > + tcg_temp_free(r_check); > + tcg_temp_free(swx_addr); > + }