Re: [Qemu-devel] Re: [PATCH 0/5] [sh4] patches for SH4 system emulation

[Shin-ichiro KAWASAKI <kawasaki@juno.dti.ne.jp>]

Jan Kiszka wrote:
>> I hope these patches will help and merged in trunk.
>> Any comments will be welcome.
> 
> I cannot comment on your patches, but maybe you can comment on (or test)
> the tas.b issue of qemu's sh4 emulation
> 
> http://comments.gmane.org/gmane.comp.emulators.qemu/27235
> 
> specifically the last proposed solution
> 
> http://permalink.gmane.org/gmane.comp.emulators.qemu/27337

Andrzej's two patches are both seem to be consistent with tas.b
specification.
To make it sure, I've taken out a function, __pthread_spin_trylock(), 
from glibc's linuxthreads/sysdeps/sh/pspinlock.c, as the test code for
tas.b.  I confirmed that current QEMU implementation fails
this test, and whichever of the two patches makes it pass the test. 

In my opinion, the first patch is the better, because it is smaller and
easier to understand.  I add the patch to the end of this mail again,
which is modified to avoid patch failure.
FYI, I add the test code, too.

Regards,
Shin-ichiro KAWASAKI


=== test code starts here ===

#include <stdio.h>

typedef volatile int pthread_spinlock_t;
#define EBUSY 16

int
__pthread_spin_trylock (pthread_spinlock_t *lock)
{
  unsigned int val;

  asm volatile ("tas.b @%1; movt %0"
		: "=r" (val)
		: "r" (lock)
		: "memory");
  return val ? 0 : EBUSY;
}

int main(int argc, char * argv[])
{
  pthread_spinlock_t lock = 0;

  printf("test #1 : ");
  printf(" %s.\n", __pthread_spin_trylock(&lock) ? "NG" : "ok");
  
  printf("test #2 : ");
  printf(" %s.\n", __pthread_spin_trylock(&lock) ? "ok" : "NG");
}

=== test code ends ===


diff -ruwN a/target-sh4/op.c b/target-sh4/op.c
--- a/target-sh4/op.c	2008-08-22 17:57:27.000000000 +0900
+++ b/target-sh4/op.c	2008-08-23 03:42:30.000000000 +0900
@@ -592,13 +592,6 @@
     RETURN();
 }
 
-void OPPROTO op_tasb_rN(void)
-{
-    cond_t((env->gregs[PARAM1] & 0xff) == 0);
-    *(int8_t *) &env->gregs[PARAM1] |= 0x80;
-    RETURN();
-}
-
 void OPPROTO op_movl_T0_rN(void)
 {
     env->gregs[PARAM1] = T0;
diff -ruwN a/target-sh4/translate.c b/target-sh4/translate.c
--- a/target-sh4/translate.c	2008-08-22 17:58:00.000000000 +0900
+++ b/target-sh4/translate.c	2008-08-23 03:42:30.000000000 +0900
@@ -1100,7 +1100,12 @@
 	gen_op_shlr16_Rn(REG(B11_8));
 	return;
     case 0x401b:		/* tas.b @Rn */
-	gen_op_tasb_rN(REG(B11_8));
+        gen_op_movl_rN_T0(REG(B11_8));
+        gen_op_movl_T0_T1();
+        gen_op_ldub_T0_T0(ctx);
+        gen_op_cmp_eq_imm_T0(0);
+        gen_op_or_imm_T0(0x80);
+        gen_op_stb_T0_T1(ctx);
 	return;
     case 0xf00d: /* fsts FPUL,FRn - FPSCR: Nothing */
 	gen_op_movl_fpul_FT0();