[Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Blue Swirl]

Revision: 5281
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5281
Author:   blueswir1
Date:     2008-09-21 14:49:09 +0000 (Sun, 21 Sep 2008)

Log Message:
-----------
Use the new concat_i32_i64 op for std and stda

Modified Paths:
--------------
    trunk/target-sparc/helper.h
    trunk/target-sparc/op_helper.c
    trunk/target-sparc/translate.c

Modified: trunk/target-sparc/helper.h
===================================================================
--- trunk/target-sparc/helper.h	2008-09-21 13:48:32 UTC (rev 5280)
+++ trunk/target-sparc/helper.h	2008-09-21 14:49:09 UTC (rev 5281)
@@ -44,7 +44,6 @@
 DEF_HELPER(void, helper_flush, (target_ulong addr))
 DEF_HELPER(target_ulong, helper_udiv, (target_ulong a, target_ulong b))
 DEF_HELPER(target_ulong, helper_sdiv, (target_ulong a, target_ulong b))
-DEF_HELPER(uint64_t, helper_pack64, (target_ulong high, target_ulong low))
 DEF_HELPER(void, helper_stdf, (target_ulong addr, int mem_idx))
 DEF_HELPER(void, helper_lddf, (target_ulong addr, int mem_idx))
 DEF_HELPER(void, helper_ldqf, (target_ulong addr, int mem_idx))

Modified: trunk/target-sparc/op_helper.c
===================================================================
--- trunk/target-sparc/op_helper.c	2008-09-21 13:48:32 UTC (rev 5280)
+++ trunk/target-sparc/op_helper.c	2008-09-21 14:49:09 UTC (rev 5281)
@@ -2281,11 +2281,6 @@
     }
 }
 
