Re: [Qemu-devel] [SPARC] Branch condition problems

[Aurelien Jarno]

On Wed, Mar 21, 2007 at 07:42:20PM +0100, Blue Swirl wrote:
> >I have noticed that the branches have some problem on the sparc target
> >in very rare conditions. This happens when a store double instruction
> >(std) is used in the delay slot, as in the following test:
> >
> >  tst  %g0
> >  bne  9b5d8
> >  std  %o2, [ %o1 ]
> >
> >Inserting a nop between bne and std "fixes" the problem.
> >
> >tst %g0 sets the zero flag, so that the branch should never be taked. It
> >happens however that it is sometimes taken. This seems to be due to the
> >fact that T2 holds the result of the condition, and std replace T2 with
> >another value. flush_T2() is called before altering T2, but it does not
> >seems to work.
> >
> >I am currently stuck at that point, I hope somebody who has better
> >understanding of the branch code on Sparc could fix that.
> 
> Nice analysis, thanks. Flush_T2 is probably a misnomer. One solution could 

You basically says that flush_T2() should not be used in this case,
right? 

> be adding a new field to CPU structure for std's (and stda's) use, so that 
> T2 does not need to be used.

>From my tests, it seems that std in a delayed branch slot occurs a
hundred of time during a boot, so not a lot. Adding a new field to the
CPU structure would probably decrease the performances (except on
hosts with a lot of registers). Therefore I am proposing something like
that (currently for std only):

--- target-sparc/translate.c.orig       2007-03-21 02:59:49.000000000 +0100
+++ target-sparc/translate.c    2007-03-21 20:23:21.000000000 +0100
@@ -2454,9 +2454,11 @@
                    gen_op_ldst(sth);
                    break;
                case 0x7:
-                    flush_T2(dc);
-                   gen_movl_reg_T2(rd + 1);
-                   gen_op_ldst(std);
+                   gen_op_ldst(st);
+                   gen_op_movl_T1_im(4);
+                   gen_op_add_T1_T0();
+                   gen_movl_reg_T1(rd + 1);
+                   gen_op_ldst(st);
                    break;
 #if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
                case 0x14:

What do you thing?

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