All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/9] tests: cris: force inlining
@ 2016-09-05 11:54 Rabin Vincent
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 2/9] tests: cris: fix syscall inline asm Rabin Vincent
                   ` (9 more replies)
  0 siblings, 10 replies; 31+ messages in thread
From: Rabin Vincent @ 2016-09-05 11:54 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, Rabin Vincent

From: Rabin Vincent <rabinv@axis.com>

The CRIS tests expect that functions marked inline are always inline.
With newer versions of GCC, building them results warnings like the
following and spurious failures when they are run.

In file included from tests/tcg/cris/check_moveq.c:5:0:
tests/tcg/cris/crisutils.h:66:20: warning: inlining failed in call to
'cris_tst_cc.constprop.0': call is unlikely and code size would grow [-Winline]
tests/tcg/cris/check_moveq.c:28:13: warning: called from here [-Winline]

Use the always_inline attribute when building them to fix this.

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 tests/tcg/cris/sys.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/tcg/cris/sys.h b/tests/tcg/cris/sys.h
index c5f88e1..b1bf4c5 100644
--- a/tests/tcg/cris/sys.h
+++ b/tests/tcg/cris/sys.h
@@ -3,6 +3,8 @@
 #define STRINGIFY(x) #x
 #define TOSTRING(x) STRINGIFY(x)
 
+#define inline inline __attribute__((always_inline))
+
 #define CURRENT_LOCATION __FILE__ ":" TOSTRING(__LINE__)
 
 #define err()                         \
-- 
2.1.4

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

* [Qemu-devel] [PATCH 2/9] tests: cris: fix syscall inline asm
  2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
@ 2016-09-05 11:54 ` Rabin Vincent
  2016-09-05 18:20   ` Richard Henderson
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 3/9] tests: cris: remove openpf4 test Rabin Vincent
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Rabin Vincent @ 2016-09-05 11:54 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, Rabin Vincent

From: Rabin Vincent <rabinv@axis.com>

Add the clobbered registeres to the inline asm for the write and exit
system calls.  Without the correct clobbers for the write() function,
correct failure messages are not printed succesfully on newer version of
GCC.

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 tests/tcg/cris/sys.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/tcg/cris/sys.c b/tests/tcg/cris/sys.c
index 551c5dd..dfa5e1c 100644
--- a/tests/tcg/cris/sys.c
+++ b/tests/tcg/cris/sys.c
@@ -34,7 +34,10 @@ void *memset (void *s, int c, size_t n) {
 
 void exit (int status) {
 	asm volatile ("moveq 1, $r9\n" /* NR_exit.  */
-		      "break 13\n");
+		      "break 13\n"
+		      :
+		      :
+		      : "r9", "memory" );
 	while(1)
 		;
 }
@@ -45,7 +48,10 @@ ssize_t write (int fd, const void *buf, size_t count) {
 	     "move.d %1, $r11\n"
 	     "move.d %2, $r12\n"
 	     "moveq 4, $r9\n" /* NR_write.  */
-	     "break 13\n" : : "r" (fd), "r" (buf), "r" (count) : "memory");
+	     "break 13\n"
+	     :
+	     : "r" (fd), "r" (buf), "r" (count)
+	     : "r9", "r10", "r11", "r12", "memory");
 	asm ("move.d $r10, %0\n" : "=r" (r));
 	return r;
 }
-- 
2.1.4

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

* [Qemu-devel] [PATCH 3/9] tests: cris: remove openpf4 test
  2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 2/9] tests: cris: fix syscall inline asm Rabin Vincent
@ 2016-09-05 11:54 ` Rabin Vincent
  2016-09-12 22:51   ` Edgar E. Iglesias
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 4/9] tests: cris: remove check_time1 Rabin Vincent
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Rabin Vincent @ 2016-09-05 11:54 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, Rabin Vincent

From: Rabin Vincent <rabinv@axis.com>

This test, borrowed from the GDB simulator test suite, is meant to test
the GDB simulator's --sysroot feature and always fails in QEMU.  Remove
it.  openpf3 tests the same sequence of system calls (without assuming
the precence of --sysroot).

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 tests/tcg/cris/Makefile        | 1 -
 tests/tcg/cris/check_openpf4.c | 5 -----
 2 files changed, 6 deletions(-)
 delete mode 100644 tests/tcg/cris/check_openpf4.c

diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile
index d34bfd8..f5230fc 100644
--- a/tests/tcg/cris/Makefile
+++ b/tests/tcg/cris/Makefile
@@ -108,7 +108,6 @@ TESTCASES += check_stat4.ctst
 TESTCASES += check_openpf1.ctst
 TESTCASES += check_openpf2.ctst
 TESTCASES += check_openpf3.ctst
-TESTCASES += check_openpf4.ctst
 TESTCASES += check_openpf5.ctst
 TESTCASES += check_mapbrk.ctst
 TESTCASES += check_mmap1.ctst
diff --git a/tests/tcg/cris/check_openpf4.c b/tests/tcg/cris/check_openpf4.c
deleted file mode 100644
index 8bbee41..0000000
--- a/tests/tcg/cris/check_openpf4.c
+++ /dev/null
@@ -1,5 +0,0 @@
-/* Basic file operations, now *with* sysroot.
-#sim: --sysroot=@exedir@
-*/
-#define PREFIX "/"
-#include "check_openpf3.c"
-- 
2.1.4

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

* [Qemu-devel] [PATCH 4/9] tests: cris: remove check_time1
  2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 2/9] tests: cris: fix syscall inline asm Rabin Vincent
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 3/9] tests: cris: remove openpf4 test Rabin Vincent
@ 2016-09-05 11:54 ` Rabin Vincent
  2016-09-12 23:00   ` Edgar E. Iglesias
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 5/9] target-cris: sync CC state at load/stores Rabin Vincent
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Rabin Vincent @ 2016-09-05 11:54 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, Rabin Vincent

From: Rabin Vincent <rabinv@axis.com>

This test, borrowed from the GDB simulator test suite, checks that every
syscall increments the time returned by gettimeofday() by exactly 1 ms.
This is not guaranteed or even desirable on QEMU so remove this test.

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 tests/tcg/cris/Makefile      |  1 -
 tests/tcg/cris/check_time1.c | 46 --------------------------------------------
 2 files changed, 47 deletions(-)
 delete mode 100644 tests/tcg/cris/check_time1.c

diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile
index f5230fc..14a9eb5 100644
--- a/tests/tcg/cris/Makefile
+++ b/tests/tcg/cris/Makefile
@@ -114,7 +114,6 @@ TESTCASES += check_mmap1.ctst
 TESTCASES += check_mmap2.ctst
 TESTCASES += check_mmap3.ctst
 TESTCASES += check_sigalrm.ctst
-TESTCASES += check_time1.ctst
 TESTCASES += check_time2.ctst
 TESTCASES += check_settls1.ctst
 
diff --git a/tests/tcg/cris/check_time1.c b/tests/tcg/cris/check_time1.c
deleted file mode 100644
index 3fcf0e1..0000000
--- a/tests/tcg/cris/check_time1.c
+++ /dev/null
@@ -1,46 +0,0 @@
-/* Basic time functionality test: check that milliseconds are
-   incremented for each syscall (does not work on host).  */
-#include <stdio.h>
-#include <time.h>
-#include <sys/time.h>
-#include <string.h>
-#include <stdlib.h>
-
-void err (const char *s)
-{
-  perror (s);
-  abort ();
-}
-
-int
-main (void)
-{
-  struct timeval t_m = {0, 0};
-  struct timezone t_z = {0, 0};
-  struct timeval t_m1 = {0, 0};
-  int i;
-
-  if (gettimeofday (&t_m, &t_z) != 0)
-    err ("gettimeofday");
-
-  for (i = 1; i < 10000; i++)
-    if (gettimeofday (&t_m1, NULL) != 0)
-      err ("gettimeofday 1");
-    else
-      if (t_m1.tv_sec * 1000000 + t_m1.tv_usec
-	  != (t_m.tv_sec * 1000000 + t_m.tv_usec + i * 1000))
-	{
-	  fprintf (stderr, "t0 (%ld, %ld), i %d, t1 (%ld, %ld)\n",
-		   t_m.tv_sec, t_m.tv_usec, i, t_m1.tv_sec, t_m1.tv_usec);
-	  abort ();
-	}
-
-  if (time (NULL) != t_m1.tv_sec)
-    {
-      fprintf (stderr, "time != gettod\n");
-      abort ();
-    }
-
-  printf ("pass\n");
-  exit (0);
-}
-- 
2.1.4

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

* [Qemu-devel] [PATCH 5/9] target-cris: sync CC state at load/stores.
  2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
                   ` (2 preceding siblings ...)
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 4/9] tests: cris: remove check_time1 Rabin Vincent
@ 2016-09-05 11:54 ` Rabin Vincent
  2016-09-05 19:02   ` Richard Henderson
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 6/9] target-cris: reduce v32isms from v10 log dumps Rabin Vincent
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Rabin Vincent @ 2016-09-05 11:54 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, Edgar E. Iglesias, Rabin Vincent

From: "Edgar E. Iglesias" <edgar@axis.com>

Icount may choose to abort and recompile a TB at any load/store.  We
need to sync the CC state at these insns.

Signed-off-by: Edgar E. Iglesias <edgar@axis.com>
Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 target-cris/translate.c     | 9 +++++++++
 target-cris/translate_v10.c | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/target-cris/translate.c b/target-cris/translate.c