-uint64_t helper_pack64(target_ulong high, target_ulong low)
-{
-    return ((uint64_t)high << 32) | (uint64_t)(low & 0xffffffff);
-}
-
 void helper_stdf(target_ulong addr, int mem_idx)
 {
     helper_check_align(addr, 7);

Modified: trunk/target-sparc/translate.c
===================================================================
--- trunk/target-sparc/translate.c	2008-09-21 13:48:32 UTC (rev 5280)
+++ trunk/target-sparc/translate.c	2008-09-21 14:49:09 UTC (rev 5281)
@@ -1715,13 +1715,14 @@
 
 static inline void gen_stda_asi(TCGv hi, TCGv addr, int insn, int rd)
 {
-    TCGv r_temp, r_asi, r_size;
+    TCGv r_low, r_asi, r_size;
 
-    r_temp = tcg_temp_new(TCG_TYPE_TL);
-    gen_movl_reg_TN(rd + 1, r_temp);
-    tcg_gen_helper_1_2(helper_pack64, cpu_tmp64, hi,
-                       r_temp);
-    tcg_temp_free(r_temp);
+    gen_movl_reg_TN(rd + 1, cpu_tmp0);
+    r_low = tcg_temp_new(TCG_TYPE_I32);
+    tcg_gen_trunc_tl_i32(r_low, cpu_tmp0);
+    tcg_gen_trunc_tl_i32(cpu_tmp32, hi);
+    tcg_gen_concat_i32_i64(cpu_tmp64, r_low, cpu_tmp32);
+    tcg_temp_free(r_low);
     r_asi = gen_get_asi(insn, addr);
     r_size = tcg_const_i32(8);
     tcg_gen_helper_0_4(helper_st_asi, addr, cpu_tmp64, r_asi, r_size);
@@ -1817,12 +1818,14 @@
 
 static inline void gen_stda_asi(TCGv hi, TCGv addr, int insn, int rd)
 {
-    TCGv r_temp, r_asi, r_size;
+    TCGv r_low, r_asi, r_size;
 
-    r_temp = tcg_temp_new(TCG_TYPE_TL);
-    gen_movl_reg_TN(rd + 1, r_temp);
-    tcg_gen_helper_1_2(helper_pack64, cpu_tmp64, hi, r_temp);
-    tcg_temp_free(r_temp);
+    gen_movl_reg_TN(rd + 1, cpu_tmp0);
+    r_low = tcg_temp_new(TCG_TYPE_I32);
+    tcg_gen_trunc_tl_i32(r_low, cpu_tmp0);
+    tcg_gen_trunc_tl_i32(cpu_tmp32, hi);
+    tcg_gen_concat_i32_i64(cpu_tmp64, r_low, cpu_tmp32);
+    tcg_temp_free(r_low);
     r_asi = tcg_const_i32(GET_FIELD(insn, 19, 26));
     r_size = tcg_const_i32(8);
     tcg_gen_helper_0_4(helper_st_asi, addr, cpu_tmp64, r_asi, r_size);
@@ -4482,10 +4485,11 @@
                         tcg_gen_helper_0_2(helper_check_align, cpu_addr,
                                            r_const); // XXX remove
                         tcg_temp_free(r_const);
-                        r_low = tcg_temp_new(TCG_TYPE_TL);
-                        gen_movl_reg_TN(rd + 1, r_low);
-                        tcg_gen_helper_1_2(helper_pack64, cpu_tmp64, cpu_val,
-                                           r_low);
+                        gen_movl_reg_TN(rd + 1, cpu_tmp0);
+                        r_low = tcg_temp_new(TCG_TYPE_I32);
+                        tcg_gen_trunc_tl_i32(r_low, cpu_tmp0);
+                        tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_val);
+                        tcg_gen_concat_i32_i64(cpu_tmp64, r_low, cpu_tmp32);
                         tcg_temp_free(r_low);
                         tcg_gen_qemu_st64(cpu_tmp64, cpu_addr, dc->mem_idx);
                     }

Re: [Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Paul Brook]

On Sunday 21 September 2008, Blue Swirl wrote:
> +    r_low = tcg_temp_new(TCG_TYPE_I32);
> +    tcg_gen_trunc_tl_i32(r_low, cpu_tmp0);
> +    tcg_gen_trunc_tl_i32(cpu_tmp32, hi);
> +    tcg_gen_concat_i32_i64(cpu_tmp64, r_low, cpu_tmp32);
> +    tcg_temp_free(r_low);

It's more efficient to not use concat_i32_i64 on 64-bit targets.
See patch below. I'll let you decide if you want to apply or ignore it.

Signed-off-by: Paul Brook <paul@codesourcery.com>

Index: target-sparc/helper.h
===================================================================
--- target-sparc/helper.h	(revision 5280)
+++ target-sparc/helper.h	(working copy)
@@ -44,7 +44,6 @@ DEF_HELPER(void, helper_restore, (void))
 DEF_HELPER(void, helper_flush, (target_ulong addr))
 DEF_HELPER(target_ulong, helper_udiv, (target_ulong a, target_ulong b))
 DEF_HELPER(target_ulong, helper_sdiv, (target_ulong a, target_ulong b))
-DEF_HELPER(uint64_t, helper_pack64, (target_ulong high, target_ulong low))
 DEF_HELPER(void, helper_stdf, (target_ulong addr, int mem_idx))
 DEF_HELPER(void, helper_lddf, (target_ulong addr, int mem_idx))
 DEF_HELPER(void, helper_ldqf, (target_ulong addr, int mem_idx))
Index: target-sparc/op_helper.c
===================================================================
--- target-sparc/op_helper.c	(revision 5280)
+++ target-sparc/op_helper.c	(working copy)
@@ -2281,11 +2281,6 @@ target_ulong helper_sdiv(target_ulong a,
     }
 }
 
-uint64_t helper_pack64(target_ulong high, target_ulong low)
-{
-    return ((uint64_t)high << 32) | (uint64_t)(low & 0xffffffff);
-}
-
 void helper_stdf(target_ulong addr, int mem_idx)
 {
     helper_check_align(addr, 7);
Index: target-sparc/translate.c
===================================================================
--- target-sparc/translate.c	(revision 5280)
+++ target-sparc/translate.c	(working copy)
@@ -1619,6 +1619,31 @@ static inline void gen_clear_float_excep
     tcg_gen_helper_0_0(helper_clear_float_exceptions);
 }
 
