From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mo0YZ-000826-4L for qemu-devel@nongnu.org; Wed, 16 Sep 2009 15:52:47 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mo0YY-00081S-LM for qemu-devel@nongnu.org; Wed, 16 Sep 2009 15:52:46 -0400 Received: from [199.232.76.173] (port=37081 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mo0YY-00081N-BW for qemu-devel@nongnu.org; Wed, 16 Sep 2009 15:52:46 -0400 Received: from hall.aurel32.net ([88.191.82.174]:55316) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Mo0YX-0003Xa-RB for qemu-devel@nongnu.org; Wed, 16 Sep 2009 15:52:46 -0400 Date: Wed, 16 Sep 2009 21:52:42 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH] Fix extlh instruction on Alpha Message-ID: <20090916195242.GE770@volta.aurel32.net> References: <20090909120628.J4195@stanley.csl.cornell.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <20090909120628.J4195@stanley.csl.cornell.edu> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vince Weaver Cc: qemu-devel@nongnu.org On Wed, Sep 09, 2009 at 12:08:25PM -0400, Vince Weaver wrote: > > (re-sending) > > The extlh instruction on Alpha currently doesn't work properly. > It's a combination of a cut/paste bug (16 where it should be 32) as well > as a "shift by 64" bug. > > Below is a patch that fixes the problem. The previous e-mails on this > problem have test cases that exhibit the bug. > > This patch uses tcg_temp_local_new() at the suggestion of Filip Navara. > > Vince > > Signed-off-by: Vince Weaver > > diff --git a/target-alpha/translate.c b/target-alpha/translate.c > index 1fc5119..4219916 100644 > --- a/target-alpha/translate.c > +++ b/target-alpha/translate.c > @@ -526,14 +526,24 @@ static always_inline void gen_ext_h(void (*tcg_gen_ext_i64)(TCGv t0, TCGv t1), > else > tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]); > } else { > + int l1; > TCGv tmp1, tmp2; > - tmp1 = tcg_temp_new(); > + tmp1 = tcg_temp_local_new(); > + l1 = gen_new_label(); > + > tcg_gen_andi_i64(tmp1, cpu_ir[rb], 7); > tcg_gen_shli_i64(tmp1, tmp1, 3); > + > + tcg_gen_mov_i64(cpu_ir[rc], cpu_ir[ra]); > + tcg_gen_brcondi_i64(TCG_COND_EQ, tmp1, 0, l1); > + > tmp2 = tcg_const_i64(64); > tcg_gen_sub_i64(tmp1, tmp2, tmp1); > tcg_temp_free(tmp2); Given that a test costs a lot (partly due to the fact temp local variable must be used), I do wonder if doing a AND here wouldn't be better: tcg_gen_andi_i64(tmp1, tmp1, 0x3f); > tcg_gen_shl_i64(cpu_ir[rc], cpu_ir[ra], tmp1); > + > + gen_set_label(l1); > + > tcg_temp_free(tmp1); > } > if (tcg_gen_ext_i64) > @@ -1320,7 +1330,7 @@ static always_inline int translate_one (DisasContext *ctx, uint32_t insn) > break; > case 0x6A: > /* EXTLH */ > - gen_ext_h(&tcg_gen_ext16u_i64, ra, rb, rc, islit, lit); > + gen_ext_h(&tcg_gen_ext32u_i64, ra, rb, rc, islit, lit); > break; > case 0x72: > /* MSKQH */ > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net