Re: [Qemu-devel] [PATCH v3 05/16] tcg: Define tcg_insn_unit for codepointerss

[Alex Bennée]

Richard Henderson <address@hidden> writes:

> To be defined by the tcg backend based on the elemental unit of the ISA.
> During the transition, allow TCG_TARGET_INSN_UNIT_SIZE to be undefined,
> which allows us to default tcg_insn_unit to the current uint8_t.
>
> Reviewed-by: Peter Maydell <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  include/exec/exec-all.h |   2 +-
>  tcg/tcg-be-ldst.h       |   4 +-
>  tcg/tcg.c               | 106 
> +++++++++++++++++++++++++++++++-----------------
>  tcg/tcg.h               |  39 ++++++++++++++----
>  translate-all.c         |  13 +++---
>  5 files changed, 108 insertions(+), 56 deletions(-)
>
<snip>
> @@ -2653,7 +2683,8 @@ static inline int tcg_gen_code_common(TCGContext *s, 
> uint8_t *gen_code_buf,
>          }
>          args += def->nb_args;
>      next:
> -        if (search_pc >= 0 && search_pc < s->code_ptr - gen_code_buf) {
> +        if (search_pc >= 0
> +            && search_pc < (intptr_t)s->code_ptr - (intptr_t)gen_code_buf) {
>              return op_index;
>          }

I thought we were trying to avoid excessive casting with pointer
arithmetic? Can this be made neater?

>          op_index++;
> @@ -2667,7 +2698,7 @@ static inline int tcg_gen_code_common(TCGContext *s, 
> uint8_t *gen_code_buf,
>      return -1;
>  }
>  
> -int tcg_gen_code(TCGContext *s, uint8_t *gen_code_buf)
> +int tcg_gen_code(TCGContext *s, tcg_insn_unit *gen_code_buf)
>  {
>  #ifdef CONFIG_PROFILER
>      {
> @@ -2686,16 +2717,17 @@ int tcg_gen_code(TCGContext *s, uint8_t *gen_code_buf)
>      tcg_gen_code_common(s, gen_code_buf, -1);
>  
>      /* flush instruction cache */
> -    flush_icache_range((uintptr_t)gen_code_buf, (uintptr_t)s->code_ptr);
> +    flush_icache_range((uintptr_t)s->code_buf, (uintptr_t)s->code_ptr);
>  
> -    return s->code_ptr -  gen_code_buf;
> +    return tcg_current_code_size(s);
>  }
>  
>  /* Return the index of the micro operation such as the pc after is <
>     offset bytes from the start of the TB.  The contents of gen_code_buf must
>     not be changed, though writing the same values is ok.
>     Return -1 if not found. */
> -int tcg_gen_code_search_pc(TCGContext *s, uint8_t *gen_code_buf, long offset)
> +int tcg_gen_code_search_pc(TCGContext *s, tcg_insn_unit *gen_code_buf,
> +                           long offset)
>  {
>      return tcg_gen_code_common(s, gen_code_buf, offset);
>  }
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index d38c92d..a3fb88c 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -146,10 +146,26 @@ typedef enum TCGOpcode {
>  #define tcg_regset_andnot(d, a, b) (d) = (a) & ~(b)
>  #define tcg_regset_not(d, a) (d) = ~(a)
>  
> +#ifndef TCG_TARGET_INSN_UNIT_SIZE
> +#define TCG_TARGET_INSN_UNIT_SIZE 1
> +#endif
> +#if TCG_TARGET_INSN_UNIT_SIZE == 1
> +typedef uint8_t tcg_insn_unit;
> +#elif TCG_TARGET_INSN_UNIT_SIZE == 2
> +typedef uint16_t tcg_insn_unit;
> +#elif TCG_TARGET_INSN_UNIT_SIZE == 4
> +typedef uint32_t tcg_insn_unit;
> +#elif TCG_TARGET_INSN_UNIT_SIZE == 8
> +typedef uint64_t tcg_insn_unit;
> +#else
> +/* The port better have done this.  */

Shouldn't we #error if the port hasn't done this then?

> +#endif
<snip>

Otherwise:

Reviewed-by: Alex Bennée <address@hidden>

-- 
Alex Bennée