+/* May clobber low.  */
+static void gen_pack64(TCGv dest, TCGv high, TCGv low)
+{
+#ifdef TARGET_SPARC64
+#if HOST_LONG_BITS == 64
+    tcg_gen_shli_i64(dest, high, 32);
+    tcg_gen_ext32u_i64(low, low);
+    tcg_gen_or_i64(dest, dest, low);
+# else
+    /* We could omit the truncation if we assumed that 64-bit TCG variables
+       can be used in 32-bit ops.  Hopefully TCG will be smart enough to
+       eliminate the copy.  */
+    TCGv low32 = tcg_temp_new(TCG_TYPE_I32);
+    TCGv high32 = tcg_temp_new(TCG_TYPE_I32);
+    tcg_gen_trunc_i64_i32(low32, low);
+    tcg_gen_trunc_i64_i32(high32, high);
+    tcg_gen_concat_i32_i64(dest, low32, high32);
+    tcg_temp_free(high32);
+    tcg_temp_free(low32);
+# endif
+#else /* !TARGET_SPARC64 */
+    tcg_gen_concat_i32_i64(dest, low, high);
+#endif
+}
+
 /* asi moves */
 #ifdef TARGET_SPARC64
 static inline TCGv gen_get_asi(int insn, TCGv r_addr)
