qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [5645] target-alpha: fix locked loads/stores
@ 2008-11-07 14:00 Aurelien Jarno
  2008-11-10 17:07 ` Krumme, Chris
  0 siblings, 1 reply; 4+ messages in thread
From: Aurelien Jarno @ 2008-11-07 14:00 UTC (permalink / raw)
  To: qemu-devel

Revision: 5645
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5645
Author:   aurel32
Date:     2008-11-07 14:00:24 +0000 (Fri, 07 Nov 2008)

Log Message:
-----------
target-alpha: fix locked loads/stores

Fix reading of cpu_lock in gen_qemu_stql_c, original patch from Laurent
Desnogues.

A new flag was added to gen_store_mem to allocate local temps instead
of temps;  this flag should be set when the tcg_gen_qemu_store callback
uses brcond before using the temps or else liveness analysis will get
rid of the temps.

This also adds lock printing in cpu_dump_state which can help
debug.

Modified Paths:
--------------
    trunk/target-alpha/helper.c
    trunk/target-alpha/translate.c

Modified: trunk/target-alpha/helper.c
===================================================================
--- trunk/target-alpha/helper.c	2008-11-07 13:48:25 UTC (rev 5644)
+++ trunk/target-alpha/helper.c	2008-11-07 14:00:24 UTC (rev 5645)
@@ -434,5 +434,6 @@
         if ((i % 3) == 2)
             cpu_fprintf(f, "\n");
     }
+    cpu_fprintf(f, "\nlock     " TARGET_FMT_lx "\n", env->lock);
 }
 

Modified: trunk/target-alpha/translate.c
===================================================================
--- trunk/target-alpha/translate.c	2008-11-07 13:48:25 UTC (rev 5644)
+++ trunk/target-alpha/translate.c	2008-11-07 14:00:24 UTC (rev 5645)
@@ -234,9 +234,13 @@
 static always_inline void gen_store_mem (DisasContext *ctx,
                                          void (*tcg_gen_qemu_store)(TCGv t0, TCGv t1, int flags),
                                          int ra, int rb, int32_t disp16,
-                                         int fp, int clear)
+                                         int fp, int clear, int local)
 {
     TCGv addr = tcg_temp_new(TCG_TYPE_I64);
+    if (local)
+        addr = tcg_temp_local_new(TCG_TYPE_I64);
+    else
+        addr = tcg_temp_new(TCG_TYPE_I64);
     if (rb != 31) {
         tcg_gen_addi_i64(addr, cpu_ir[rb], disp16);
         if (clear)
@@ -252,7 +256,11 @@
         else
             tcg_gen_qemu_store(cpu_ir[ra], addr, ctx->mem_idx);
     } else {
-        TCGv zero = tcg_const_i64(0);
+        TCGv zero;
+        if (local)
+            zero = tcg_const_local_i64(0);
+        else
+            zero = tcg_const_i64(0);
         tcg_gen_qemu_store(zero, addr, ctx->mem_idx);
         tcg_temp_free(zero);
     }
@@ -636,15 +644,15 @@
         break;
     case 0x0D:
         /* STW */
-        gen_store_mem(ctx, &tcg_gen_qemu_st16, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &tcg_gen_qemu_st16, ra, rb, disp16, 0, 0, 0);
         break;
     case 0x0E:
         /* STB */
-        gen_store_mem(ctx, &tcg_gen_qemu_st8, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &tcg_gen_qemu_st8, ra, rb, disp16, 0, 0, 0);
         break;
     case 0x0F:
         /* STQ_U */
-        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 1);
+        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 1, 0);
         break;
     case 0x10:
         switch (fn7) {
@@ -2090,19 +2098,19 @@
         break;
     case 0x24:
         /* STF */
-        gen_store_mem(ctx, &gen_qemu_stf, ra, rb, disp16, 1, 0);
+        gen_store_mem(ctx, &gen_qemu_stf, ra, rb, disp16, 1, 0, 0);
         break;
     case 0x25:
         /* STG */
-        gen_store_mem(ctx, &gen_qemu_stg, ra, rb, disp16, 1, 0);
+        gen_store_mem(ctx, &gen_qemu_stg, ra, rb, disp16, 1, 0, 0);
         break;
     case 0x26:
         /* STS */
-        gen_store_mem(ctx, &gen_qemu_sts, ra, rb, disp16, 1, 0);
+        gen_store_mem(ctx, &gen_qemu_sts, ra, rb, disp16, 1, 0, 0);
         break;
     case 0x27:
         /* STT */
-        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 1, 0);
+        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 1, 0, 0);
         break;
     case 0x28:
         /* LDL */
@@ -2122,19 +2130,19 @@
         break;
     case 0x2C:
         /* STL */
-        gen_store_mem(ctx, &tcg_gen_qemu_st32, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &tcg_gen_qemu_st32, ra, rb, disp16, 0, 0, 0);
         break;
     case 0x2D:
         /* STQ */
-        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 0, 0);
         break;
     case 0x2E:
         /* STL_C */
-        gen_store_mem(ctx, &gen_qemu_stl_c, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &gen_qemu_stl_c, ra, rb, disp16, 0, 0, 1);
         break;
     case 0x2F:
         /* STQ_C */
-        gen_store_mem(ctx, &gen_qemu_stq_c, ra, rb, disp16, 0, 0);
+        gen_store_mem(ctx, &gen_qemu_stq_c, ra, rb, disp16, 0, 0, 1);
         break;
     case 0x30:
         /* BR */

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [Qemu-devel] [5645] target-alpha: fix locked loads/stores
  2008-11-07 14:00 [Qemu-devel] [5645] target-alpha: fix locked loads/stores Aurelien Jarno
@ 2008-11-10 17:07 ` Krumme, Chris
  2008-11-10 17:18   ` Laurent Desnogues
  2008-11-11 11:30   ` Aurelien Jarno
  0 siblings, 2 replies; 4+ messages in thread
