From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1HXySc-0006rK-PT for qemu-devel@nongnu.org; Sun, 01 Apr 2007 07:43:02 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1HXySa-0006r8-BZ for qemu-devel@nongnu.org; Sun, 01 Apr 2007 07:43:01 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1HXySa-0006r5-8c for qemu-devel@nongnu.org; Sun, 01 Apr 2007 07:43:00 -0400 Received: from farad.aurel32.net ([82.232.2.251] helo=mail.aurel32.net) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1HXyPe-000672-RO for qemu-devel@nongnu.org; Sun, 01 Apr 2007 07:39:59 -0400 Date: Sun, 1 Apr 2007 13:39:56 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH][SPARC] Fix the shift instructions for theSPARC target Message-ID: <20070401113956.GA19821@amd64.aurel32.net> References: <20070325140539.GA11179@amd64.aurel32.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel@nongnu.org On Sun, Apr 01, 2007 at 11:32:06AM +0200, Blue Swirl wrote: > >The shift instructions on the SPARC target currently take into account > >the whole register as the shift count. According to the SPARC v8 and v9 > >manuals, only the lower 5 bits should be taken into account for 32-bit > >instructions (SLL, SRL, SRA), and only the lower 6 bits for 64-bit > >instructions (SLLX, SRLX, SRAX). > > > >The patch below fixes that. Note that SLL and SLLX are now different, as > >they don't take into account the same number of bits. Please apply. > > Can you check what happens in real hardware, especially in the case when > the shift amount is in a register, not immediate, and the value is either > >32 or ==32? I have just made the test on real hardware. Specifying a value of 32 leave the register unchanged. Specifying a value > 32 only takes the lower 5 bits of the value. So exactly as specified in the manual. > The 64-bit mask should be 0x3f, not 0x2f. Oops you are right, please find an updated patch below. Index: target-sparc/op.c =================================================================== RCS file: /sources/qemu/qemu/target-sparc/op.c,v retrieving revision 1.26 diff -u -d -p -r1.26 op.c --- target-sparc/op.c 23 Mar 2007 20:01:20 -0000 1.26 +++ target-sparc/op.c 25 Mar 2007 13:50:45 -0000 @@ -965,38 +965,43 @@ void OPPROTO op_logic_T0_cc(void) void OPPROTO op_sll(void) { - T0 <<= T1; + T0 <<= (T1 & 0x1f); } #ifdef TARGET_SPARC64 +void OPPROTO op_sllx(void) +{ + T0 <<= (T1 & 0x3f); +} + void OPPROTO op_srl(void) { - T0 = (T0 & 0xffffffff) >> T1; + T0 = (T0 & 0xffffffff) >> (T1 & 0x1f); } void OPPROTO op_srlx(void) { - T0 >>= T1; + T0 >>= (T1 & 0x3f); } void OPPROTO op_sra(void) { - T0 = ((int32_t) (T0 & 0xffffffff)) >> T1; + T0 = ((int32_t) (T0 & 0xffffffff)) >> (T1 & 0x1f); } void OPPROTO op_srax(void) { - T0 = ((int64_t) T0) >> T1; + T0 = ((int64_t) T0) >> (T1 & 0x3f); } #else void OPPROTO op_srl(void) { - T0 >>= T1; + T0 >>= (T1 & 0x1f); } void OPPROTO op_sra(void) { - T0 = ((int32_t) T0) >> T1; + T0 = ((int32_t) T0) >> (T1 & 0x1f); } #endif Index: target-sparc/translate.c =================================================================== RCS file: /sources/qemu/qemu/target-sparc/translate.c,v retrieving revision 1.38 diff -u -d -p -r1.38 translate.c --- target-sparc/translate.c 25 Mar 2007 07:55:52 -0000 1.38 +++ target-sparc/translate.c 25 Mar 2007 13:50:45 -0000 @@ -1693,7 +1693,7 @@ static void disas_sparc_insn(DisasContex } #endif #ifdef TARGET_SPARC64 - } else if (xop == 0x25) { /* sll, V9 sllx ( == sll) */ + } else if (xop == 0x25) { /* sll, V9 sllx */ rs1 = GET_FIELD(insn, 13, 17); gen_movl_reg_T0(rs1); if (IS_IMM) { /* immediate */ @@ -1703,7 +1703,10 @@ static void disas_sparc_insn(DisasContex rs2 = GET_FIELD(insn, 27, 31); gen_movl_reg_T1(rs2); } - gen_op_sll(); + if (insn & (1 << 12)) + gen_op_sllx(); + else + gen_op_sll(); gen_movl_T0_reg(rd); } else if (xop == 0x26) { /* srl, V9 srlx */ rs1 = GET_FIELD(insn, 13, 17); -- .''`. Aurelien Jarno | GPG: 1024D/F1BCDB73 : :' : Debian developer | Electrical Engineer `. `' aurel32@debian.org | aurelien@aurel32.net `- people.debian.org/~aurel32 | www.aurel32.net