Patchwork [09/26] target-xtensa: add special and user registers

login
register
mail settings
Submitter Max Filippov
Date May 17, 2011, 10:32 p.m.
Message ID <1305671572-5899-10-git-send-email-jcmvbkbc@gmail.com>
Download mbox | patch
Permalink /patch/96071/
State New
Headers show

Comments

Max Filippov - May 17, 2011, 10:32 p.m.
Special Registers hold the majority of the state added to the processor
by the options. See ISA, 5.3 for details.

User Registers hold state added in support of designer's TIE and in some
cases of options that Tensilica provides. See ISA, 5.4 for details.

Only registers mapped in sregnames or uregnames are considered valid.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 target-xtensa/cpu.h       |    7 ++++++
 target-xtensa/translate.c |   47 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 2 deletions(-)
Richard Henderson - May 19, 2011, 8:59 p.m.
On 05/17/2011 03:32 PM, Max Filippov wrote:
> +enum {
> +    THREADPTR = 231,
> +    FCR = 232,
> +    FSR = 233,
> +};
> +
>  typedef struct XtensaConfig {
>      const char *name;
>      uint64_t options;
> @@ -109,6 +115,7 @@ typedef struct CPUXtensaState {
>      uint32_t regs[16];
>      uint32_t pc;
>      uint32_t sregs[256];
> +    uint32_t uregs[256];

Is it really worthwhile allocating 2k worth of space in the
CPUState when only several of the slots are actually used?

I would think that it might be better to have a function to
map between number to offset/register.  E.g.

int ur_offset(int ur)
{
    switch (ur) {
    case THREADPTR:
        return offsetof(CPUState, ur_threadptr);
    case FCR:
        return offsetof(CPUState, ur_fcr);
    case FSR:
        return offsetof(CPUState, ur_fsr);
    }
    return -1;
}

where the individual slots are allocated by hand in the
CPUState.  The fact that they'll be named in the struct
will also make it easier to dump the value inside gdb and
see what the individual values are.


r~
Max Filippov - May 20, 2011, 7:34 a.m.
> > +enum {
> > +    THREADPTR = 231,
> > +    FCR = 232,
> > +    FSR = 233,
> > +};
> > +
> >  typedef struct XtensaConfig {
> >      const char *name;
> >      uint64_t options;
> > @@ -109,6 +115,7 @@ typedef struct CPUXtensaState {
> >      uint32_t regs[16];
> >      uint32_t pc;
> >      uint32_t sregs[256];
> > +    uint32_t uregs[256];
> 
> Is it really worthwhile allocating 2k worth of space in the
> CPUState when only several of the slots are actually used?
> 
> I would think that it might be better to have a function to
> map between number to offset/register.  E.g.
> 
> int ur_offset(int ur)
> {
>     switch (ur) {
>     case THREADPTR:
>         return offsetof(CPUState, ur_threadptr);
>     case FCR:
>         return offsetof(CPUState, ur_fcr);
>     case FSR:
>         return offsetof(CPUState, ur_fsr);
>     }
>     return -1;
> }
> 
> where the individual slots are allocated by hand in the
> CPUState.  The fact that they'll be named in the struct
> will also make it easier to dump the value inside gdb and
> see what the individual values are.

User registers represent TIE states that may appear in custom xtensa configurations. I'd better change RUR and WUR so that they can access all user registers but warn on those not defined globally or in the CPUEnv::config.
Is it OK?

Thanks.
-- Max
Richard Henderson - May 20, 2011, 2:18 p.m.
On 05/20/2011 12:34 AM, Max Filippov wrote:
> User registers represent TIE states that may appear in custom xtensa
> configurations. I'd better change RUR and WUR so that they can access
> all user registers but warn on those not defined globally or in the
> CPUEnv::config. Is it OK?

Well, it's ok if you change nothing.  However, I wanted you to think
about other ways that might make sense than simply allocating all of
the registers.


r~

Patch

diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index d8cd923..0b40fa8 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -99,6 +99,12 @@  enum {
     XTENSA_OPTIN_TRACE_PORT,
 };
 
+enum {
+    THREADPTR = 231,
+    FCR = 232,
+    FSR = 233,
+};
+
 typedef struct XtensaConfig {
     const char *name;
     uint64_t options;
@@ -109,6 +115,7 @@  typedef struct CPUXtensaState {
     uint32_t regs[16];
     uint32_t pc;
     uint32_t sregs[256];
+    uint32_t uregs[256];
 
     CPU_COMMON
 } CPUXtensaState;
diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index 0a43ec0..2fcd36d 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -52,9 +52,20 @@  typedef struct DisasContext {
 static TCGv_ptr cpu_env;
 static TCGv_i32 cpu_pc;
 static TCGv_i32 cpu_R[16];
+static TCGv_i32 cpu_SR[256];
+static TCGv_i32 cpu_UR[256];
 
 #include "gen-icount.h"
 
+static const char * const sregnames[256] = {
+};
+
+static const char * const uregnames[256] = {
+    [THREADPTR] = "THREADPTR",
+    [FCR] = "FCR",
+    [FSR] = "FSR",
+};
+
 void xtensa_translate_init(void)
 {
     static const char * const regnames[] = {
@@ -74,6 +85,22 @@  void xtensa_translate_init(void)
                 offsetof(CPUState, regs[i]),
                 regnames[i]);
     }
+
+    for (i = 0; i < 256; ++i) {
+        if (sregnames[i]) {
+            cpu_SR[i] = tcg_global_mem_new_i32(TCG_AREG0,
+                    offsetof(CPUState, sregs[i]),
+                    sregnames[i]);
+        }
+    }
+
+    for (i = 0; i < 256; ++i) {
+        if (uregnames[i]) {
+            cpu_UR[i] = tcg_global_mem_new_i32(TCG_AREG0,
+                    offsetof(CPUState, uregs[i]),
+                    uregnames[i]);
+        }
+    }
 }
 
 static inline int option_enabled(DisasContext *dc, int opt)
@@ -777,9 +804,25 @@  void gen_intermediate_code_pc(CPUState *env, TranslationBlock *tb)
 void cpu_dump_state(CPUState *env, FILE *f, fprintf_function cpu_fprintf,
         int flags)
 {
-    int i;
+    int i, j;
+
+    cpu_fprintf(f, "PC=%08x\n\n", env->pc);
+
+    for (i = j = 0; i < 256; ++i)
+        if (sregnames[i]) {
+            cpu_fprintf(f, "%s=%08x%c", sregnames[i], env->sregs[i],
+                    (j++ % 4) == 3 ? '\n' : ' ');
+        }
+
+    cpu_fprintf(f, (j % 4) == 0 ? "\n" : "\n\n");
+
+    for (i = j = 0; i < 256; ++i)
+        if (uregnames[i]) {
+            cpu_fprintf(f, "%s=%08x%c", uregnames[i], env->uregs[i],
+                    (j++ % 4) == 3 ? '\n' : ' ');
+        }
 
-    cpu_fprintf(f, "PC=%08x\n", env->pc);
+    cpu_fprintf(f, (j % 4) == 0 ? "\n" : "\n\n");
 
     for (i = 0; i < 16; ++i)
         cpu_fprintf(f, "A%02d=%08x%c", i, env->regs[i],