From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56732) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XIO7f-0007ZA-K0 for qemu-devel@nongnu.org; Fri, 15 Aug 2014 16:29:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XIO7W-0003ov-JX for qemu-devel@nongnu.org; Fri, 15 Aug 2014 16:29:15 -0400 Sender: Richard Henderson Message-ID: <53EE6D8A.7090900@twiddle.net> Date: Fri, 15 Aug 2014 10:28:58 -1000 From: Richard Henderson MIME-Version: 1.0 References: <1407851110-8075-1-git-send-email-tommusta@gmail.com> <1407851110-8075-2-git-send-email-tommusta@gmail.com> In-Reply-To: <1407851110-8075-2-git-send-email-tommusta@gmail.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [V2 PATCH 1/8] target-ppc: Bug Fix: rlwinm List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tom Musta , qemu-devel@nongnu.org, qemu-ppc@nongnu.org Cc: agraf@suse.de, david@gibson.dropbear.id.au On 08/12/2014 03:45 AM, Tom Musta wrote: > The rlwinm specification includes the ROTL32 operation, which is defined > to be a left rotation of two copies of the least significant 32 bits of > the source GPR. > > The current implementation is incorrect on 64-bit implementations in that > it rotates a single copy of the least significant 32 bits, padding with > zeroes in the most significant bits. > > Fix the code to properly implement this ROTL32 operation. > > Example: > R3 = F7487D82EC6F75DF > rlwinm 3,3,5,12,4 > > R3 expected : 8DEEBBFD880EBBFD > R3 actual : 00000000880EBBFD (without this fix) > > Signed-off-by: Tom Musta > --- > target-ppc/translate.c | 8 +++----- > 1 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index b23933f..a27d063 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -1672,11 +1672,9 @@ static void gen_rlwinm(DisasContext *ctx) > } else { > TCGv t0 = tcg_temp_new(); > #if defined(TARGET_PPC64) > - TCGv_i32 t1 = tcg_temp_new_i32(); > - tcg_gen_trunc_i64_i32(t1, cpu_gpr[rS(ctx->opcode)]); > - tcg_gen_rotli_i32(t1, t1, sh); > - tcg_gen_extu_i32_i64(t0, t1); > - tcg_temp_free_i32(t1); > + tcg_gen_deposit_i64(t0, cpu_gpr[rS(ctx->opcode)], > + cpu_gpr[rS(ctx->opcode)], 32, 32); > + tcg_gen_rotli_i64(t0, t0, sh); > #else > tcg_gen_rotli_i32(t0, cpu_gpr[rS(ctx->opcode)], sh); > #endif > You might want to retain a special case for mb==0 and me==31 which would be a straight 32-bit rotate with subsequent zero-extension. Just like we've got special cases for the normal shifts. r~