index f4a8d7d..c280e24 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -1098,6 +1098,9 @@ static void gen_load64(DisasContext *dc, TCGv_i64 dst, TCGv addr)
 {
     int mem_index = cpu_mmu_index(&dc->cpu->env, false);
 
+    /* Due to icount, we need to update the CC flags on load/stores.  */
+    cris_evaluate_flags(dc);
+
     /* If we get a fault on a delayslot we must keep the jmp state in
        the cpu-state to be able to re-execute the jmp.  */
     if (dc->delayed_branch == 1) {
@@ -1112,6 +1115,9 @@ static void gen_load(DisasContext *dc, TCGv dst, TCGv addr,
 {
     int mem_index = cpu_mmu_index(&dc->cpu->env, false);
 
+    /* Due to icount, we need to update the CC flags on load/stores.  */
+    cris_evaluate_flags(dc);
+
     /* If we get a fault on a delayslot we must keep the jmp state in
        the cpu-state to be able to re-execute the jmp.  */
     if (dc->delayed_branch == 1) {
@@ -1127,6 +1133,9 @@ static void gen_store (DisasContext *dc, TCGv addr, TCGv val,
 {
     int mem_index = cpu_mmu_index(&dc->cpu->env, false);
 
+    /* Due to icount, we need to update the CC flags on load/stores.  */
+    cris_evaluate_flags(dc);
+
     /* If we get a fault on a delayslot we must keep the jmp state in
        the cpu-state to be able to re-execute the jmp.  */
     if (dc->delayed_branch == 1) {
diff --git a/target-cris/translate_v10.c b/target-cris/translate_v10.c
index 4707a18..a3da425 100644
--- a/target-cris/translate_v10.c
+++ b/target-cris/translate_v10.c
@@ -99,6 +99,9 @@ static void gen_store_v10(DisasContext *dc, TCGv addr, TCGv val,
 {
     int mem_index = cpu_mmu_index(&dc->cpu->env, false);
 
+    /* Due to icount, we need to update the CC flags on load/stores.  */
+    cris_evaluate_flags(dc);
+
     /* If we get a fault on a delayslot we must keep the jmp state in
        the cpu-state to be able to re-execute the jmp.  */
     if (dc->delayed_branch == 1) {
-- 
2.1.4

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

* [Qemu-devel] [PATCH 6/9] target-cris: reduce v32isms from v10 log dumps
  2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
                   ` (3 preceding siblings ...)
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 5/9] target-cris: sync CC state at load/stores Rabin Vincent
@ 2016-09-05 11:54 ` Rabin Vincent
  2016-09-12 22:59   ` Edgar E. Iglesias
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 7/9] target-cris: ignore prefix insns in singlestep Rabin Vincent
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Rabin Vincent @ 2016-09-05 11:54 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, Hans-Peter Nilsson, Rabin Vincent

From: Hans-Peter Nilsson <hp@axis.com>

Use the correct register names for v10 and don't dump support function
registers for pre-v32.

Signed-off-by: Hans-Peter Nilsson <hp@axis.com>
Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 target-cris/translate.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/target-cris/translate.c b/target-cris/translate.c
index c280e24..a4512b5 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -140,14 +140,14 @@ static void gen_BUG(DisasContext *dc, const char *file, int line)
     cpu_abort(CPU(dc->cpu), "%s:%d\n", file, line);
 }
 
-static const char *regnames[] =
+static const char *regnames_v32[] =
 {
     "$r0", "$r1", "$r2", "$r3",
     "$r4", "$r5", "$r6", "$r7",
     "$r8", "$r9", "$r10", "$r11",
     "$r12", "$r13", "$sp", "$acr",
 };
-static const char *pregnames[] =
+static const char *pregnames_v32[] =
 {
     "$bz", "$vr", "$pid", "$srs",
     "$wz", "$exs", "$eda", "$mof",
@@ -3336,12 +3336,20 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
 {
     CRISCPU *cpu = CRIS_CPU(cs);
     CPUCRISState *env = &cpu->env;
+    const char **regnames;
+    const char **pregnames;
     int i;
-    uint32_t srs;
 
     if (!env || !f) {
         return;
     }
+    if (env->pregs[PR_VR] < 32) {
+        pregnames = pregnames_v10;
+        regnames = regnames_v10;
+    } else {
+        pregnames = pregnames_v32;
+        regnames = regnames_v32;
+    }
 
     cpu_fprintf(f, "PC=%x CCS=%x btaken=%d btarget=%x\n"
             "cc_op=%d cc_src=%d cc_dest=%d cc_result=%x cc_mask=%x\n",
@@ -3363,14 +3371,16 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
             cpu_fprintf(f, "\n");
         }
     }
-    srs = env->pregs[PR_SRS];
-    cpu_fprintf(f, "\nsupport function regs bank %x:\n", srs);
-    if (srs < ARRAY_SIZE(env->sregs)) {
-        for (i = 0; i < 16; i++) {
-            cpu_fprintf(f, "s%2.2d=%8.8x ",
-                    i, env->sregs[srs][i]);
-            if ((i + 1) % 4 == 0) {
-                cpu_fprintf(f, "\n");
+    if (env->pregs[PR_SRS] >= 32) {
+        uint32_t srs = env->pregs[PR_SRS];
+        cpu_fprintf(f, "\nsupport function regs bank %x:\n", srs);
+        if (srs < ARRAY_SIZE(env->sregs)) {
+            for (i = 0; i < 16; i++) {
+                cpu_fprintf(f, "s%2.2d=%8.8x ",
+                        i, env->sregs[srs][i]);
+                if ((i + 1) % 4 == 0) {
+                    cpu_fprintf(f, "\n");
+                }
             }
         }
     }
@@ -3415,12 +3425,12 @@ void cris_initialize_tcg(void)
     for (i = 0; i < 16; i++) {
         cpu_R[i] = tcg_global_mem_new(cpu_env,
                                       offsetof(CPUCRISState, regs[i]),
-                                      regnames[i]);
+                                      regnames_v32[i]);
     }
     for (i = 0; i < 16; i++) {
         cpu_PR[i] = tcg_global_mem_new(cpu_env,
                                        offsetof(CPUCRISState, pregs[i]),
-                                       pregnames[i]);
+                                       pregnames_v32[i]);
     }
 }
 
-- 
2.1.4

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

* [Qemu-devel] [PATCH 7/9] target-cris: ignore prefix insns in singlestep
  2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
                   ` (4 preceding siblings ...)
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 6/9] target-cris: reduce v32isms from v10 log dumps Rabin Vincent
@ 2016-09-05 11:54 ` Rabin Vincent
  2016-09-12 22:49   ` Edgar E. Iglesias
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 8/9] target-cris: add v17 CPU Rabin Vincent
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Rabin Vincent @ 2016-09-05 11:54 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, Hans-Peter Nilsson, Rabin Vincent

From: Hans-Peter Nilsson <hp@axis.com>

Don't count prefix instructions as separate when singlestepping.

For example, for this following instruction

 1ad8:       a011 e00b               move.d r0,[r1-96]

before this patch, we get two register dumps:

 PC=1ad8 CCS=0 btaken=1 btarget=1ac6
 cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
 $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
 $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
 $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
 $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4

 PC=1ada CCS=800 btaken=1 btarget=1ac6
 cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
 $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
 $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
 $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
 $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4

With the patch, we get only one:

 PC=1ad8 CCS=0 btaken=1 btarget=1ac6
 cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
 $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
 $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
 $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
 $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4

Signed-off-by: Hans-Peter Nilsson <hp@axis.com>
Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 target-cris/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-cris/translate.c b/target-cris/translate.c
index a4512b5..c9b1e65 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3262,7 +3262,8 @@ void gen_intermediate_code(CPUCRISState *env, struct TranslationBlock *tb)
         }
     } while (!dc->is_jmp && !dc->cpustate_changed
             && !tcg_op_buf_full()
-            && !singlestep
+            /* We don't count prefix insns as separate wrt. singlestep.  */
+            && (!singlestep || (dc->tb_flags & PFIX_FLAG))
             && (dc->pc < next_page_start)
             && num_insns < max_insns);
 
-- 
2.1.4

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

* [Qemu-devel] [PATCH 8/9] target-cris: add v17 CPU
  2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
                   ` (5 preceding siblings ...)
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 7/9] target-cris: ignore prefix insns in singlestep Rabin Vincent
@ 2016-09-05 11:54 ` Rabin Vincent
  2016-09-12 22:18   ` Edgar E. Iglesias
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 9/9] tests: cris: add v17 ADDC test Rabin Vincent
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 31+ messages in thread
From: Rabin Vincent @ 2016-09-05 11:54 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, Rabin Vincent

From: Rabin Vincent <rabinv@axis.com>

In the CRIS v17 CPU an ADDC (add with carry) instruction has been added
compared to the v10 instruction set.

 Assembler syntax:

  ADDC [Rs],Rd
  ADDC [Rs+],Rd

 Size: Dword

 Description:

  The source data is added together with the carry flag to the
  destination register. The size of the operation is dword.

 Operation:

  Rd += s + C-flag;

 Flags affected:

  S R P U I X N Z V C
  - - - - - 0 * * * *

 Instruction format: ADDC [Rs],Rd

  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
  |Destination(Rd)| 1   0   0   1   1   0   1   0 |   Source(Rs)  |
  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+

 Instruction format: ADDC [Rs+],Rd

  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
  |Destination(Rd)| 1   1   0   1   1   0   1   0 |   Source(Rs)  |
  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 target-cris/cpu.c            | 14 ++++++++++++++
 target-cris/crisv10-decode.h |  1 +
 target-cris/translate_v10.c  |  8 ++++++++
 3 files changed, 23 insertions(+)

diff --git a/target-cris/cpu.c b/target-cris/cpu.c
index c5a656b..d680cfb 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -246,6 +246,16 @@ static void crisv11_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_read_register = crisv10_cpu_gdb_read_register;
 }
 
+static void crisv17_cpu_class_init(ObjectClass *oc, void *data)
+{
+    CPUClass *cc = CPU_CLASS(oc);
+    CRISCPUClass *ccc = CRIS_CPU_CLASS(oc);
+
+    ccc->vr = 17;
+    cc->do_interrupt = crisv10_cpu_do_interrupt;
+    cc->gdb_read_register = crisv10_cpu_gdb_read_register;
+}
+
 static void crisv32_cpu_class_init(ObjectClass *oc, void *data)
 {
     CRISCPUClass *ccc = CRIS_CPU_CLASS(oc);
@@ -273,6 +283,10 @@ static const TypeInfo cris_cpu_model_type_infos[] = {
         .parent = TYPE_CRIS_CPU,
         .class_init = crisv11_cpu_class_init,
     }, {
+        .name = TYPE("crisv17"),
+        .parent = TYPE_CRIS_CPU,
+        .class_init = crisv17_cpu_class_init,
+    }, {
         .name = TYPE("crisv32"),
         .parent = TYPE_CRIS_CPU,
         .class_init = crisv32_cpu_class_init,
diff --git a/target-cris/crisv10-decode.h b/target-cris/crisv10-decode.h
index 587fbdd..bdb4b6d 100644
--- a/target-cris/crisv10-decode.h
+++ b/target-cris/crisv10-decode.h
@@ -92,6 +92,7 @@
 #define CRISV10_IND_JUMP_M       4
 #define CRISV10_IND_DIP          5
 #define CRISV10_IND_JUMP_R       6
+#define CRISV17_IND_ADDC         6
 #define CRISV10_IND_BOUND        7
 #define CRISV10_IND_BCC_M        7
 #define CRISV10_IND_MOVE_M_SPR   8
diff --git a/target-cris/translate_v10.c b/target-cris/translate_v10.c
index a3da425..33d86eb 100644
--- a/target-cris/translate_v10.c
+++ b/target-cris/translate_v10.c
@@ -1097,6 +1097,14 @@ static unsigned int dec10_ind(CPUCRISState *env, DisasContext *dc)
                 insn_len = dec10_bdap_m(env, dc, size);
                 break;
             default:
+                if (dc->opcode == CRISV17_IND_ADDC && dc->size == 2 &&
+                    env->pregs[PR_VR] == 17) {
+                    LOG_DIS("addc op=%d %d\n",  dc->src, dc->dst);
+                    cris_cc_mask(dc, CC_MASK_NZVC);
+                    insn_len += dec10_ind_alu(env, dc, CC_OP_ADDC, size);
+                    break;
+                }
+
                 LOG_DIS("pc=%x var-ind.%d %d r%d r%d\n",
                           dc->pc, size, dc->opcode, dc->src, dc->dst);
                 cpu_abort(CPU(dc->cpu), "Unhandled opcode");
-- 
2.1.4

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

* [Qemu-devel] [PATCH 9/9] tests: cris: add v17 ADDC test
  2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
                   ` (6 preceding siblings ...)
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 8/9] target-cris: add v17 CPU Rabin Vincent
@ 2016-09-05 11:54 ` Rabin Vincent
  2016-09-12 23:16   ` Edgar E. Iglesias
  2016-09-06 21:53 ` [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Edgar E. Iglesias
  2016-09-08 18:06 ` Peter Maydell
  9 siblings, 1 reply; 31+ messages in thread
From: Rabin Vincent @ 2016-09-05 11:54 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, Rabin Vincent

From: Rabin Vincent <rabinv@axis.com>

Add a test for the newly implemented ADDC instruction in the v17 CRIS
CPU.

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 tests/tcg/cris/Makefile        | 19 ++++++++++--
 tests/tcg/cris/check_addcv17.s | 65 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/cris/check_addcv17.s

diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile
index 14a9eb5..6b3dba4 100644
--- a/tests/tcg/cris/Makefile
+++ b/tests/tcg/cris/Makefile
@@ -23,6 +23,7 @@ SYS        = sys.o
 TESTCASES += check_abs.tst
 TESTCASES += check_addc.tst
 TESTCASES += check_addcm.tst
+TESTCASES += check_addcv17.tst
 TESTCASES += check_addo.tst
 TESTCASES += check_addoq.tst
 TESTCASES += check_addi.tst
@@ -134,13 +135,27 @@ all: build
 %.ctst: %.o
 	$(CC) $(CFLAGS) $(LDLIBS) $< -o $@
 
+
+sysv10.o: sys.c
+	$(CC) $(CFLAGS) -mcpu=v10 -c $< -o $@
+
+crtv10.o: crt.s
+	$(AS) $(ASFLAGS) -mcpu=v10 -c $< -o $@
+
+check_addcv17.tst: ASFLAGS += -mcpu=v10
+check_addcv17.tst: CRT := crtv10.o
+check_addcv17.tst: SYS := sysv10.o
+check_addcv17.tst: crtv10.o sysv10.o
+
 build: $(CRT) $(SYS) $(TESTCASES)
 
 check: $(CRT) $(SYS) $(TESTCASES)
 	@echo -e "\nQEMU simulator."
 	for case in $(TESTCASES); do \
 		echo -n "$$case "; \
-		$(SIM) ./$$case; \
+		SIMARGS=; \
+		case $$case in *v17*) SIMARGS="-cpu crisv17";; esac; \
+		$(SIM) $$SIMARGS ./$$case; \
 	done
 check-g: $(CRT) $(SYS) $(TESTCASES)
 	@echo -e "\nGDB simulator."
@@ -150,4 +165,4 @@ check-g: $(CRT) $(SYS) $(TESTCASES)
 	done
 
 clean:
-	$(RM) -fr $(TESTCASES) $(CRT) $(SYS)
+	$(RM) -fr $(TESTCASES) *.o
diff --git a/tests/tcg/cris/check_addcv17.s b/tests/tcg/cris/check_addcv17.s
new file mode 100644
index 0000000..52ef7a9
--- /dev/null
+++ b/tests/tcg/cris/check_addcv17.s
@@ -0,0 +1,65 @@
+# mach:  crisv17
+
+ .include "testutils.inc"
+
+ .macro addc Rs Rd inc=0
+# Create the instruction manually since there is no assembler support yet
+ .word (\Rd << 12) | \Rs | (\inc << 10) | 0x09a0
+ .endm
+
+ start
+
+ .data
+mem1:
+ .dword 0x0
+mem2:
+ .dword 0x12345678
+
+ .text
+ move.d mem1,r4
+ clearf nzvc
+ addc 4 3
+ test_cc 0 1 0 0
+ checkr3 0
+
+ move.d mem1,r4
+ clearf nzvc
+ ax
+ addc 4 3
+ test_cc 0 0 0 0
+ checkr3 0
+
+ move.d mem1,r4
+ clearf nzvc
+ setf c
+ addc 4 3
+ test_cc 0 0 0 0
+ checkr3 1
+
+ move.d mem2,r4
+ moveq 2, r3
+ clearf nzvc
+ setf c
+ addc 4 3
+ test_cc 0 0 0 0
+ checkr3 1234567b
+
+ move.d mem2,r5
+ clearf nzvc
+ cmp.d r4,r5
+ test_cc 0 1 0 0
+
+ move.d mem2,r4
+ moveq 2, r3
+ clearf nzvc
+ addc 4 3 inc=1
+ test_cc 0 0 0 0
+ checkr3 1234567a
+
+ move.d mem2,r5
+ clearf nzvc
+ addq 4,r5
+ cmp.d r4,r5
+ test_cc 0 1 0 0
+
+ quit
-- 
2.1.4

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

* Re: [Qemu-devel] [PATCH 2/9] tests: cris: fix syscall inline asm
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 2/9] tests: cris: fix syscall inline asm Rabin Vincent
@ 2016-09-05 18:20   ` Richard Henderson
  2016-09-08 11:38     ` [Qemu-devel] [PATCHv2 2/8] " Rabin Vincent
  0 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2016-09-05 18:20 UTC (permalink / raw)
  To: Rabin Vincent, edgar.iglesias; +Cc: qemu-devel, Rabin Vincent

On 09/05/2016 04:54 AM, Rabin Vincent wrote:
> From: Rabin Vincent <rabinv@axis.com>
>
> Add the clobbered registeres to the inline asm for the write and exit
> system calls.  Without the correct clobbers for the write() function,
> correct failure messages are not printed succesfully on newer version of
> GCC.
>
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  tests/tcg/cris/sys.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tests/tcg/cris/sys.c b/tests/tcg/cris/sys.c
> index 551c5dd..dfa5e1c 100644
> --- a/tests/tcg/cris/sys.c
> +++ b/tests/tcg/cris/sys.c
> @@ -34,7 +34,10 @@ void *memset (void *s, int c, size_t n) {
>
>  void exit (int status) {
>  	asm volatile ("moveq 1, $r9\n" /* NR_exit.  */
> -		      "break 13\n");
> +		      "break 13\n"
> +		      :
> +		      :
> +		      : "r9", "memory" );

Better to simply set up the arguments properly.  See

http://www.uclibc-ng.org/browser/uclibc-ng/libc/sysdeps/linux/cris/bits/syscalls.h?rev=3b4650a23ff372dde44c264d43af8cfa4ac88e0a


r~

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

* Re: [Qemu-devel] [PATCH 5/9] target-cris: sync CC state at load/stores.
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 5/9] target-cris: sync CC state at load/stores Rabin Vincent
@ 2016-09-05 19:02   ` Richard Henderson
  2016-09-06 21:52     ` Edgar E. Iglesias
  0 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2016-09-05 19:02 UTC (permalink / raw)
  To: Rabin Vincent, edgar.iglesias
  Cc: Edgar E. Iglesias, qemu-devel, Rabin Vincent

On 09/05/2016 04:54 AM, Rabin Vincent wrote:
> From: "Edgar E. Iglesias" <edgar@axis.com>
>
> Icount may choose to abort and recompile a TB at any load/store.  We
> need to sync the CC state at these insns.
>
> Signed-off-by: Edgar E. Iglesias <edgar@axis.com>
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  target-cris/translate.c     | 9 +++++++++
>  target-cris/translate_v10.c | 3 +++
>  2 files changed, 12 insertions(+)
>
> diff --git a/target-cris/translate.c b/target-cris/translate.c
> index f4a8d7d..c280e24 100644
> --- a/target-cris/translate.c
> +++ b/target-cris/translate.c
> @@ -1098,6 +1098,9 @@ static void gen_load64(DisasContext *dc, TCGv_i64 dst, TCGv addr)
>  {
>      int mem_index = cpu_mmu_index(&dc->cpu->env, false);
>
> +    /* Due to icount, we need to update the CC flags on load/stores.  */
> +    cris_evaluate_flags(dc);
> +

This is not the proper way to handle this.  You should arrange to sync the CC 
flags in restore_state_to_opc.  There are plenty of examples in the tree you 
can examine.  It looks like there's plenty of room for cleanup for cris here.


r~

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

* Re: [Qemu-devel] [PATCH 5/9] target-cris: sync CC state at load/stores.
  2016-09-05 19:02   ` Richard Henderson
@ 2016-09-06 21:52     ` Edgar E. Iglesias
  0 siblings, 0 replies; 31+ messages in thread
From: Edgar E. Iglesias @ 2016-09-06 21:52 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Rabin Vincent, Edgar E. Iglesias, qemu-devel, Rabin Vincent

On Mon, Sep 05, 2016 at 12:02:14PM -0700, Richard Henderson wrote:
> On 09/05/2016 04:54 AM, Rabin Vincent wrote:
> >From: "Edgar E. Iglesias" <edgar@axis.com>
> >
> >Icount may choose to abort and recompile a TB at any load/store.  We
> >need to sync the CC state at these insns.
> >
> >Signed-off-by: Edgar E. Iglesias <edgar@axis.com>
> >Signed-off-by: Rabin Vincent <rabinv@axis.com>
> >---
> > target-cris/translate.c     | 9 +++++++++
> > target-cris/translate_v10.c | 3 +++
> > 2 files changed, 12 insertions(+)
> >
> >diff --git a/target-cris/translate.c b/target-cris/translate.c
> >index f4a8d7d..c280e24 100644
> >--- a/target-cris/translate.c
> >+++ b/target-cris/translate.c
> >@@ -1098,6 +1098,9 @@ static void gen_load64(DisasContext *dc, TCGv_i64 dst, TCGv addr)
> > {
> >     int mem_index = cpu_mmu_index(&dc->cpu->env, false);
> >
> >+    /* Due to icount, we need to update the CC flags on load/stores.  */
> >+    cris_evaluate_flags(dc);
> >+
> 
> This is not the proper way to handle this.  You should arrange to sync the
> CC flags in restore_state_to_opc.  There are plenty of examples in the tree
> you can examine.  It looks like there's plenty of room for cleanup for cris
> here.
>

Thanks, agreed.

Rabin, most of the CRIS patches that I left in the AXIS tree without upstreaming
need more work. I don't mind you posting them but keep in mind that there's
not much that can go up without tidy up.

Best regards,
Edgar

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

* Re: [Qemu-devel] [PATCH 1/9] tests: cris: force inlining
  2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
                   ` (7 preceding siblings ...)
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 9/9] tests: cris: add v17 ADDC test Rabin Vincent
@ 2016-09-06 21:53 ` Edgar E. Iglesias
  2016-09-08 11:41   ` Rabin Vincent
  2016-09-08 18:06 ` Peter Maydell
  9 siblings, 1 reply; 31+ messages in thread
From: Edgar E. Iglesias @ 2016-09-06 21:53 UTC (permalink / raw)
  To: Rabin Vincent; +Cc: qemu-devel, Rabin Vincent

On Mon, Sep 05, 2016 at 01:54:04PM +0200, Rabin Vincent wrote:
> From: Rabin Vincent <rabinv@axis.com>
> 
> The CRIS tests expect that functions marked inline are always inline.
> With newer versions of GCC, building them results warnings like the
> following and spurious failures when they are run.
> 
> In file included from tests/tcg/cris/check_moveq.c:5:0:
> tests/tcg/cris/crisutils.h:66:20: warning: inlining failed in call to
> 'cris_tst_cc.constprop.0': call is unlikely and code size would grow [-Winline]
> tests/tcg/cris/check_moveq.c:28:13: warning: called from here [-Winline]
> 
> Use the always_inline attribute when building them to fix this.

Hi Rabin,

Do you happen to have a public git tree/branch with these changes?

Best regards,
Edgar


> 
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  tests/tcg/cris/sys.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/tcg/cris/sys.h b/tests/tcg/cris/sys.h
> index c5f88e1..b1bf4c5 100644
> --- a/tests/tcg/cris/sys.h
> +++ b/tests/tcg/cris/sys.h
> @@ -3,6 +3,8 @@
>  #define STRINGIFY(x) #x
>  #define TOSTRING(x) STRINGIFY(x)
>  
> +#define inline inline __attribute__((always_inline))
> +
>  #define CURRENT_LOCATION __FILE__ ":" TOSTRING(__LINE__)
>  
>  #define err()                         \
> -- 
> 2.1.4
> 

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

* [Qemu-devel] [PATCHv2 2/8] tests: cris: fix syscall inline asm
  2016-09-05 18:20   ` Richard Henderson
@ 2016-09-08 11:38     ` Rabin Vincent
  2016-09-08 16:10       ` Richard Henderson
  0 siblings, 1 reply; 31+ messages in thread
From: Rabin Vincent @ 2016-09-08 11:38 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: rth, qemu-devel, Rabin Vincent

From: Rabin Vincent <rabinv@axis.com>

Add the appropriate register constraints for the inline asm for the
write and exit system calls.  Without the correct constraints for the
write() function, correct failure messages are not printed succesfully
on newer version of GCC.

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
v2: reworked to set up arguments correctly

 tests/tcg/cris/sys.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/tests/tcg/cris/sys.c b/tests/tcg/cris/sys.c
index 551c5dd..21f08c0 100644
--- a/tests/tcg/cris/sys.c
+++ b/tests/tcg/cris/sys.c
@@ -33,19 +33,27 @@ void *memset (void *s, int c, size_t n) {
 }
 
 void exit (int status) {
-	asm volatile ("moveq 1, $r9\n" /* NR_exit.  */
-		      "break 13\n");
+	register unsigned int callno asm ("r9") = 1; /* NR_exit */
+
+	asm volatile ("break 13\n"
+		      :
+		      : "r" (callno)
+		      : "memory" );
 	while(1)
 		;
 }
 
 ssize_t write (int fd, const void *buf, size_t count) {
-	int r;
-	asm ("move.d %0, $r10\n"
-	     "move.d %1, $r11\n"
-	     "move.d %2, $r12\n"
-	     "moveq 4, $r9\n" /* NR_write.  */
-	     "break 13\n" : : "r" (fd), "r" (buf), "r" (count) : "memory");
-	asm ("move.d $r10, %0\n" : "=r" (r));
+	register unsigned int callno asm ("r9") = 4; /* NR_write */
+	register unsigned int r10 asm ("r10") = fd;
+	register const void *r11 asm ("r11") = buf;
+	register size_t r12 asm ("r12") = count;
+	register unsigned int r asm ("r10");
+
+	asm volatile ("break 13\n"
+	     : "=r" (r)
+	     : "r" (callno), "0" (r10), "r" (r11), "r" (r12)
+	     : "memory");
+
 	return r;
 }
-- 
2.1.4

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

* Re: [Qemu-devel] [PATCH 1/9] tests: cris: force inlining
  2016-09-06 21:53 ` [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Edgar E. Iglesias
@ 2016-09-08 11:41   ` Rabin Vincent
  0 siblings, 0 replies; 31+ messages in thread
From: Rabin Vincent @ 2016-09-08 11:41 UTC (permalink / raw)
  To: Edgar E. Iglesias; +Cc: qemu-devel

On Tue, Sep 06, 2016 at 11:53:46PM +0200, Edgar E. Iglesias wrote:
> On Mon, Sep 05, 2016 at 01:54:04PM +0200, Rabin Vincent wrote:
> > From: Rabin Vincent <rabinv@axis.com>
> > 
> > The CRIS tests expect that functions marked inline are always inline.
> > With newer versions of GCC, building them results warnings like the
> > following and spurious failures when they are run.
> > 
> > In file included from tests/tcg/cris/check_moveq.c:5:0:
> > tests/tcg/cris/crisutils.h:66:20: warning: inlining failed in call to
> > 'cris_tst_cc.constprop.0': call is unlikely and code size would grow [-Winline]
> > tests/tcg/cris/check_moveq.c:28:13: warning: called from here [-Winline]
> > 
> > Use the always_inline attribute when building them to fix this.
> 
> Hi Rabin,
> 
> Do you happen to have a public git tree/branch with these changes?

I've dropped the "sync CC state" patch for now and pushed the rest up to
the cris branch of https://github.com/rabinv/qemu/.

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

* Re: [Qemu-devel] [PATCHv2 2/8] tests: cris: fix syscall inline asm
  2016-09-08 11:38     ` [Qemu-devel] [PATCHv2 2/8] " Rabin Vincent
@ 2016-09-08 16:10       ` Richard Henderson
  0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2016-09-08 16:10 UTC (permalink / raw)
  To: Rabin Vincent, edgar.iglesias; +Cc: qemu-devel, Rabin Vincent

On 09/08/2016 04:38 AM, Rabin Vincent wrote:
> From: Rabin Vincent <rabinv@axis.com>
> 
> Add the appropriate register constraints for the inline asm for the
> write and exit system calls.  Without the correct constraints for the
> write() function, correct failure messages are not printed succesfully
> on newer version of GCC.
> 
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
> v2: reworked to set up arguments correctly
> 
>  tests/tcg/cris/sys.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

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

* Re: [Qemu-devel] [PATCH 1/9] tests: cris: force inlining
  2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
                   ` (8 preceding siblings ...)
  2016-09-06 21:53 ` [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Edgar E. Iglesias
@ 2016-09-08 18:06 ` Peter Maydell
  2016-09-08 18:21   ` Eric Blake
  2016-09-12  9:43   ` [Qemu-devel] [PATCHv2 1/8] " Rabin Vincent
  9 siblings, 2 replies; 31+ messages in thread
From: Peter Maydell @ 2016-09-08 18:06 UTC (permalink / raw)
  To: Rabin Vincent; +Cc: Edgar E. Iglesias, QEMU Developers, Rabin Vincent

On 5 September 2016 at 12:54, Rabin Vincent <rabin.vincent@axis.com> wrote:
> From: Rabin Vincent <rabinv@axis.com>
>
> The CRIS tests expect that functions marked inline are always inline.
> With newer versions of GCC, building them results warnings like the
> following and spurious failures when they are run.
>
> In file included from tests/tcg/cris/check_moveq.c:5:0:
> tests/tcg/cris/crisutils.h:66:20: warning: inlining failed in call to
> 'cris_tst_cc.constprop.0': call is unlikely and code size would grow [-Winline]
> tests/tcg/cris/check_moveq.c:28:13: warning: called from here [-Winline]
>
> Use the always_inline attribute when building them to fix this.

This test code is pretty horrific; it relies on a whole bunch
of stuff that the compiler doesn't actually guarantee you,
and using always-inline is just a bandaid over the real
problem (which is that it should really be using multi-insn
asm statements when it cares about order, and not assuming
it can put two asm statements next to each other and set
cflags in one and read them in another).

That said, you probably aren't interested in rewriting it, so:

> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  tests/tcg/cris/sys.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tests/tcg/cris/sys.h b/tests/tcg/cris/sys.h
> index c5f88e1..b1bf4c5 100644
> --- a/tests/tcg/cris/sys.h
> +++ b/tests/tcg/cris/sys.h
> @@ -3,6 +3,8 @@
>  #define STRINGIFY(x) #x
>  #define TOSTRING(x) STRINGIFY(x)
>
> +#define inline inline __attribute__((always_inline))
> +

I think redefining C keywords is generally a bad idea.
Can you instead define an "always_inline" macro and
use it where necessary?

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH 1/9] tests: cris: force inlining
  2016-09-08 18:06 ` Peter Maydell
@ 2016-09-08 18:21   ` Eric Blake
  2016-09-12  9:43   ` [Qemu-devel] [PATCHv2 1/8] " Rabin Vincent
  1 sibling, 0 replies; 31+ messages in thread
From: Eric Blake @ 2016-09-08 18:21 UTC (permalink / raw)
  To: Peter Maydell, Rabin Vincent
  Cc: Edgar E. Iglesias, QEMU Developers, Rabin Vincent

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

On 09/08/2016 01:06 PM, Peter Maydell wrote:

>> +++ b/tests/tcg/cris/sys.h
>> @@ -3,6 +3,8 @@
>>  #define STRINGIFY(x) #x
>>  #define TOSTRING(x) STRINGIFY(x)
>>
>> +#define inline inline __attribute__((always_inline))
>> +
> 
> I think redefining C keywords is generally a bad idea.
> Can you instead define an "always_inline" macro and
> use it where necessary?

In fact, commit b11d029b is an instance where compilation failed weirdly
because we redefined inline. I agree that a separate macro name, added
on where desired, rather than redefining 'inline' itself, is desirable.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* [Qemu-devel] [PATCHv2 1/8] tests: cris: force inlining
  2016-09-08 18:06 ` Peter Maydell
  2016-09-08 18:21   ` Eric Blake
@ 2016-09-12  9:43   ` Rabin Vincent
  1 sibling, 0 replies; 31+ messages in thread
From: Rabin Vincent @ 2016-09-12  9:43 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: peter.maydell, qemu-devel, Rabin Vincent

From: Rabin Vincent <rabinv@axis.com>

The CRIS tests expect that functions marked inline are always inline.
With newer versions of GCC, building them results warnings like the
following and spurious failures when they are run.

In file included from tests/tcg/cris/check_moveq.c:5:0:
tests/tcg/cris/crisutils.h:66:20: warning: inlining failed in call to
'cris_tst_cc.constprop.0': call is unlikely and code size would grow [-Winline]
tests/tcg/cris/check_moveq.c:28:13: warning: called from here [-Winline]

Use the always_inline attribute when building them to fix this.

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
v2: don't redefine inline

 tests/tcg/cris/check_abs.c   |  4 ++--
 tests/tcg/cris/check_addc.c  |  2 +-
 tests/tcg/cris/check_addcm.c |  4 ++--
 tests/tcg/cris/check_bound.c |  6 +++---
 tests/tcg/cris/check_ftag.c  |  8 ++++----
 tests/tcg/cris/check_int64.c |  4 ++--
 tests/tcg/cris/check_lz.c    |  2 +-
 tests/tcg/cris/check_swap.c  |  2 +-
 tests/tcg/cris/crisutils.h   | 20 ++++++++++----------
 tests/tcg/cris/sys.h         |  2 ++
 10 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/tests/tcg/cris/check_abs.c b/tests/tcg/cris/check_abs.c
index 9770a8d..08b67b6 100644
--- a/tests/tcg/cris/check_abs.c
+++ b/tests/tcg/cris/check_abs.c
@@ -4,14 +4,14 @@
 #include "sys.h"
 #include "crisutils.h"
 
-static inline int cris_abs(int n)
+static always_inline int cris_abs(int n)
 {
 	int r;
 	asm ("abs\t%1, %0\n" : "=r" (r) : "r" (n));
 	return r;
 }
 
-static inline void
+static always_inline void
 verify_abs(int val, int res,
 	   const int n, const int z, const int v, const int c)
 {
diff --git a/tests/tcg/cris/check_addc.c b/tests/tcg/cris/check_addc.c
index facd1be..fc3fb1f 100644
--- a/tests/tcg/cris/check_addc.c
+++ b/tests/tcg/cris/check_addc.c
@@ -4,7 +4,7 @@
 #include "sys.h"
 #include "crisutils.h"
 
-static inline int cris_addc(int a, const int b)
+static always_inline int cris_addc(int a, const int b)
 {
 	asm ("addc\t%1, %0\n" : "+r" (a) : "r" (b));
 	return a;
diff --git a/tests/tcg/cris/check_addcm.c b/tests/tcg/cris/check_addcm.c
index 7928bc9..b355ba1 100644
--- a/tests/tcg/cris/check_addcm.c
+++ b/tests/tcg/cris/check_addcm.c
@@ -5,14 +5,14 @@
 #include "crisutils.h"
 
 /* need to avoid acr as source here.  */
-static inline int cris_addc_m(int a, const int *b)
+static always_inline int cris_addc_m(int a, const int *b)
 {
 	asm volatile ("addc [%1], %0\n" : "+r" (a) : "r" (b));
 	return a;
 }
 
 /* 'b' is a crisv32 constrain to avoid postinc with $acr.  */
-static inline int cris_addc_pi_m(int a, int **b)
+static always_inline int cris_addc_pi_m(int a, int **b)
 {
 	asm volatile ("addc [%1+], %0\n" : "+r" (a), "+b" (*b));
 	return a;
diff --git a/tests/tcg/cris/check_bound.c b/tests/tcg/cris/check_bound.c
index e883175..d956ab9 100644
--- a/tests/tcg/cris/check_bound.c
+++ b/tests/tcg/cris/check_bound.c
@@ -4,21 +4,21 @@
 #include "sys.h"
 #include "crisutils.h"
 
-static inline int cris_bound_b(int v, int b)
+static always_inline int cris_bound_b(int v, int b)
 {
 	int r = v;
 	asm ("bound.b\t%1, %0\n" : "+r" (r) : "ri" (b));
 	return r;
 }
 
-static inline int cris_bound_w(int v, int b)
+static always_inline int cris_bound_w(int v, int b)
 {
 	int r = v;
 	asm ("bound.w\t%1, %0\n" : "+r" (r) : "ri" (b));
 	return r;
 }
 
-static inline int cris_bound_d(int v, int b)
+static always_inline int cris_bound_d(int v, int b)
 {
 	int r = v;
 	asm ("bound.d\t%1, %0\n" : "+r" (r) : "ri" (b));
diff --git a/tests/tcg/cris/check_ftag.c b/tests/tcg/cris/check_ftag.c
index 908773a..aaa5c97 100644
--- a/tests/tcg/cris/check_ftag.c
+++ b/tests/tcg/cris/check_ftag.c
@@ -4,22 +4,22 @@
 #include "sys.h"
 #include "crisutils.h"
 
-static inline void cris_ftag_i(unsigned int x)
+static always_inline void cris_ftag_i(unsigned int x)
 {
 	register unsigned int v asm("$r10") = x;
 	asm ("ftagi\t[%0]\n" : : "r" (v) );
 }
-static inline void cris_ftag_d(unsigned int x)
+static always_inline void cris_ftag_d(unsigned int x)
 {
 	register unsigned int v asm("$r10") = x;
 	asm ("ftagd\t[%0]\n" : : "r" (v) );
 }
-static inline void cris_fidx_i(unsigned int x)
+static always_inline void cris_fidx_i(unsigned int x)
 {
 	register unsigned int v asm("$r10") = x;
 	asm ("fidxi\t[%0]\n" : : "r" (v) );
 }
-static inline void cris_fidx_d(unsigned int x)
+static always_inline void cris_fidx_d(unsigned int x)
 {
 	register unsigned int v asm("$r10") = x;
 	asm ("fidxd\t[%0]\n" : : "r" (v) );
diff --git a/tests/tcg/cris/check_int64.c b/tests/tcg/cris/check_int64.c
index fc60017..69caec1 100644
--- a/tests/tcg/cris/check_int64.c
+++ b/tests/tcg/cris/check_int64.c
@@ -5,12 +5,12 @@
 #include "crisutils.h"
 
 
-static inline int64_t add64(const int64_t a, const int64_t b)
+static always_inline int64_t add64(const int64_t a, const int64_t b)
 {
 	return a + b;
 }
 
-static inline int64_t sub64(const int64_t a, const int64_t b)
+static always_inline int64_t sub64(const int64_t a, const int64_t b)
 {
 	return a - b;
 }
diff --git a/tests/tcg/cris/check_lz.c b/tests/tcg/cris/check_lz.c
index 69c2e6d..bf051a6 100644
--- a/tests/tcg/cris/check_lz.c
+++ b/tests/tcg/cris/check_lz.c
@@ -3,7 +3,7 @@
 #include <stdint.h>
 #include "sys.h"
 
-static inline int cris_lz(int x)
+static always_inline int cris_lz(int x)
 {
 	int r;
 	asm ("lz\t%1, %0\n" : "=r" (r) : "r" (x));
diff --git a/tests/tcg/cris/check_swap.c b/tests/tcg/cris/check_swap.c
index f851cbc..9a68c1e 100644
--- a/tests/tcg/cris/check_swap.c
+++ b/tests/tcg/cris/check_swap.c
@@ -9,7 +9,7 @@
 #define B 2
 #define R 1
 
-static inline int cris_swap(const int mode, int x)
+static always_inline int cris_swap(const int mode, int x)
 {
 	switch (mode)
 	{
diff --git a/tests/tcg/cris/crisutils.h b/tests/tcg/cris/crisutils.h
index 3456b9d..bbbe6c5 100644
--- a/tests/tcg/cris/crisutils.h
+++ b/tests/tcg/cris/crisutils.h
@@ -13,57 +13,57 @@ void _err(void) {
 	_fail(tst_cc_loc);
 }
 
-static inline void cris_tst_cc_n1(void)
+static always_inline void cris_tst_cc_n1(void)
 {
 	asm volatile ("bpl _err\n"
 		      "nop\n");
 }
-static inline void cris_tst_cc_n0(void)
+static always_inline void cris_tst_cc_n0(void)
 {
 	asm volatile ("bmi _err\n"
 		      "nop\n");
 }
 
-static inline void cris_tst_cc_z1(void)
+static always_inline void cris_tst_cc_z1(void)
 {
 	asm volatile ("bne _err\n"
 		      "nop\n");
 }
-static inline void cris_tst_cc_z0(void)
+static always_inline void cris_tst_cc_z0(void)
 {
 	asm volatile ("beq _err\n"
 		      "nop\n");
 }
-static inline void cris_tst_cc_v1(void)
+static always_inline void cris_tst_cc_v1(void)
 {
 	asm volatile ("bvc _err\n"
 		      "nop\n");
 }
-static inline void cris_tst_cc_v0(void)
+static always_inline void cris_tst_cc_v0(void)
 {
 	asm volatile ("bvs _err\n"
 		      "nop\n");
 }
 
-static inline void cris_tst_cc_c1(void)
+static always_inline void cris_tst_cc_c1(void)
 {
 	asm volatile ("bcc _err\n"
 		      "nop\n");
 }
-static inline void cris_tst_cc_c0(void)
+static always_inline void cris_tst_cc_c0(void)
 {
 	asm volatile ("bcs _err\n"
 		      "nop\n");
 }
 
-static inline void cris_tst_mov_cc(int n, int z)
+static always_inline void cris_tst_mov_cc(int n, int z)
 {
 	if (n) cris_tst_cc_n1(); else cris_tst_cc_n0();
 	if (z) cris_tst_cc_z1(); else cris_tst_cc_z0();
 	asm volatile ("" : : "g" (_err));
 }
 
-static inline void cris_tst_cc(const int n, const int z,
+static always_inline void cris_tst_cc(const int n, const int z,
 			       const int v, const int c)
 {
 	if (n) cris_tst_cc_n1(); else cris_tst_cc_n0();
diff --git a/tests/tcg/cris/sys.h b/tests/tcg/cris/sys.h
index c5f88e1..3dd47bb 100644
--- a/tests/tcg/cris/sys.h
+++ b/tests/tcg/cris/sys.h
@@ -3,6 +3,8 @@
 #define STRINGIFY(x) #x
 #define TOSTRING(x) STRINGIFY(x)
 
+#define always_inline inline __attribute__((always_inline))
+
 #define CURRENT_LOCATION __FILE__ ":" TOSTRING(__LINE__)
 
 #define err()                         \
-- 
2.1.4

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

* Re: [Qemu-devel] [PATCH 8/9] target-cris: add v17 CPU
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 8/9] target-cris: add v17 CPU Rabin Vincent
@ 2016-09-12 22:18   ` Edgar E. Iglesias
  2016-09-26  7:17     ` Rabin Vincent
  0 siblings, 1 reply; 31+ messages in thread
From: Edgar E. Iglesias @ 2016-09-12 22:18 UTC (permalink / raw)
  To: Rabin Vincent; +Cc: qemu-devel, Rabin Vincent

On Mon, Sep 05, 2016 at 01:54:11PM +0200, Rabin Vincent wrote:
> From: Rabin Vincent <rabinv@axis.com>
> 
> In the CRIS v17 CPU an ADDC (add with carry) instruction has been added
> compared to the v10 instruction set.
> 
>  Assembler syntax:
> 
>   ADDC [Rs],Rd
>   ADDC [Rs+],Rd
> 
>  Size: Dword
> 
>  Description:
> 
>   The source data is added together with the carry flag to the
>   destination register. The size of the operation is dword.
> 
>  Operation:
> 
>   Rd += s + C-flag;
> 
>  Flags affected:
> 
>   S R P U I X N Z V C
>   - - - - - 0 * * * *
> 
>  Instruction format: ADDC [Rs],Rd
> 
>   +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
>   |Destination(Rd)| 1   0   0   1   1   0   1   0 |   Source(Rs)  |
>   +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> 
>  Instruction format: ADDC [Rs+],Rd
> 
>   +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
>   |Destination(Rd)| 1   1   0   1   1   0   1   0 |   Source(Rs)  |
>   +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> 
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  target-cris/cpu.c            | 14 ++++++++++++++
>  target-cris/crisv10-decode.h |  1 +
>  target-cris/translate_v10.c  |  8 ++++++++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/target-cris/cpu.c b/target-cris/cpu.c
> index c5a656b..d680cfb 100644
> --- a/target-cris/cpu.c
> +++ b/target-cris/cpu.c
> @@ -246,6 +246,16 @@ static void crisv11_cpu_class_init(ObjectClass *oc, void *data)
>      cc->gdb_read_register = crisv10_cpu_gdb_read_register;
>  }
>  
> +static void crisv17_cpu_class_init(ObjectClass *oc, void *data)
> +{
> +    CPUClass *cc = CPU_CLASS(oc);
> +    CRISCPUClass *ccc = CRIS_CPU_CLASS(oc);
> +
> +    ccc->vr = 17;
> +    cc->do_interrupt = crisv10_cpu_do_interrupt;
> +    cc->gdb_read_register = crisv10_cpu_gdb_read_register;
> +}
> +
>  static void crisv32_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      CRISCPUClass *ccc = CRIS_CPU_CLASS(oc);
> @@ -273,6 +283,10 @@ static const TypeInfo cris_cpu_model_type_infos[] = {
>          .parent = TYPE_CRIS_CPU,
>          .class_init = crisv11_cpu_class_init,
>      }, {
> +        .name = TYPE("crisv17"),
> +        .parent = TYPE_CRIS_CPU,
> +        .class_init = crisv17_cpu_class_init,
> +    }, {
>          .name = TYPE("crisv32"),
>          .parent = TYPE_CRIS_CPU,
>          .class_init = crisv32_cpu_class_init,
> diff --git a/target-cris/crisv10-decode.h b/target-cris/crisv10-decode.h
> index 587fbdd..bdb4b6d 100644
> --- a/target-cris/crisv10-decode.h
> +++ b/target-cris/crisv10-decode.h
> @@ -92,6 +92,7 @@
>  #define CRISV10_IND_JUMP_M       4
>  #define CRISV10_IND_DIP          5
>  #define CRISV10_IND_JUMP_R       6
> +#define CRISV17_IND_ADDC         6
>  #define CRISV10_IND_BOUND        7
>  #define CRISV10_IND_BCC_M        7
>  #define CRISV10_IND_MOVE_M_SPR   8
> diff --git a/target-cris/translate_v10.c b/target-cris/translate_v10.c
> index a3da425..33d86eb 100644
> --- a/target-cris/translate_v10.c
> +++ b/target-cris/translate_v10.c
> @@ -1097,6 +1097,14 @@ static unsigned int dec10_ind(CPUCRISState *env, DisasContext *dc)
>                  insn_len = dec10_bdap_m(env, dc, size);
>                  break;
>              default:
> +                if (dc->opcode == CRISV17_IND_ADDC && dc->size == 2 &&
> +                    env->pregs[PR_VR] == 17) {

Hi Rabin,

Could you please add some comments on the insn encoding?
Put the stuff from the commit msg in here.
IIRC, ADDC and v17 are modifications made to the CRISv10 family of
cores that never made it into the public manuals. Or am I wrong?

Cheers,
Edgar


> +                    LOG_DIS("addc op=%d %d\n",  dc->src, dc->dst);
> +                    cris_cc_mask(dc, CC_MASK_NZVC);
> +                    insn_len += dec10_ind_alu(env, dc, CC_OP_ADDC, size);
> +                    break;
> +                }
> +
>                  LOG_DIS("pc=%x var-ind.%d %d r%d r%d\n",
>                            dc->pc, size, dc->opcode, dc->src, dc->dst);
>                  cpu_abort(CPU(dc->cpu), "Unhandled opcode");
> -- 
> 2.1.4
> 

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

* Re: [Qemu-devel] [PATCH 7/9] target-cris: ignore prefix insns in singlestep
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 7/9] target-cris: ignore prefix insns in singlestep Rabin Vincent
@ 2016-09-12 22:49   ` Edgar E. Iglesias
  2016-09-14 13:32     ` Hans-Peter Nilsson
  0 siblings, 1 reply; 31+ messages in thread
From: Edgar E. Iglesias @ 2016-09-12 22:49 UTC (permalink / raw)
  To: Rabin Vincent; +Cc: qemu-devel, Hans-Peter Nilsson, Rabin Vincent

On Mon, Sep 05, 2016 at 01:54:10PM +0200, Rabin Vincent wrote:
> From: Hans-Peter Nilsson <hp@axis.com>
> 
> Don't count prefix instructions as separate when singlestepping.
> 
> For example, for this following instruction
> 
>  1ad8:       a011 e00b               move.d r0,[r1-96]
> 
> before this patch, we get two register dumps:
> 
>  PC=1ad8 CCS=0 btaken=1 btarget=1ac6
>  cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
>  $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
>  $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
>  $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
>  $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4
> 
>  PC=1ada CCS=800 btaken=1 btarget=1ac6
>  cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
>  $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
>  $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
>  $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
>  $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4
> 
> With the patch, we get only one:
> 
>  PC=1ad8 CCS=0 btaken=1 btarget=1ac6
>  cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
>  $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
>  $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
>  $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
>  $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4

Hi,

A concern I have is that we can't guard against all split prefix
sequences (e.g at page boundaries or with icount). So it may be more
confusing to see the prefix insns sometimes than every time.

Perhaps we should more clearly be showing prefix state in the logs?

BTW, are you guys doing post-processing on this or is it only
for human inspection?

Cheers,
Edgar


> 
> Signed-off-by: Hans-Peter Nilsson <hp@axis.com>
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  target-cris/translate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target-cris/translate.c b/target-cris/translate.c
> index a4512b5..c9b1e65 100644
> --- a/target-cris/translate.c
> +++ b/target-cris/translate.c
> @@ -3262,7 +3262,8 @@ void gen_intermediate_code(CPUCRISState *env, struct TranslationBlock *tb)
>          }
>      } while (!dc->is_jmp && !dc->cpustate_changed
>              && !tcg_op_buf_full()
> -            && !singlestep
> +            /* We don't count prefix insns as separate wrt. singlestep.  */
> +            && (!singlestep || (dc->tb_flags & PFIX_FLAG))
>              && (dc->pc < next_page_start)
>              && num_insns < max_insns);
>  
> -- 
> 2.1.4
> 

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

* Re: [Qemu-devel] [PATCH 3/9] tests: cris: remove openpf4 test
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 3/9] tests: cris: remove openpf4 test Rabin Vincent
@ 2016-09-12 22:51   ` Edgar E. Iglesias
  0 siblings, 0 replies; 31+ messages in thread
From: Edgar E. Iglesias @ 2016-09-12 22:51 UTC (permalink / raw)
  To: Rabin Vincent; +Cc: qemu-devel, Rabin Vincent

On Mon, Sep 05, 2016 at 01:54:06PM +0200, Rabin Vincent wrote:
> From: Rabin Vincent <rabinv@axis.com>
> 
> This test, borrowed from the GDB simulator test suite, is meant to test
> the GDB simulator's --sysroot feature and always fails in QEMU.  Remove
> it.  openpf3 tests the same sequence of system calls (without assuming
> the precence of --sysroot).
> 
> Signed-off-by: Rabin Vincent <rabinv@axis.com>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  tests/tcg/cris/Makefile        | 1 -
>  tests/tcg/cris/check_openpf4.c | 5 -----
>  2 files changed, 6 deletions(-)
>  delete mode 100644 tests/tcg/cris/check_openpf4.c
> 
> diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile
> index d34bfd8..f5230fc 100644
> --- a/tests/tcg/cris/Makefile
> +++ b/tests/tcg/cris/Makefile
> @@ -108,7 +108,6 @@ TESTCASES += check_stat4.ctst
>  TESTCASES += check_openpf1.ctst
>  TESTCASES += check_openpf2.ctst
>  TESTCASES += check_openpf3.ctst
> -TESTCASES += check_openpf4.ctst
>  TESTCASES += check_openpf5.ctst
>  TESTCASES += check_mapbrk.ctst
>  TESTCASES += check_mmap1.ctst
> diff --git a/tests/tcg/cris/check_openpf4.c b/tests/tcg/cris/check_openpf4.c
> deleted file mode 100644
> index 8bbee41..0000000
> --- a/tests/tcg/cris/check_openpf4.c
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -/* Basic file operations, now *with* sysroot.
> -#sim: --sysroot=@exedir@
> -*/
> -#define PREFIX "/"
> -#include "check_openpf3.c"
> -- 
> 2.1.4
> 

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

* Re: [Qemu-devel] [PATCH 6/9] target-cris: reduce v32isms from v10 log dumps
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 6/9] target-cris: reduce v32isms from v10 log dumps Rabin Vincent
@ 2016-09-12 22:59   ` Edgar E. Iglesias
  2016-09-14 12:48     ` Hans-Peter Nilsson
  2016-09-26  7:06     ` Rabin Vincent
  0 siblings, 2 replies; 31+ messages in thread
From: Edgar E. Iglesias @ 2016-09-12 22:59 UTC (permalink / raw)
  To: Rabin Vincent; +Cc: qemu-devel, Hans-Peter Nilsson, Rabin Vincent

On Mon, Sep 05, 2016 at 01:54:09PM +0200, Rabin Vincent wrote:
> From: Hans-Peter Nilsson <hp@axis.com>
> 
> Use the correct register names for v10 and don't dump support function
> registers for pre-v32.
> 
> Signed-off-by: Hans-Peter Nilsson <hp@axis.com>
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  target-cris/translate.c | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/target-cris/translate.c b/target-cris/translate.c
> index c280e24..a4512b5 100644
> --- a/target-cris/translate.c
> +++ b/target-cris/translate.c
> @@ -140,14 +140,14 @@ static void gen_BUG(DisasContext *dc, const char *file, int line)
>      cpu_abort(CPU(dc->cpu), "%s:%d\n", file, line);
>  }
>  
> -static const char *regnames[] =
> +static const char *regnames_v32[] =
>  {
>      "$r0", "$r1", "$r2", "$r3",
>      "$r4", "$r5", "$r6", "$r7",
>      "$r8", "$r9", "$r10", "$r11",
>      "$r12", "$r13", "$sp", "$acr",
>  };
> -static const char *pregnames[] =
> +static const char *pregnames_v32[] =
>  {
>      "$bz", "$vr", "$pid", "$srs",
>      "$wz", "$exs", "$eda", "$mof",
> @@ -3336,12 +3336,20 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
>  {
>      CRISCPU *cpu = CRIS_CPU(cs);
>      CPUCRISState *env = &cpu->env;
> +    const char **regnames;
> +    const char **pregnames;
>      int i;
> -    uint32_t srs;
>  
>      if (!env || !f) {
>          return;
>      }
> +    if (env->pregs[PR_VR] < 32) {
> +        pregnames = pregnames_v10;
> +        regnames = regnames_v10;
> +    } else {
> +        pregnames = pregnames_v32;
> +        regnames = regnames_v32;
> +    }
>  
>      cpu_fprintf(f, "PC=%x CCS=%x btaken=%d btarget=%x\n"
>              "cc_op=%d cc_src=%d cc_dest=%d cc_result=%x cc_mask=%x\n",
> @@ -3363,14 +3371,16 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
>              cpu_fprintf(f, "\n");
>          }
>      }
> -    srs = env->pregs[PR_SRS];
> -    cpu_fprintf(f, "\nsupport function regs bank %x:\n", srs);
> -    if (srs < ARRAY_SIZE(env->sregs)) {
> -        for (i = 0; i < 16; i++) {
> -            cpu_fprintf(f, "s%2.2d=%8.8x ",
> -                    i, env->sregs[srs][i]);
> -            if ((i + 1) % 4 == 0) {
> -                cpu_fprintf(f, "\n");
> +    if (env->pregs[PR_SRS] >= 32) {


did you mean env->pregs[PR_VR] >= 32 here?


> +        uint32_t srs = env->pregs[PR_SRS];
> +        cpu_fprintf(f, "\nsupport function regs bank %x:\n", srs);
> +        if (srs < ARRAY_SIZE(env->sregs)) {
> +            for (i = 0; i < 16; i++) {
> +                cpu_fprintf(f, "s%2.2d=%8.8x ",
> +                        i, env->sregs[srs][i]);
> +                if ((i + 1) % 4 == 0) {
> +                    cpu_fprintf(f, "\n");
> +                }
>              }
>          }
>      }
> @@ -3415,12 +3425,12 @@ void cris_initialize_tcg(void)
>      for (i = 0; i < 16; i++) {
>          cpu_R[i] = tcg_global_mem_new(cpu_env,
>                                        offsetof(CPUCRISState, regs[i]),
> -                                      regnames[i]);
> +                                      regnames_v32[i]);
>      }
>      for (i = 0; i < 16; i++) {
>          cpu_PR[i] = tcg_global_mem_new(cpu_env,
>                                         offsetof(CPUCRISState, pregs[i]),
> -                                       pregnames[i]);
> +                                       pregnames_v32[i]);
>      }
>  }
>  
> -- 
> 2.1.4
> 

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

* Re: [Qemu-devel] [PATCH 4/9] tests: cris: remove check_time1
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 4/9] tests: cris: remove check_time1 Rabin Vincent
@ 2016-09-12 23:00   ` Edgar E. Iglesias
  0 siblings, 0 replies; 31+ messages in thread
From: Edgar E. Iglesias @ 2016-09-12 23:00 UTC (permalink / raw)
  To: Rabin Vincent; +Cc: qemu-devel, Rabin Vincent

On Mon, Sep 05, 2016 at 01:54:07PM +0200, Rabin Vincent wrote:
> From: Rabin Vincent <rabinv@axis.com>
> 
> This test, borrowed from the GDB simulator test suite, checks that every
> syscall increments the time returned by gettimeofday() by exactly 1 ms.
> This is not guaranteed or even desirable on QEMU so remove this test.
> 
> Signed-off-by: Rabin Vincent <rabinv@axis.com>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  tests/tcg/cris/Makefile      |  1 -
>  tests/tcg/cris/check_time1.c | 46 --------------------------------------------
>  2 files changed, 47 deletions(-)
>  delete mode 100644 tests/tcg/cris/check_time1.c
> 
> diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile
> index f5230fc..14a9eb5 100644
> --- a/tests/tcg/cris/Makefile
> +++ b/tests/tcg/cris/Makefile
> @@ -114,7 +114,6 @@ TESTCASES += check_mmap1.ctst
>  TESTCASES += check_mmap2.ctst
>  TESTCASES += check_mmap3.ctst
>  TESTCASES += check_sigalrm.ctst
> -TESTCASES += check_time1.ctst
>  TESTCASES += check_time2.ctst
>  TESTCASES += check_settls1.ctst
>  
> diff --git a/tests/tcg/cris/check_time1.c b/tests/tcg/cris/check_time1.c
> deleted file mode 100644
> index 3fcf0e1..0000000
> --- a/tests/tcg/cris/check_time1.c
> +++ /dev/null
> @@ -1,46 +0,0 @@
> -/* Basic time functionality test: check that milliseconds are
> -   incremented for each syscall (does not work on host).  */
> -#include <stdio.h>
> -#include <time.h>
> -#include <sys/time.h>
> -#include <string.h>
> -#include <stdlib.h>
> -
> -void err (const char *s)
> -{
> -  perror (s);
> -  abort ();
> -}
> -
> -int
> -main (void)
> -{
> -  struct timeval t_m = {0, 0};
> -  struct timezone t_z = {0, 0};
> -  struct timeval t_m1 = {0, 0};
> -  int i;
> -
> -  if (gettimeofday (&t_m, &t_z) != 0)
> -    err ("gettimeofday");
> -
> -  for (i = 1; i < 10000; i++)
> -    if (gettimeofday (&t_m1, NULL) != 0)
> -      err ("gettimeofday 1");
> -    else
> -      if (t_m1.tv_sec * 1000000 + t_m1.tv_usec
> -	  != (t_m.tv_sec * 1000000 + t_m.tv_usec + i * 1000))
> -	{
> -	  fprintf (stderr, "t0 (%ld, %ld), i %d, t1 (%ld, %ld)\n",
> -		   t_m.tv_sec, t_m.tv_usec, i, t_m1.tv_sec, t_m1.tv_usec);
> -	  abort ();
> -	}
> -
> -  if (time (NULL) != t_m1.tv_sec)
> -    {
> -      fprintf (stderr, "time != gettod\n");
> -      abort ();
> -    }
> -
> -  printf ("pass\n");
> -  exit (0);
> -}
> -- 
> 2.1.4
> 

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

* Re: [Qemu-devel] [PATCH 9/9] tests: cris: add v17 ADDC test
  2016-09-05 11:54 ` [Qemu-devel] [PATCH 9/9] tests: cris: add v17 ADDC test Rabin Vincent
@ 2016-09-12 23:16   ` Edgar E. Iglesias
  0 siblings, 0 replies; 31+ messages in thread
From: Edgar E. Iglesias @ 2016-09-12 23:16 UTC (permalink / raw)
  To: Rabin Vincent; +Cc: qemu-devel, Rabin Vincent

On Mon, Sep 05, 2016 at 01:54:12PM +0200, Rabin Vincent wrote:
> From: Rabin Vincent <rabinv@axis.com>
> 
> Add a test for the newly implemented ADDC instruction in the v17 CRIS
> CPU.
> 
> Signed-off-by: Rabin Vincent <rabinv@axis.com>


Acked-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  tests/tcg/cris/Makefile        | 19 ++++++++++--
>  tests/tcg/cris/check_addcv17.s | 65 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 82 insertions(+), 2 deletions(-)
>  create mode 100644 tests/tcg/cris/check_addcv17.s
> 
> diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile
> index 14a9eb5..6b3dba4 100644
> --- a/tests/tcg/cris/Makefile
> +++ b/tests/tcg/cris/Makefile
> @@ -23,6 +23,7 @@ SYS        = sys.o
>  TESTCASES += check_abs.tst
>  TESTCASES += check_addc.tst
>  TESTCASES += check_addcm.tst
> +TESTCASES += check_addcv17.tst
>  TESTCASES += check_addo.tst
>  TESTCASES += check_addoq.tst
>  TESTCASES += check_addi.tst
> @@ -134,13 +135,27 @@ all: build
>  %.ctst: %.o
>  	$(CC) $(CFLAGS) $(LDLIBS) $< -o $@
>  
> +
> +sysv10.o: sys.c
> +	$(CC) $(CFLAGS) -mcpu=v10 -c $< -o $@
> +
> +crtv10.o: crt.s
> +	$(AS) $(ASFLAGS) -mcpu=v10 -c $< -o $@
> +
> +check_addcv17.tst: ASFLAGS += -mcpu=v10
> +check_addcv17.tst: CRT := crtv10.o
> +check_addcv17.tst: SYS := sysv10.o
> +check_addcv17.tst: crtv10.o sysv10.o
> +
>  build: $(CRT) $(SYS) $(TESTCASES)
>  
>  check: $(CRT) $(SYS) $(TESTCASES)
>  	@echo -e "\nQEMU simulator."
>  	for case in $(TESTCASES); do \
>  		echo -n "$$case "; \
> -		$(SIM) ./$$case; \
> +		SIMARGS=; \
> +		case $$case in *v17*) SIMARGS="-cpu crisv17";; esac; \
> +		$(SIM) $$SIMARGS ./$$case; \
>  	done
>  check-g: $(CRT) $(SYS) $(TESTCASES)
>  	@echo -e "\nGDB simulator."
> @@ -150,4 +165,4 @@ check-g: $(CRT) $(SYS) $(TESTCASES)
>  	done
>  
>  clean:
> -	$(RM) -fr $(TESTCASES) $(CRT) $(SYS)
> +	$(RM) -fr $(TESTCASES) *.o
> diff --git a/tests/tcg/cris/check_addcv17.s b/tests/tcg/cris/check_addcv17.s
> new file mode 100644
> index 0000000..52ef7a9
> --- /dev/null
> +++ b/tests/tcg/cris/check_addcv17.s
> @@ -0,0 +1,65 @@
> +# mach:  crisv17
> +
> + .include "testutils.inc"
> +
> + .macro addc Rs Rd inc=0
> +# Create the instruction manually since there is no assembler support yet
> + .word (\Rd << 12) | \Rs | (\inc << 10) | 0x09a0
> + .endm
> +
> + start
> +
> + .data
> +mem1:
> + .dword 0x0
> +mem2:
> + .dword 0x12345678
> +
> + .text
> + move.d mem1,r4
> + clearf nzvc
> + addc 4 3
> + test_cc 0 1 0 0
> + checkr3 0
> +
> + move.d mem1,r4
> + clearf nzvc
> + ax
> + addc 4 3
> + test_cc 0 0 0 0
> + checkr3 0
> +
> + move.d mem1,r4
> + clearf nzvc
> + setf c
> + addc 4 3
> + test_cc 0 0 0 0
> + checkr3 1
> +
> + move.d mem2,r4
> + moveq 2, r3
> + clearf nzvc
> + setf c
> + addc 4 3
> + test_cc 0 0 0 0
> + checkr3 1234567b
> +
> + move.d mem2,r5
> + clearf nzvc
> + cmp.d r4,r5
> + test_cc 0 1 0 0
> +
> + move.d mem2,r4
> + moveq 2, r3
> + clearf nzvc
> + addc 4 3 inc=1
> + test_cc 0 0 0 0
> + checkr3 1234567a
> +
> + move.d mem2,r5
> + clearf nzvc
> + addq 4,r5
> + cmp.d r4,r5
> + test_cc 0 1 0 0
> +
> + quit
> -- 
> 2.1.4
> 

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

* Re: [Qemu-devel] [PATCH 6/9] target-cris: reduce v32isms from v10 log dumps
  2016-09-12 22:59   ` Edgar E. Iglesias
@ 2016-09-14 12:48     ` Hans-Peter Nilsson
  2016-09-26  7:06     ` Rabin Vincent
  1 sibling, 0 replies; 31+ messages in thread
From: Hans-Peter Nilsson @ 2016-09-14 12:48 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, rabinv

> Date: Tue, 13 Sep 2016 00:59:51 +0200
> From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> > @@ -3336,12 +3336,20 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
> >  {
> >      CRISCPU *cpu = CRIS_CPU(cs);
> >      CPUCRISState *env = &cpu->env;
> > +    const char **regnames;
> > +    const char **pregnames;
> >      int i;
> > -    uint32_t srs;
> >  
> >      if (!env || !f) {
> >          return;
> >      }
> > +    if (env->pregs[PR_VR] < 32) {
> > +        pregnames = pregnames_v10;
> > +        regnames = regnames_v10;
> > +    } else {
> > +        pregnames = pregnames_v32;
> > +        regnames = regnames_v32;
> > +    }
> >  
> >      cpu_fprintf(f, "PC=%x CCS=%x btaken=%d btarget=%x\n"
> >              "cc_op=%d cc_src=%d cc_dest=%d cc_result=%x cc_mask=%x\n",
> > @@ -3363,14 +3371,16 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
> >              cpu_fprintf(f, "\n");
> >          }
> >      }
> > -    srs = env->pregs[PR_SRS];
> > -    cpu_fprintf(f, "\nsupport function regs bank %x:\n", srs);
> > -    if (srs < ARRAY_SIZE(env->sregs)) {
> > -        for (i = 0; i < 16; i++) {
> > -            cpu_fprintf(f, "s%2.2d=%8.8x ",
> > -                    i, env->sregs[srs][i]);
> > -            if ((i + 1) % 4 == 0) {
> > -                cpu_fprintf(f, "\n");
> > +    if (env->pregs[PR_SRS] >= 32) {
> 
> 
> did you mean env->pregs[PR_VR] >= 32 here?

Oops!  Most definitely yes.

brgds, H-P

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

* Re: [Qemu-devel] [PATCH 7/9] target-cris: ignore prefix insns in singlestep
  2016-09-12 22:49   ` Edgar E. Iglesias
@ 2016-09-14 13:32     ` Hans-Peter Nilsson
  0 siblings, 0 replies; 31+ messages in thread
From: Hans-Peter Nilsson @ 2016-09-14 13:32 UTC (permalink / raw)
  To: edgar.iglesias; +Cc: qemu-devel, rabinv

> Date: Tue, 13 Sep 2016 00:49:51 +0200
> From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> On Mon, Sep 05, 2016 at 01:54:10PM +0200, Rabin Vincent wrote:
> > From: Hans-Peter Nilsson <hp@axis.com>
> > 
> > Don't count prefix instructions as separate when singlestepping.
> > 
> > For example, for this following instruction
> > 
> >  1ad8:       a011 e00b               move.d r0,[r1-96]
> > 
> > before this patch, we get two register dumps:
> > 
> >  PC=1ad8 CCS=0 btaken=1 btarget=1ac6
> >  cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
> >  $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
> >  $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
> >  $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
> >  $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4
> > 
> >  PC=1ada CCS=800 btaken=1 btarget=1ac6
> >  cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
> >  $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
> >  $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
> >  $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
> >  $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4
> > 
> > With the patch, we get only one:
> > 
> >  PC=1ad8 CCS=0 btaken=1 btarget=1ac6
> >  cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0
> >  $r0=00000000 $r1=00004360 $r2=00004308 $r3=0000026c
> >  $r4=00002076 $r5=00002022 $r6=00000000 $r7=00000000
> >  $r8=00000000 $r9=00000ea2 $r10=00000002 $r11=00004308
> >  $r12=00001080 $r13=00000ec0 $sp=0000bfd8 $pc=00001ad4
> 
> Hi,
> 
> A concern I have is that we can't guard against all split prefix
> sequences (e.g at page boundaries or with icount). So it may be more
> confusing to see the prefix insns sometimes than every time.

I've forgotten most details, and have to refer to Rabin for
useful answers.

However, from the few glimpses I remember, basically I had to
key on "singlestep" and force a re-translation to get usable
dumps that didn't skip dump of state from all instructions
inside a translated hunk on subsequent executions.  There were
some hacks^Wpatches left-out (for being too invasive and
hackish, IIRC) to that effect, not sure how they affected this
particular code here.

> Perhaps we should more clearly be showing prefix state in the logs?

No: I'd rather just not have prefix instructions dumped
separately whenever possible as dumps get really big really
quick, but maybe both this patch and a prefix indicator then, to
reduce confusion.  I guess a better deal would be a method that
enables consistent dumps; page boundaries and icount shouldn't
affect dumps.  Maybe qemu has improved and there's a generic
method to get useful register dumps *from every instruction at
every execution* these days.

> BTW, are you guys doing post-processing on this

Certainly.  The output was massaged to have the same format as
dumps from another simulator, then fed into a script for use
together with the executable to get readable output (somewhat at
call-graph-level).

> or is it only
> for human inspection?
> 
> Cheers,
> Edgar
> 
> 
> > 
> > Signed-off-by: Hans-Peter Nilsson <hp@axis.com>
> > Signed-off-by: Rabin Vincent <rabinv@axis.com>
> > ---
> >  target-cris/translate.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target-cris/translate.c b/target-cris/translate.c
> > index a4512b5..c9b1e65 100644
> > --- a/target-cris/translate.c
> > +++ b/target-cris/translate.c
> > @@ -3262,7 +3262,8 @@ void gen_intermediate_code(CPUCRISState *env, struct TranslationBlock *tb)
> >          }
> >      } while (!dc->is_jmp && !dc->cpustate_changed
> >              && !tcg_op_buf_full()
> > -            && !singlestep
> > +            /* We don't count prefix insns as separate wrt. singlestep.  */
> > +            && (!singlestep || (dc->tb_flags & PFIX_FLAG))
> >              && (dc->pc < next_page_start)
> >              && num_insns < max_insns);
> >  
> > -- 
> > 2.1.4
> > 
> 

brgds, H-P

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

* Re: [Qemu-devel] [PATCH 6/9] target-cris: reduce v32isms from v10 log dumps
  2016-09-12 22:59   ` Edgar E. Iglesias
  2016-09-14 12:48     ` Hans-Peter Nilsson
@ 2016-09-26  7:06     ` Rabin Vincent
  1 sibling, 0 replies; 31+ messages in thread
From: Rabin Vincent @ 2016-09-26  7:06 UTC (permalink / raw)
  To: Edgar E. Iglesias; +Cc: qemu-devel, Hans-Peter Nilsson

On Tue, Sep 13, 2016 at 12:59:51AM +0200, Edgar E. Iglesias wrote:
> On Mon, Sep 05, 2016 at 01:54:09PM +0200, Rabin Vincent wrote:
> > @@ -3363,14 +3371,16 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
> >              cpu_fprintf(f, "\n");
> >          }
> >      }
> > -    srs = env->pregs[PR_SRS];
> > -    cpu_fprintf(f, "\nsupport function regs bank %x:\n", srs);
> > -    if (srs < ARRAY_SIZE(env->sregs)) {
> > -        for (i = 0; i < 16; i++) {
> > -            cpu_fprintf(f, "s%2.2d=%8.8x ",
> > -                    i, env->sregs[srs][i]);
> > -            if ((i + 1) % 4 == 0) {
> > -                cpu_fprintf(f, "\n");
> > +    if (env->pregs[PR_SRS] >= 32) {
> 
> 
> did you mean env->pregs[PR_VR] >= 32 here?

Here's an updated patch:

8<-------------
>From 4143896189ba073d14f6d89ad98ef4109d267bec Mon Sep 17 00:00:00 2001
From: Hans-Peter Nilsson <hp@axis.com>
Date: Mon, 15 Aug 2016 13:44:46 +0200
Subject: [PATCHv2] target-cris: reduce v32isms from v10 log dumps

Use the correct register names for v10 and don't dump support function
registers for pre-v32.

Signed-off-by: Hans-Peter Nilsson <hp@axis.com>
Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
v2: Fix PR_SRS -> PR_VR

 target-cris/translate.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/target-cris/translate.c b/target-cris/translate.c
index f4a8d7d..b5ab0a5 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -140,14 +140,14 @@ static void gen_BUG(DisasContext *dc, const char *file, int line)
     cpu_abort(CPU(dc->cpu), "%s:%d\n", file, line);
 }
 
-static const char *regnames[] =
+static const char *regnames_v32[] =
 {
     "$r0", "$r1", "$r2", "$r3",
     "$r4", "$r5", "$r6", "$r7",
     "$r8", "$r9", "$r10", "$r11",
     "$r12", "$r13", "$sp", "$acr",
 };
-static const char *pregnames[] =
+static const char *pregnames_v32[] =
 {
     "$bz", "$vr", "$pid", "$srs",
     "$wz", "$exs", "$eda", "$mof",
@@ -3327,12 +3327,20 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
 {
     CRISCPU *cpu = CRIS_CPU(cs);
     CPUCRISState *env = &cpu->env;
+    const char **regnames;
+    const char **pregnames;
     int i;
-    uint32_t srs;
 
     if (!env || !f) {
         return;
     }
+    if (env->pregs[PR_VR] < 32) {
+        pregnames = pregnames_v10;
+        regnames = regnames_v10;
+    } else {
+        pregnames = pregnames_v32;
+        regnames = regnames_v32;
+    }
 
     cpu_fprintf(f, "PC=%x CCS=%x btaken=%d btarget=%x\n"
             "cc_op=%d cc_src=%d cc_dest=%d cc_result=%x cc_mask=%x\n",
@@ -3354,14 +3362,16 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
             cpu_fprintf(f, "\n");
         }
     }
-    srs = env->pregs[PR_SRS];
-    cpu_fprintf(f, "\nsupport function regs bank %x:\n", srs);
-    if (srs < ARRAY_SIZE(env->sregs)) {
-        for (i = 0; i < 16; i++) {
-            cpu_fprintf(f, "s%2.2d=%8.8x ",
-                    i, env->sregs[srs][i]);
-            if ((i + 1) % 4 == 0) {
-                cpu_fprintf(f, "\n");
+    if (env->pregs[PR_VR] >= 32) {
+        uint32_t srs = env->pregs[PR_SRS];
+        cpu_fprintf(f, "\nsupport function regs bank %x:\n", srs);
+        if (srs < ARRAY_SIZE(env->sregs)) {
+            for (i = 0; i < 16; i++) {
+                cpu_fprintf(f, "s%2.2d=%8.8x ",
+                        i, env->sregs[srs][i]);
+                if ((i + 1) % 4 == 0) {
+                    cpu_fprintf(f, "\n");
+                }
             }
         }
     }
@@ -3406,12 +3416,12 @@ void cris_initialize_tcg(void)
     for (i = 0; i < 16; i++) {
         cpu_R[i] = tcg_global_mem_new(cpu_env,
                                       offsetof(CPUCRISState, regs[i]),
-                                      regnames[i]);
+                                      regnames_v32[i]);
     }
     for (i = 0; i < 16; i++) {
         cpu_PR[i] = tcg_global_mem_new(cpu_env,
                                        offsetof(CPUCRISState, pregs[i]),
-                                       pregnames[i]);
+                                       pregnames_v32[i]);
     }
 }
 
-- 
2.1.4

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

* Re: [Qemu-devel] [PATCH 8/9] target-cris: add v17 CPU
  2016-09-12 22:18   ` Edgar E. Iglesias
@ 2016-09-26  7:17     ` Rabin Vincent
  2016-09-28 10:42       ` Edgar E. Iglesias
  0 siblings, 1 reply; 31+ messages in thread
From: Rabin Vincent @ 2016-09-26  7:17 UTC (permalink / raw)
  To: Edgar E. Iglesias; +Cc: qemu-devel

On Tue, Sep 13, 2016 at 12:18:00AM +0200, Edgar E. Iglesias wrote:
> On Mon, Sep 05, 2016 at 01:54:11PM +0200, Rabin Vincent wrote:
> > diff --git a/target-cris/translate_v10.c b/target-cris/translate_v10.c
> > index a3da425..33d86eb 100644
> > --- a/target-cris/translate_v10.c
> > +++ b/target-cris/translate_v10.c
> > @@ -1097,6 +1097,14 @@ static unsigned int dec10_ind(CPUCRISState *env, DisasContext *dc)
> >                  insn_len = dec10_bdap_m(env, dc, size);
> >                  break;
> >              default:
> > +                if (dc->opcode == CRISV17_IND_ADDC && dc->size == 2 &&
> > +                    env->pregs[PR_VR] == 17) {
> 
> Could you please add some comments on the insn encoding?
> Put the stuff from the commit msg in here.

OK, see new patch below.

> IIRC, ADDC and v17 are modifications made to the CRISv10 family of
> cores that never made it into the public manuals. Or am I wrong?

No, you're right.

8<---------------
>From 513465ad3f007885bafba3482705ba57cacd588b Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabinv@axis.com>
Date: Mon, 15 Aug 2016 13:59:32 +0200
Subject: [PATCH] target-cris: add v17 CPU

In the CRIS v17 CPU an ADDC (add with carry) instruction has been added
compared to the v10 instruction set.

 Assembler syntax:

  ADDC [Rs],Rd
  ADDC [Rs+],Rd

 Size: Dword

 Description:

  The source data is added together with the carry flag to the
  destination register. The size of the operation is dword.

 Operation:

  Rd += s + C-flag;

 Flags affected:

  S R P U I X N Z V C
  - - - - - 0 * * * *

 Instruction format: ADDC [Rs],Rd

  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
  |Destination(Rd)| 1   0   0   1   1   0   1   0 |   Source(Rs)  |
  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+

 Instruction format: ADDC [Rs+],Rd

  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
  |Destination(Rd)| 1   1   0   1   1   0   1   0 |   Source(Rs)  |
  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 target-cris/cpu.c            | 14 ++++++++++++++
 target-cris/crisv10-decode.h |  1 +
 target-cris/translate_v10.c  | 23 +++++++++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/target-cris/cpu.c b/target-cris/cpu.c
index c5a656b..d680cfb 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -246,6 +246,16 @@ static void crisv11_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_read_register = crisv10_cpu_gdb_read_register;
 }
 
+static void crisv17_cpu_class_init(ObjectClass *oc, void *data)
+{
+    CPUClass *cc = CPU_CLASS(oc);
+    CRISCPUClass *ccc = CRIS_CPU_CLASS(oc);
+
+    ccc->vr = 17;
+    cc->do_interrupt = crisv10_cpu_do_interrupt;
+    cc->gdb_read_register = crisv10_cpu_gdb_read_register;
+}
+
 static void crisv32_cpu_class_init(ObjectClass *oc, void *data)
 {
     CRISCPUClass *ccc = CRIS_CPU_CLASS(oc);
@@ -273,6 +283,10 @@ static const TypeInfo cris_cpu_model_type_infos[] = {
         .parent = TYPE_CRIS_CPU,
         .class_init = crisv11_cpu_class_init,
     }, {
+        .name = TYPE("crisv17"),
+        .parent = TYPE_CRIS_CPU,
+        .class_init = crisv17_cpu_class_init,
+    }, {
         .name = TYPE("crisv32"),
         .parent = TYPE_CRIS_CPU,
         .class_init = crisv32_cpu_class_init,
diff --git a/target-cris/crisv10-decode.h b/target-cris/crisv10-decode.h
index 587fbdd..bdb4b6d 100644
--- a/target-cris/crisv10-decode.h
+++ b/target-cris/crisv10-decode.h
@@ -92,6 +92,7 @@
 #define CRISV10_IND_JUMP_M       4
 #define CRISV10_IND_DIP          5
 #define CRISV10_IND_JUMP_R       6
+#define CRISV17_IND_ADDC         6
 #define CRISV10_IND_BOUND        7
 #define CRISV10_IND_BCC_M        7
 #define CRISV10_IND_MOVE_M_SPR   8
diff --git a/target-cris/translate_v10.c b/target-cris/translate_v10.c
index 4707a18..0e4d039 100644
--- a/target-cris/translate_v10.c
+++ b/target-cris/translate_v10.c
@@ -1094,6 +1094,29 @@ static unsigned int dec10_ind(CPUCRISState *env, DisasContext *dc)
                 insn_len = dec10_bdap_m(env, dc, size);
                 break;
             default:
+                /*
+                 * ADDC for v17:
+                 *
+                 * Instruction format: ADDC [Rs],Rd
+                 *
+                 *  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+                 *  |Destination(Rd)| 1   0   0   1   1   0   1   0 |   Source(Rs)  |
+                 *  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+                 *
+                 * Instruction format: ADDC [Rs+],Rd
+                 *
+                 *  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+                 *  |Destination(Rd)| 1   1   0   1   1   0   1   0 |   Source(Rs)  |
+                 *  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
+                 */
+                if (dc->opcode == CRISV17_IND_ADDC && dc->size == 2 &&
+                    env->pregs[PR_VR] == 17) {
+                    LOG_DIS("addc op=%d %d\n",  dc->src, dc->dst);
+                    cris_cc_mask(dc, CC_MASK_NZVC);
+                    insn_len += dec10_ind_alu(env, dc, CC_OP_ADDC, size);
+                    break;
+                }
+
                 LOG_DIS("pc=%x var-ind.%d %d r%d r%d\n",
                           dc->pc, size, dc->opcode, dc->src, dc->dst);
                 cpu_abort(CPU(dc->cpu), "Unhandled opcode");
-- 
2.1.4

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

* Re: [Qemu-devel] [PATCH 8/9] target-cris: add v17 CPU
  2016-09-26  7:17     ` Rabin Vincent
@ 2016-09-28 10:42       ` Edgar E. Iglesias
  2016-09-30 16:50         ` Rabin Vincent
  0 siblings, 1 reply; 31+ messages in thread
From: Edgar E. Iglesias @ 2016-09-28 10:42 UTC (permalink / raw)
  To: Rabin Vincent; +Cc: qemu-devel

On Mon, Sep 26, 2016 at 09:17:33AM +0200, Rabin Vincent wrote:
> On Tue, Sep 13, 2016 at 12:18:00AM +0200, Edgar E. Iglesias wrote:
> > On Mon, Sep 05, 2016 at 01:54:11PM +0200, Rabin Vincent wrote:
> > > diff --git a/target-cris/translate_v10.c b/target-cris/translate_v10.c
> > > index a3da425..33d86eb 100644
> > > --- a/target-cris/translate_v10.c
> > > +++ b/target-cris/translate_v10.c
> > > @@ -1097,6 +1097,14 @@ static unsigned int dec10_ind(CPUCRISState *env, DisasContext *dc)
> > >                  insn_len = dec10_bdap_m(env, dc, size);
> > >                  break;
> > >              default:
> > > +                if (dc->opcode == CRISV17_IND_ADDC && dc->size == 2 &&
> > > +                    env->pregs[PR_VR] == 17) {
> > 
> > Could you please add some comments on the insn encoding?
> > Put the stuff from the commit msg in here.
> 
> OK, see new patch below.
> 
> > IIRC, ADDC and v17 are modifications made to the CRISv10 family of
> > cores that never made it into the public manuals. Or am I wrong?


Thanks Rabin,

I've applied these except patch #7 "ignore prefix insns in singlestep".
Patch #8 had an issue with checkpatch that I fixed up.

Cheers,
Edgar


> 
> No, you're right.
> 
> 8<---------------
> From 513465ad3f007885bafba3482705ba57cacd588b Mon Sep 17 00:00:00 2001
> From: Rabin Vincent <rabinv@axis.com>
> Date: Mon, 15 Aug 2016 13:59:32 +0200
> Subject: [PATCH] target-cris: add v17 CPU
> 
> In the CRIS v17 CPU an ADDC (add with carry) instruction has been added
> compared to the v10 instruction set.
> 
>  Assembler syntax:
> 
>   ADDC [Rs],Rd
>   ADDC [Rs+],Rd
> 
>  Size: Dword
> 
>  Description:
> 
>   The source data is added together with the carry flag to the
>   destination register. The size of the operation is dword.
> 
>  Operation:
> 
>   Rd += s + C-flag;
> 
>  Flags affected:
> 
>   S R P U I X N Z V C
>   - - - - - 0 * * * *
> 
>  Instruction format: ADDC [Rs],Rd
> 
>   +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
>   |Destination(Rd)| 1   0   0   1   1   0   1   0 |   Source(Rs)  |
>   +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> 
>  Instruction format: ADDC [Rs+],Rd
> 
>   +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
>   |Destination(Rd)| 1   1   0   1   1   0   1   0 |   Source(Rs)  |
>   +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> 
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  target-cris/cpu.c            | 14 ++++++++++++++
>  target-cris/crisv10-decode.h |  1 +
>  target-cris/translate_v10.c  | 23 +++++++++++++++++++++++
>  3 files changed, 38 insertions(+)
> 
> diff --git a/target-cris/cpu.c b/target-cris/cpu.c
> index c5a656b..d680cfb 100644
> --- a/target-cris/cpu.c
> +++ b/target-cris/cpu.c
> @@ -246,6 +246,16 @@ static void crisv11_cpu_class_init(ObjectClass *oc, void *data)
>      cc->gdb_read_register = crisv10_cpu_gdb_read_register;
>  }
>  
> +static void crisv17_cpu_class_init(ObjectClass *oc, void *data)
> +{
> +    CPUClass *cc = CPU_CLASS(oc);
> +    CRISCPUClass *ccc = CRIS_CPU_CLASS(oc);
> +
> +    ccc->vr = 17;
> +    cc->do_interrupt = crisv10_cpu_do_interrupt;
> +    cc->gdb_read_register = crisv10_cpu_gdb_read_register;
> +}
> +
>  static void crisv32_cpu_class_init(ObjectClass *oc, void *data)
>  {
>      CRISCPUClass *ccc = CRIS_CPU_CLASS(oc);
> @@ -273,6 +283,10 @@ static const TypeInfo cris_cpu_model_type_infos[] = {
>          .parent = TYPE_CRIS_CPU,
>          .class_init = crisv11_cpu_class_init,
>      }, {
> +        .name = TYPE("crisv17"),
> +        .parent = TYPE_CRIS_CPU,
> +        .class_init = crisv17_cpu_class_init,
> +    }, {
>          .name = TYPE("crisv32"),
>          .parent = TYPE_CRIS_CPU,
>          .class_init = crisv32_cpu_class_init,
> diff --git a/target-cris/crisv10-decode.h b/target-cris/crisv10-decode.h
> index 587fbdd..bdb4b6d 100644
> --- a/target-cris/crisv10-decode.h
> +++ b/target-cris/crisv10-decode.h
> @@ -92,6 +92,7 @@
>  #define CRISV10_IND_JUMP_M       4
>  #define CRISV10_IND_DIP          5
>  #define CRISV10_IND_JUMP_R       6
> +#define CRISV17_IND_ADDC         6
>  #define CRISV10_IND_BOUND        7
>  #define CRISV10_IND_BCC_M        7
>  #define CRISV10_IND_MOVE_M_SPR   8
> diff --git a/target-cris/translate_v10.c b/target-cris/translate_v10.c
> index 4707a18..0e4d039 100644
> --- a/target-cris/translate_v10.c
> +++ b/target-cris/translate_v10.c
> @@ -1094,6 +1094,29 @@ static unsigned int dec10_ind(CPUCRISState *env, DisasContext *dc)
>                  insn_len = dec10_bdap_m(env, dc, size);
>                  break;
>              default:
> +                /*
> +                 * ADDC for v17:
> +                 *
> +                 * Instruction format: ADDC [Rs],Rd
> +                 *
> +                 *  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> +                 *  |Destination(Rd)| 1   0   0   1   1   0   1   0 |   Source(Rs)  |
> +                 *  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> +                 *
> +                 * Instruction format: ADDC [Rs+],Rd
> +                 *
> +                 *  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> +                 *  |Destination(Rd)| 1   1   0   1   1   0   1   0 |   Source(Rs)  |
> +                 *  +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
> +                 */
> +                if (dc->opcode == CRISV17_IND_ADDC && dc->size == 2 &&
> +                    env->pregs[PR_VR] == 17) {
> +                    LOG_DIS("addc op=%d %d\n",  dc->src, dc->dst);
> +                    cris_cc_mask(dc, CC_MASK_NZVC);
> +                    insn_len += dec10_ind_alu(env, dc, CC_OP_ADDC, size);
> +                    break;
> +                }
> +
>                  LOG_DIS("pc=%x var-ind.%d %d r%d r%d\n",
>                            dc->pc, size, dc->opcode, dc->src, dc->dst);
>                  cpu_abort(CPU(dc->cpu), "Unhandled opcode");
> -- 
> 2.1.4
> 

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

* Re: [Qemu-devel] [PATCH 8/9] target-cris: add v17 CPU
  2016-09-28 10:42       ` Edgar E. Iglesias
@ 2016-09-30 16:50         ` Rabin Vincent
  0 siblings, 0 replies; 31+ messages in thread
From: Rabin Vincent @ 2016-09-30 16:50 UTC (permalink / raw)
  To: Edgar E. Iglesias; +Cc: qemu-devel

On Wed, Sep 28, 2016 at 12:42:41PM +0200, Edgar E. Iglesias wrote:
> I've applied these except patch #7 "ignore prefix insns in
> singlestep".  Patch #8 had an issue with checkpatch that I fixed up.

Oops, thank you.

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

end of thread, other threads:[~2016-09-30 16:50 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-05 11:54 [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Rabin Vincent
2016-09-05 11:54 ` [Qemu-devel] [PATCH 2/9] tests: cris: fix syscall inline asm Rabin Vincent
2016-09-05 18:20   ` Richard Henderson
2016-09-08 11:38     ` [Qemu-devel] [PATCHv2 2/8] " Rabin Vincent
2016-09-08 16:10       ` Richard Henderson
2016-09-05 11:54 ` [Qemu-devel] [PATCH 3/9] tests: cris: remove openpf4 test Rabin Vincent
2016-09-12 22:51   ` Edgar E. Iglesias
2016-09-05 11:54 ` [Qemu-devel] [PATCH 4/9] tests: cris: remove check_time1 Rabin Vincent
2016-09-12 23:00   ` Edgar E. Iglesias
2016-09-05 11:54 ` [Qemu-devel] [PATCH 5/9] target-cris: sync CC state at load/stores Rabin Vincent
2016-09-05 19:02   ` Richard Henderson
2016-09-06 21:52     ` Edgar E. Iglesias
2016-09-05 11:54 ` [Qemu-devel] [PATCH 6/9] target-cris: reduce v32isms from v10 log dumps Rabin Vincent
2016-09-12 22:59   ` Edgar E. Iglesias
2016-09-14 12:48     ` Hans-Peter Nilsson
2016-09-26  7:06     ` Rabin Vincent
2016-09-05 11:54 ` [Qemu-devel] [PATCH 7/9] target-cris: ignore prefix insns in singlestep Rabin Vincent
2016-09-12 22:49   ` Edgar E. Iglesias
2016-09-14 13:32     ` Hans-Peter Nilsson
2016-09-05 11:54 ` [Qemu-devel] [PATCH 8/9] target-cris: add v17 CPU Rabin Vincent
2016-09-12 22:18   ` Edgar E. Iglesias
2016-09-26  7:17     ` Rabin Vincent
2016-09-28 10:42       ` Edgar E. Iglesias
2016-09-30 16:50         ` Rabin Vincent
2016-09-05 11:54 ` [Qemu-devel] [PATCH 9/9] tests: cris: add v17 ADDC test Rabin Vincent
2016-09-12 23:16   ` Edgar E. Iglesias
2016-09-06 21:53 ` [Qemu-devel] [PATCH 1/9] tests: cris: force inlining Edgar E. Iglesias
2016-09-08 11:41   ` Rabin Vincent
2016-09-08 18:06 ` Peter Maydell
2016-09-08 18:21   ` Eric Blake
2016-09-12  9:43   ` [Qemu-devel] [PATCHv2 1/8] " Rabin Vincent

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.