Patchwork [v3,07/13] target-sh4: Make increment_urc() take SuperHCPU

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 target-sh4/helper.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)

On 14 April 2012 23:12, Andreas Färber <afaerber@suse.de> wrote:
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Is this really an improvement?

-- PMM

> ---
>  target-sh4/helper.c |   23 ++++++++++++-----------
>  1 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/target-sh4/helper.c b/target-sh4/helper.c
> index 655faaa..2d5a4e4 100644
> --- a/target-sh4/helper.c
> +++ b/target-sh4/helper.c
> @@ -269,17 +269,18 @@ static int find_tlb_entry(CPUSH4State * env, target_ulong address,
>     return match;
>  }
>
> -static void increment_urc(CPUSH4State * env)
> +static void increment_urc(SuperHCPU *cpu)
>  {
>     uint8_t urb, urc;
>
>     /* Increment URC */
> -    urb = ((env->mmucr) >> 18) & 0x3f;
> -    urc = ((env->mmucr) >> 10) & 0x3f;
> +    urb = ((cpu->env.mmucr) >> 18) & 0x3f;
> +    urc = ((cpu->env.mmucr) >> 10) & 0x3f;
>     urc++;
> -    if ((urb > 0 && urc > urb) || urc > (UTLB_SIZE - 1))
> +    if ((urb > 0 && urc > urb) || urc > (UTLB_SIZE - 1)) {
>        urc = 0;
> -    env->mmucr = (env->mmucr & 0xffff03ff) | (urc << 10);
> +    }
> +    cpu->env.mmucr = (cpu->env.mmucr & 0xffff03ff) | (urc << 10);
>  }
>
>  /* Copy and utlb entry into itlb
> @@ -324,7 +325,7 @@ static int find_itlb_entry(CPUSH4State * env, target_ulong address,
>  static int find_utlb_entry(CPUSH4State * env, target_ulong address, int use_asid)
>  {
>     /* per utlb access */
> -    increment_urc(env);
> +    increment_urc(sh_env_get_cpu(env));
>
>     /* Return entry */
>     return find_tlb_entry(env, address, env->utlb, UTLB_SIZE, use_asid);
> @@ -660,7 +661,7 @@ uint32_t cpu_sh4_read_mmaped_utlb_addr(CPUSH4State *s,
>     int index = (addr & 0x00003f00) >> 8;
>     tlb_t * entry = &s->utlb[index];
>
> -    increment_urc(s); /* per utlb access */
> +    increment_urc(sh_env_get_cpu(s)); /* per utlb access */
>
>     return (entry->vpn  << 10) |
>            (entry->v    <<  8) |
> @@ -702,7 +703,7 @@ void cpu_sh4_write_mmaped_utlb_addr(CPUSH4State *s, target_phys_addr_t addr,
>                entry->d = d;
>                utlb_match_entry = entry;
>            }
> -           increment_urc(s); /* per utlb access */
> +            increment_urc(sh_env_get_cpu(s)); /* per utlb access */
>        }
>
>        /* search ITLB */
> @@ -735,7 +736,7 @@ void cpu_sh4_write_mmaped_utlb_addr(CPUSH4State *s, target_phys_addr_t addr,
>        entry->vpn = vpn;
>        entry->d = d;
>        entry->v = v;
> -       increment_urc(s);
> +        increment_urc(sh_env_get_cpu(s));
>     }
>  }
>
> @@ -746,7 +747,7 @@ uint32_t cpu_sh4_read_mmaped_utlb_data(CPUSH4State *s,
>     int index = (addr & 0x00003f00) >> 8;
>     tlb_t * entry = &s->utlb[index];
>
> -    increment_urc(s); /* per utlb access */
> +    increment_urc(sh_env_get_cpu(s)); /* per utlb access */
>
>     if (array == 0) {
>         /* ITLB Data Array 1 */
> @@ -773,7 +774,7 @@ void cpu_sh4_write_mmaped_utlb_data(CPUSH4State *s, target_phys_addr_t addr,
>     int index = (addr & 0x00003f00) >> 8;
>     tlb_t * entry = &s->utlb[index];
>
> -    increment_urc(s); /* per utlb access */
> +    increment_urc(sh_env_get_cpu(s)); /* per utlb access */
>
>     if (array == 0) {
>         /* UTLB Data Array 1 */
> --
> 1.7.7
>
>

Am 16.04.2012 16:53, schrieb Peter Maydell:
> On 14 April 2012 23:12, Andreas Färber <afaerber@suse.de> wrote:
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
> 
> Is this really an improvement?

Yes, these patches are propagating the use of QOM *CPU throughout code
so that we can gradually drop *_env_get_cpu() calls and at some point
remove it completely. Expect to see more of such incremental cleanups in
the followup series - having CPUState be QOM in 03/13 is not the end
goal, it's having the code actually use it instead of CPU*State. General
idea is CPUArchState -> CPUState (therefore the automated CPUState ->
CPU*State refactorings) and CPU*State -> *CPU except for TCG (Richard's
pointer-to-middle concept, therefore "first member of" in the various
conversions).

Looking at this particular patch again, adding a local env variable
would avoid some of the cpu->env.* accesses. That should be better.

Andreas