From: Krumme, Chris @ 2008-11-10 17:07 UTC (permalink / raw)
  To: qemu-devel

 

> -----Original Message-----
> From: 
> qemu-devel-bounces+chris.krumme=windriver.com@nongnu.org 
> [mailto:qemu-devel-bounces+chris.krumme=windriver.com@nongnu.o
> rg] On Behalf Of Aurelien Jarno
> Sent: Friday, November 07, 2008 8:01 AM
> To: qemu-devel@nongnu.org
> Subject: [Qemu-devel] [5645] target-alpha: fix locked loads/stores
> 
> Revision: 5645
>           
> http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5645
> Author:   aurel32
> Date:     2008-11-07 14:00:24 +0000 (Fri, 07 Nov 2008)
> 
> Log Message:
> -----------
> target-alpha: fix locked loads/stores
> 
> Fix reading of cpu_lock in gen_qemu_stql_c, original patch 
> from Laurent Desnogues.
> 
> A new flag was added to gen_store_mem to allocate local temps 
> instead of temps;  this flag should be set when the 
> tcg_gen_qemu_store callback uses brcond before using the 
> temps or else liveness analysis will get rid of the temps.
> 
> This also adds lock printing in cpu_dump_state which can help debug.
> 
> Modified Paths:
> --------------
>     trunk/target-alpha/helper.c
>     trunk/target-alpha/translate.c
> 
> Modified: trunk/target-alpha/helper.c
> ===================================================================
> --- trunk/target-alpha/helper.c	2008-11-07 13:48:25 UTC 
> (rev 5644)
> +++ trunk/target-alpha/helper.c	2008-11-07 14:00:24 UTC 
> (rev 5645)
> @@ -434,5 +434,6 @@
>          if ((i % 3) == 2)
>              cpu_fprintf(f, "\n");
>      }
> +    cpu_fprintf(f, "\nlock     " TARGET_FMT_lx "\n", env->lock);
>  }
>  
> 
> Modified: trunk/target-alpha/translate.c 
> ===================================================================
> --- trunk/target-alpha/translate.c	2008-11-07 13:48:25 UTC 
> (rev 5644)
> +++ trunk/target-alpha/translate.c	2008-11-07 14:00:24 UTC 
> (rev 5645)
> @@ -234,9 +234,13 @@
>  static always_inline void gen_store_mem (DisasContext *ctx,
>                                           void 
> (*tcg_gen_qemu_store)(TCGv t0, TCGv t1, int flags),
>                                           int ra, int rb, 
> int32_t disp16,
> -                                         int fp, int clear)
> +                                         int fp, int clear, 
> int local)
>  {
>      TCGv addr = tcg_temp_new(TCG_TYPE_I64);
> +    if (local)
> +        addr = tcg_temp_local_new(TCG_TYPE_I64);
> +    else
> +        addr = tcg_temp_new(TCG_TYPE_I64);

I don't fully understand how these things work, but for the non-local
case you are calling tcg_temp_new twice.

Thanks

Chris

>      if (rb != 31) {
>          tcg_gen_addi_i64(addr, cpu_ir[rb], disp16);
>          if (clear)
> @@ -252,7 +256,11 @@
>          else
>              tcg_gen_qemu_store(cpu_ir[ra], addr, ctx->mem_idx);
>      } else {
> -        TCGv zero = tcg_const_i64(0);
> +        TCGv zero;
> +        if (local)
> +            zero = tcg_const_local_i64(0);
> +        else
> +            zero = tcg_const_i64(0);
>          tcg_gen_qemu_store(zero, addr, ctx->mem_idx);
>          tcg_temp_free(zero);
>      }
> @@ -636,15 +644,15 @@
>          break;
>      case 0x0D:
>          /* STW */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st16, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st16, ra, rb, disp16, 0, 0, 
> + 0);
>          break;
>      case 0x0E:
>          /* STB */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st8, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st8, ra, rb, 
> disp16, 0, 0, 0);
>          break;
>      case 0x0F:
>          /* STQ_U */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 1);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 1, 
> + 0);
>          break;
>      case 0x10:
>          switch (fn7) {
> @@ -2090,19 +2098,19 @@
>          break;
>      case 0x24:
>          /* STF */
> -        gen_store_mem(ctx, &gen_qemu_stf, ra, rb, disp16, 1, 0);
> +        gen_store_mem(ctx, &gen_qemu_stf, ra, rb, disp16, 1, 0, 0);
>          break;
>      case 0x25:
>          /* STG */
> -        gen_store_mem(ctx, &gen_qemu_stg, ra, rb, disp16, 1, 0);
> +        gen_store_mem(ctx, &gen_qemu_stg, ra, rb, disp16, 1, 0, 0);
>          break;
>      case 0x26:
>          /* STS */
> -        gen_store_mem(ctx, &gen_qemu_sts, ra, rb, disp16, 1, 0);
> +        gen_store_mem(ctx, &gen_qemu_sts, ra, rb, disp16, 1, 0, 0);
>          break;
>      case 0x27:
>          /* STT */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 1, 0);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 1, 0, 
> + 0);
>          break;
>      case 0x28:
>          /* LDL */
> @@ -2122,19 +2130,19 @@
>          break;
>      case 0x2C:
>          /* STL */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st32, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st32, ra, rb, disp16, 0, 0, 
> + 0);
>          break;
>      case 0x2D:
>          /* STQ */
> -        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &tcg_gen_qemu_st64, ra, rb, disp16, 0, 0, 
> + 0);
>          break;
>      case 0x2E:
>          /* STL_C */
> -        gen_store_mem(ctx, &gen_qemu_stl_c, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &gen_qemu_stl_c, ra, rb, disp16, 0, 0, 1);
>          break;
>      case 0x2F:
>          /* STQ_C */
> -        gen_store_mem(ctx, &gen_qemu_stq_c, ra, rb, disp16, 0, 0);
> +        gen_store_mem(ctx, &gen_qemu_stq_c, ra, rb, disp16, 0, 0, 1);
>          break;
>      case 0x30:
>          /* BR */
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [5645] target-alpha: fix locked loads/stores
  2008-11-10 17:07 ` Krumme, Chris
@ 2008-11-10 17:18   ` Laurent Desnogues
  2008-11-11 11:30   ` Aurelien Jarno
  1 sibling, 0 replies; 4+ messages in thread
From: Laurent Desnogues @ 2008-11-10 17:18 UTC (permalink / raw)
  To: qemu-devel

On Mon, Nov 10, 2008 at 6:07 PM, Krumme, Chris
<Chris.Krumme@windriver.com> wrote:
>
>>      TCGv addr = tcg_temp_new(TCG_TYPE_I64);
>> +    if (local)
>> +        addr = tcg_temp_local_new(TCG_TYPE_I64);
>> +    else
>> +        addr = tcg_temp_new(TCG_TYPE_I64);
>
> I don't fully understand how these things work, but for the non-local
> case you are calling tcg_temp_new twice.

You are right, the call in the declaration should be removed.


Laurent

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [5645] target-alpha: fix locked loads/stores
  2008-11-10 17:07 ` Krumme, Chris
  2008-11-10 17:18   ` Laurent Desnogues
@ 2008-11-11 11:30   ` Aurelien Jarno
  1 sibling, 0 replies; 4+ messages in thread