@@ -1719,8 +1744,7 @@ static inline void gen_stda_asi(TCGv hi,
 
     r_temp = tcg_temp_new(TCG_TYPE_TL);
     gen_movl_reg_TN(rd + 1, r_temp);
-    tcg_gen_helper_1_2(helper_pack64, cpu_tmp64, hi,
-                       r_temp);
+    gen_pack64(cpu_tmp64, hi, r_temp);
     tcg_temp_free(r_temp);
     r_asi = gen_get_asi(insn, addr);
     r_size = tcg_const_i32(8);
@@ -1821,7 +1845,7 @@ static inline void gen_stda_asi(TCGv hi,
 
     r_temp = tcg_temp_new(TCG_TYPE_TL);
     gen_movl_reg_TN(rd + 1, r_temp);
-    tcg_gen_helper_1_2(helper_pack64, cpu_tmp64, hi, r_temp);
+    gen_pack64(cpu_tmp64, hi, r_temp);
     tcg_temp_free(r_temp);
     r_asi = tcg_const_i32(GET_FIELD(insn, 19, 26));
     r_size = tcg_const_i32(8);
@@ -4484,8 +4508,7 @@ static void disas_sparc_insn(DisasContex
                         tcg_temp_free(r_const);
                         r_low = tcg_temp_new(TCG_TYPE_TL);
                         gen_movl_reg_TN(rd + 1, r_low);
-                        tcg_gen_helper_1_2(helper_pack64, cpu_tmp64, cpu_val,
-                                           r_low);
+                        gen_pack64(cpu_tmp64, cpu_val, r_low);
                         tcg_temp_free(r_low);
                         tcg_gen_qemu_st64(cpu_tmp64, cpu_addr, dc->mem_idx);
                     }

Re: [Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Blue Swirl]

On 9/21/08, Paul Brook <paul@codesourcery.com> wrote:
> On Sunday 21 September 2008, Blue Swirl wrote:
>  > +    r_low = tcg_temp_new(TCG_TYPE_I32);
>  > +    tcg_gen_trunc_tl_i32(r_low, cpu_tmp0);
>  > +    tcg_gen_trunc_tl_i32(cpu_tmp32, hi);
>  > +    tcg_gen_concat_i32_i64(cpu_tmp64, r_low, cpu_tmp32);
>  > +    tcg_temp_free(r_low);
>
>
> It's more efficient to not use concat_i32_i64 on 64-bit targets.
>  See patch below. I'll let you decide if you want to apply or ignore it.

Good point. How about adding instead a concat_i64_i64 starting from this piece:
>  +    tcg_gen_shli_i64(dest, high, 32);
>  +    tcg_gen_ext32u_i64(low, low);
>  +    tcg_gen_or_i64(dest, dest, low);

and also add defines for concat_tl_i64? That would be both clean and efficient.

I think ldd on i386 would benefit from a reverse operation (64 to two
32/64 bit words).

Re: [Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 895 bytes --]

On 9/21/08, Blue Swirl <blauwirbel@gmail.com> wrote:
> On 9/21/08, Paul Brook <paul@codesourcery.com> wrote:
>  > On Sunday 21 September 2008, Blue Swirl wrote:
>  >  > +    r_low = tcg_temp_new(TCG_TYPE_I32);
>  >  > +    tcg_gen_trunc_tl_i32(r_low, cpu_tmp0);
>  >  > +    tcg_gen_trunc_tl_i32(cpu_tmp32, hi);
>  >  > +    tcg_gen_concat_i32_i64(cpu_tmp64, r_low, cpu_tmp32);
>  >  > +    tcg_temp_free(r_low);
>  >
>  >
>  > It's more efficient to not use concat_i32_i64 on 64-bit targets.
>  >  See patch below. I'll let you decide if you want to apply or ignore it.
>
>
> Good point. How about adding instead a concat_i64_i64 starting from this piece:
>
> >  +    tcg_gen_shli_i64(dest, high, 32);
>  >  +    tcg_gen_ext32u_i64(low, low);
>  >  +    tcg_gen_or_i64(dest, dest, low);
>
>
> and also add defines for concat_tl_i64? That would be both clean and efficient.

Like these patches?

[-- Attachment #2: concat_ops.diff --]
[-- Type: plain/text, Size: 1142 bytes --]

[-- Attachment #3: use_concat_ops.diff --]
[-- Type: plain/text, Size: 2524 bytes --]

Re: [Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Paul Brook]

> > It's more efficient to not use concat_i32_i64 on 64-bit targets.
> >  See patch below. I'll let you decide if you want to apply or ignore it.
>
> Good point. How about adding instead a concat_i64_i64 starting from this
> piece: 

I'd call it concat32_i64 (c.f. extu_i32_i64 v.s. ext32u_i64).

> >  +    tcg_gen_shli_i64(dest, high, 32);
> >  +    tcg_gen_ext32u_i64(low, low);
> >  +    tcg_gen_or_i64(dest, dest, low);
>
> and also add defines for concat_tl_i64? That would be both clean and
> efficient.

Sounds reasonable. The cheat for tl->i64 on 32-bit hosts is ok if it's 
confined to tcg-opc.h.

> I think ldd on i386 would benefit from a reverse operation (64 to two
> 32/64 bit words).

64 to 32 should be much easier for TCG to optimize automatically. It's just a 
a series of copies, and can often be implemented without requiring any extra 
temporaries.  The 32 to 64 case is a trickier because you generally need 
additional temporaries for type correctness and have to propagate all the 
zeros through the 64-bit logical or.

Paul

Re: [Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Paul Brook]

> Like these patches?

> +static inline void tcg_gen_concat_i64_i64(TCGv dest, TCGv low, TCGv high)
> +{
> +    TCGv tmp = tcg_temp_new(TCG_TYPE_I64);
> +    tcg_gen_shli_i64(tmp, high, 32);
> +    tcg_gen_or_i64(dest, low, tmp);
> +    tcg_temp_free(tmp);
> +}

This should use concat_i32_i64 on 32-bit hosts.

Ok with that change, the rename I suggested in my previous mail, and if you 
add documentation to tcg/README.

Paul

Re: [Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

On 9/21/08, Paul Brook <paul@codesourcery.com> wrote:
> > Like these patches?
>
>  > +static inline void tcg_gen_concat_i64_i64(TCGv dest, TCGv low, TCGv high)
>  > +{
>  > +    TCGv tmp = tcg_temp_new(TCG_TYPE_I64);
>  > +    tcg_gen_shli_i64(tmp, high, 32);
>  > +    tcg_gen_or_i64(dest, low, tmp);
>  > +    tcg_temp_free(tmp);
>  > +}
>
>  This should use concat_i32_i64 on 32-bit hosts.
>
>  Ok with that change, the rename I suggested in my previous mail, and if you
>  add documentation to tcg/README.

Updated. I'll run a couple of tests.

[-- Attachment #2: concat_ops.diff --]
[-- Type: plain/text, Size: 1737 bytes --]

[-- Attachment #3: use_concat_ops.diff --]
[-- Type: plain/text, Size: 2524 bytes --]

Re: [Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Thiemo Seufer]

Blue Swirl wrote:
> On 9/21/08, Paul Brook <paul@codesourcery.com> wrote:
> > > Like these patches?
> >
> >  > +static inline void tcg_gen_concat_i64_i64(TCGv dest, TCGv low, TCGv high)
> >  > +{
> >  > +    TCGv tmp = tcg_temp_new(TCG_TYPE_I64);
> >  > +    tcg_gen_shli_i64(tmp, high, 32);
> >  > +    tcg_gen_or_i64(dest, low, tmp);
> >  > +    tcg_temp_free(tmp);
> >  > +}
> >
> >  This should use concat_i32_i64 on 32-bit hosts.
> >
> >  Ok with that change, the rename I suggested in my previous mail, and if you
> >  add documentation to tcg/README.
> 
> Updated. I'll run a couple of tests.

I noticed I could also use the complement ("split"?) to those
instructions in the mips backend. Maybe the same is true for
sparc.


Thiemo

Re: [Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Blue Swirl]

On 9/22/08, Thiemo Seufer <ths@networkno.de> wrote:
> Blue Swirl wrote:
>  > On 9/21/08, Paul Brook <paul@codesourcery.com> wrote:
>  > > > Like these patches?
>  > >
>  > >  > +static inline void tcg_gen_concat_i64_i64(TCGv dest, TCGv low, TCGv high)
>  > >  > +{
>  > >  > +    TCGv tmp = tcg_temp_new(TCG_TYPE_I64);
>  > >  > +    tcg_gen_shli_i64(tmp, high, 32);
>  > >  > +    tcg_gen_or_i64(dest, low, tmp);
>  > >  > +    tcg_temp_free(tmp);
>  > >  > +}
>  > >
>  > >  This should use concat_i32_i64 on 32-bit hosts.
>  > >
>  > >  Ok with that change, the rename I suggested in my previous mail, and if you
>  > >  add documentation to tcg/README.
>  >
>  > Updated. I'll run a couple of tests.
>
>
> I noticed I could also use the complement ("split"?) to those
>  instructions in the mips backend. Maybe the same is true for
>  sparc.

Currently I'm using the following:
                        tcg_gen_trunc_i64_tl(cpu_tmp0, cpu_tmp64);
                        tcg_gen_andi_tl(cpu_tmp0, cpu_tmp0, 0xffffffffULL);
                        gen_movl_TN_reg(rd + 1, cpu_tmp0);
                        tcg_gen_shri_i64(cpu_tmp64, cpu_tmp64, 32);
                        tcg_gen_trunc_i64_tl(cpu_val, cpu_tmp64);
                        tcg_gen_andi_tl(cpu_val, cpu_val, 0xffffffffULL);

On a 32 bit host a much more efficient method could be used if wrapped in an op.

Re: [Qemu-devel] [5281] Use the new concat_i32_i64 op for std and stda

[Paul Brook]

On Monday 22 September 2008, Blue Swirl wrote:
> On 9/22/08, Thiemo Seufer <ths@networkno.de> wrote:
> > Blue Swirl wrote:
> >  > On 9/21/08, Paul Brook <paul@codesourcery.com> wrote:
> >  > > > Like these patches?
> >  > > >
> >  > >  > +static inline void tcg_gen_concat_i64_i64(TCGv dest, TCGv low,
> >  > >  > TCGv high) +{
> >  > >  > +    TCGv tmp = tcg_temp_new(TCG_TYPE_I64);
> >  > >  > +    tcg_gen_shli_i64(tmp, high, 32);
> >  > >  > +    tcg_gen_or_i64(dest, low, tmp);
> >  > >  > +    tcg_temp_free(tmp);
> >  > >  > +}
> >  > >
> >  > >  This should use concat_i32_i64 on 32-bit hosts.
> >  > >
> >  > >  Ok with that change, the rename I suggested in my previous mail,
> >  > > and if you add documentation to tcg/README.
> >  >
> >  > Updated. I'll run a couple of tests.
> >
> > I noticed I could also use the complement ("split"?) to those
> >  instructions in the mips backend. Maybe the same is true for
> >  sparc.
>
> Currently I'm using the following:
>                         tcg_gen_trunc_i64_tl(cpu_tmp0, cpu_tmp64);
>                         tcg_gen_andi_tl(cpu_tmp0, cpu_tmp0, 0xffffffffULL);
>                         gen_movl_TN_reg(rd + 1, cpu_tmp0);
>                         tcg_gen_shri_i64(cpu_tmp64, cpu_tmp64, 32);
>                         tcg_gen_trunc_i64_tl(cpu_val, cpu_tmp64);
>                         tcg_gen_andi_tl(cpu_val, cpu_val, 0xffffffffULL);
>
> On a 32 bit host a much more efficient method could be used if wrapped in
> an op.

Which bits would be more efficient?

Note that the final andi is superfluous.

Paul