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
`. `' [EMAIL PROTECTED] | [EMAIL PROTECTED]
`- people.debian.org/~aurel32 | www.aurel32.net