From: Aurelien Jarno @ 2008-11-11 11:30 UTC (permalink / raw)
  To: qemu-devel

On Mon, Nov 10, 2008 at 09:07:55AM -0800, Krumme, Chris wrote:
>  
> 
> > -----Original Message-----
> > From: 
> > qemu-devel-bounces+chris.krumme=windriver.com@nongnu.org 
> > [mailto:qemu-devel-bounces+chris.krumme=windriver.com@nongnu.o
> > rg] On Behalf Of Aurelien Jarno
> > Sent: Friday, November 07, 2008 8:01 AM
> > To: qemu-devel@nongnu.org
> > Subject: [Qemu-devel] [5645] target-alpha: fix locked loads/stores
> > 
> > Revision: 5645
> >           
> > http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5645
> > Author:   aurel32
> > Date:     2008-11-07 14:00:24 +0000 (Fri, 07 Nov 2008)
> > 
> > Log Message:
> > -----------
> > target-alpha: fix locked loads/stores
> > 
> > Fix reading of cpu_lock in gen_qemu_stql_c, original patch 
> > from Laurent Desnogues.
> > 
> > A new flag was added to gen_store_mem to allocate local temps 
> > instead of temps;  this flag should be set when the 
> > tcg_gen_qemu_store callback uses brcond before using the 
> > temps or else liveness analysis will get rid of the temps.
> > 
> > This also adds lock printing in cpu_dump_state which can help debug.
> > 
> > Modified Paths:
> > --------------
> >     trunk/target-alpha/helper.c
> >     trunk/target-alpha/translate.c
> > 
> > Modified: trunk/target-alpha/helper.c
> > ===================================================================
> > --- trunk/target-alpha/helper.c	2008-11-07 13:48:25 UTC 
> > (rev 5644)
> > +++ trunk/target-alpha/helper.c	2008-11-07 14:00:24 UTC 
> > (rev 5645)
> > @@ -434,5 +434,6 @@
> >          if ((i % 3) == 2)
> >              cpu_fprintf(f, "\n");
> >      }
> > +    cpu_fprintf(f, "\nlock     " TARGET_FMT_lx "\n", env->lock);
> >  }
> >  
> > 
> > Modified: trunk/target-alpha/translate.c 
> > ===================================================================
> > --- trunk/target-alpha/translate.c	2008-11-07 13:48:25 UTC 
> > (rev 5644)
> > +++ trunk/target-alpha/translate.c	2008-11-07 14:00:24 UTC 
> > (rev 5645)
> > @@ -234,9 +234,13 @@
> >  static always_inline void gen_store_mem (DisasContext *ctx,
> >                                           void 
> > (*tcg_gen_qemu_store)(TCGv t0, TCGv t1, int flags),
> >                                           int ra, int rb, 
> > int32_t disp16,
> > -                                         int fp, int clear)
> > +                                         int fp, int clear, 
> > int local)
> >  {
> >      TCGv addr = tcg_temp_new(TCG_TYPE_I64);
> > +    if (local)
> > +        addr = tcg_temp_local_new(TCG_TYPE_I64);
> > +    else
> > +        addr = tcg_temp_new(TCG_TYPE_I64);
> 
> I don't fully understand how these things work, but for the non-local
> case you are calling tcg_temp_new twice.

Thanks, fixed in r5671.

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-11-11 11:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-07 14:00 [Qemu-devel] [5645] target-alpha: fix locked loads/stores Aurelien Jarno
2008-11-10 17:07 ` Krumme, Chris
2008-11-10 17:18   ` Laurent Desnogues
2008-11-11 11:30   ` Aurelien Jarno

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).