[Qemu-devel] [5123] Fix y register loads and stores

[Qemu-devel] [5123] Fix y register loads and stores

[Blue Swirl]

Revision: 5123
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5123
Author:   blueswir1
Date:     2008-09-01 19:35:29 +0000 (Mon, 01 Sep 2008)

Log Message:
-----------
Fix y register loads and stores

Modified Paths:
--------------
    trunk/target-sparc/translate.c

Modified: trunk/target-sparc/translate.c
===================================================================
--- trunk/target-sparc/translate.c	2008-09-01 13:09:21 UTC (rev 5122)
+++ trunk/target-sparc/translate.c	2008-09-01 19:35:29 UTC (rev 5123)
@@ -713,36 +713,32 @@
 
 static inline void gen_op_mulscc(TCGv dst, TCGv src1, TCGv src2)
 {
-    TCGv r_temp, r_temp2;
+    TCGv r_temp;
     int l1;
 
     l1 = gen_new_label();
     r_temp = tcg_temp_new(TCG_TYPE_TL);
-    r_temp2 = tcg_temp_new(TCG_TYPE_I32);
 
     /* old op:
     if (!(env->y & 1))
         T1 = 0;
     */
     tcg_gen_mov_tl(cpu_cc_src, src1);
-    tcg_gen_ld32u_tl(r_temp, cpu_env, offsetof(CPUSPARCState, y));
-    tcg_gen_trunc_tl_i32(r_temp2, r_temp);
-    tcg_gen_andi_i32(r_temp2, r_temp2, 0x1);
+    tcg_gen_ld_tl(r_temp, cpu_env, offsetof(CPUSPARCState, y));
+    tcg_gen_andi_tl(r_temp, r_temp, 0x1);
     tcg_gen_mov_tl(cpu_cc_src2, src2);
-    tcg_gen_brcondi_i32(TCG_COND_NE, r_temp2, 0, l1);
+    tcg_gen_brcondi_tl(TCG_COND_NE, r_temp, 0, l1);
     tcg_gen_movi_tl(cpu_cc_src2, 0);
     gen_set_label(l1);
 
     // b2 = T0 & 1;
     // env->y = (b2 << 31) | (env->y >> 1);
-    tcg_gen_trunc_tl_i32(r_temp2, cpu_cc_src);
-    tcg_gen_andi_i32(r_temp2, r_temp2, 0x1);
-    tcg_gen_shli_i32(r_temp2, r_temp2, 31);
-    tcg_gen_ld_i32(cpu_tmp32, cpu_env, offsetof(CPUSPARCState, y));
-    tcg_gen_shri_i32(cpu_tmp32, cpu_tmp32, 1);
-    tcg_gen_or_i32(cpu_tmp32, cpu_tmp32, r_temp2);
-    tcg_temp_free(r_temp2);
-    tcg_gen_st_i32(cpu_tmp32, cpu_env, offsetof(CPUSPARCState, y));
+    tcg_gen_andi_tl(r_temp, cpu_cc_src, 0x1);
+    tcg_gen_shli_tl(r_temp, r_temp, 31);
+    tcg_gen_ld_tl(cpu_tmp0, cpu_env, offsetof(CPUSPARCState, y));
+    tcg_gen_shri_tl(cpu_tmp0, cpu_tmp0, 1);
+    tcg_gen_or_tl(cpu_tmp0, cpu_tmp0, r_temp);
+    tcg_gen_st_tl(cpu_tmp0, cpu_env, offsetof(CPUSPARCState, y));
 
     // b1 = N ^ V;
     gen_mov_reg_N(cpu_tmp0, cpu_psr);
@@ -778,9 +774,10 @@
     tcg_gen_mul_i64(r_temp2, r_temp, r_temp2);
 
     tcg_gen_shri_i64(r_temp, r_temp2, 32);
-    tcg_gen_trunc_i64_i32(r_temp, r_temp);
-    tcg_gen_st_i32(r_temp, cpu_env, offsetof(CPUSPARCState, y));
+    tcg_gen_trunc_i64_tl(cpu_tmp0, r_temp);
     tcg_temp_free(r_temp);
+    tcg_gen_andi_tl(cpu_tmp0, cpu_tmp0, 0xffffffff);
+    tcg_gen_st_tl(cpu_tmp0, cpu_env, offsetof(CPUSPARCState, y));
 #ifdef TARGET_SPARC64
     tcg_gen_mov_i64(dst, r_temp2);
 #else
@@ -801,9 +798,10 @@
     tcg_gen_mul_i64(r_temp2, r_temp, r_temp2);
 
     tcg_gen_shri_i64(r_temp, r_temp2, 32);
-    tcg_gen_trunc_i64_i32(r_temp, r_temp);
-    tcg_gen_st_i32(r_temp, cpu_env, offsetof(CPUSPARCState, y));
+    tcg_gen_trunc_i64_tl(cpu_tmp0, r_temp);
     tcg_temp_free(r_temp);
+    tcg_gen_andi_tl(cpu_tmp0, cpu_tmp0, 0xffffffff);
+    tcg_gen_st_tl(cpu_tmp0, cpu_env, offsetof(CPUSPARCState, y));
 #ifdef TARGET_SPARC64
     tcg_gen_mov_i64(dst, r_temp2);
 #else

Re: [Qemu-devel] [5123] Fix y register loads and stores

[Vince Weaver]

On Mon, 1 Sep 2008, Blue Swirl wrote:
> Revision: 5123
>          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5123
> Author:   blueswir1
> Date:     2008-09-01 19:35:29 +0000 (Mon, 01 Sep 2008)

My sample test case still fails (both on x86_64 and x86) even with this 
patch.

I think something is still odd with the 64-bit multiply code (the rest of 
the tcg instructions seem to work fine).

I'll investigate further.

Vince

Re: [Qemu-devel] [5123] Fix y register loads and stores

[Vince Weaver]

> I think something is still odd with the 64-bit multiply code (the rest of the 
> tcg instructions seem to work fine).

I found the problem (this required writing some x86_64 assembly routines 
with pasted in values from out_asm results).

The problem is that my test case does a signed multiply of
    0x386d4380 x 0xc22e4507

These should both be sign extended, but for some reason despite the

     tcg_gen_ext_tl_i64(r_temp, src2);
     tcg_gen_ext_tl_i64(r_temp2, src1);

calls in gen_op_smul() the values aren't sign extended.  This matters for 
the second operand, and thus we get the wrong result.

It wasn't easy to see this with the remote-gdb stuff because annoyingly I 
can't get gdb to display the 64-bit register contents when using 
sparcv8plus binaries.

So something is wrong with the sign extension.  Unfortunately I won't have 
time to look into this more until tomorrow, but I wanted to just get this 
out there.

Vince

Re: [Qemu-devel] [5123] Fix y register loads and stores

[Vince Weaver]

> So something is wrong with the sign extension.  Unfortunately I won't have 
> time to look into this more until tomorrow, but I wanted to just get this out 
> there.

I did have time to look at it in the end.

Here's a patch that fixes my test case.  I'll have to look at the sparcv9 
manual closer to make sure it's the right thing to do.

Vince

--- ./target-sparc/translate.c.orig	2008-09-01 19:03:10.000000000 -0400
+++ ./target-sparc/translate.c	2008-09-01 19:02:54.000000000 -0400
@@ -793,8 +793,8 @@
      r_temp = tcg_temp_new(TCG_TYPE_I64);
      r_temp2 = tcg_temp_new(TCG_TYPE_I64);

-    tcg_gen_ext_tl_i64(r_temp, src2);
-    tcg_gen_ext_tl_i64(r_temp2, src1);
+    tcg_gen_ext_i32_i64(r_temp, src2);
+    tcg_gen_ext_i32_i64(r_temp2, src1);
      tcg_gen_mul_i64(r_temp2, r_temp, r_temp2);

      tcg_gen_shri_i64(r_temp, r_temp2, 32);

Re: [Qemu-devel] [5123] Fix y register loads and stores

[Blue Swirl]

On 9/2/08, Vince Weaver <vince@csl.cornell.edu> wrote:
>
>
> > So something is wrong with the sign extension.  Unfortunately I won't have
> time to look into this more until tomorrow, but I wanted to just get this
> out there.
> >
>
>  I did have time to look at it in the end.
>
>  Here's a patch that fixes my test case.  I'll have to look at the sparcv9
> manual closer to make sure it's the right thing to do.
>
>  Vince
>
>  --- ./target-sparc/translate.c.orig     2008-09-01
> 19:03:10.000000000 -0400
>  +++ ./target-sparc/translate.c  2008-09-01 19:02:54.000000000 -0400
>  @@ -793,8 +793,8 @@
>      r_temp = tcg_temp_new(TCG_TYPE_I64);
>      r_temp2 = tcg_temp_new(TCG_TYPE_I64);
>
>  -    tcg_gen_ext_tl_i64(r_temp, src2);
>  -    tcg_gen_ext_tl_i64(r_temp2, src1);
>  +    tcg_gen_ext_i32_i64(r_temp, src2);
>  +    tcg_gen_ext_i32_i64(r_temp2, src1);
>
>      tcg_gen_mul_i64(r_temp2, r_temp, r_temp2);
>
>      tcg_gen_shri_i64(r_temp, r_temp2, 32);

Thanks, applied to both umul and smul.