Re: [Qemu-devel] [PATCH for-2.5 03/10] tcg/optimize: track const/copy status separately

[Alex Bennée]

Aurelien Jarno <address@hidden> writes:

> Use two bools to track constants and copies instead of an enum.

More of an explanation would be useful as to why, otherwise it seems to
me we are just increasing the size of the structure (assuming the
compiler uses the same base sizes for the bool and enum).


>
> Cc: Richard Henderson <address@hidden>
> Signed-off-by: Aurelien Jarno <address@hidden>
> ---
>  tcg/optimize.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index d2b63a4..f16eb1e 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -35,14 +35,9 @@
>          glue(glue(case INDEX_op_, x), _i32):    \
>          glue(glue(case INDEX_op_, x), _i64)
>  
> -typedef enum {
> -    TCG_TEMP_UNDEF = 0,
> -    TCG_TEMP_CONST,
> -    TCG_TEMP_COPY,
> -} tcg_temp_state;
> -
>  struct tcg_temp_info {
> -    tcg_temp_state state;
> +    bool is_const;
> +    bool is_copy;
>      uint16_t prev_copy;
>      uint16_t next_copy;
>      tcg_target_ulong val;
> @@ -54,12 +49,12 @@ static TCGTempSet temps_used;
>  
>  static inline bool temp_is_const(TCGArg arg)
>  {
> -    return temps[arg].state == TCG_TEMP_CONST;
> +    return temps[arg].is_const;
>  }
>  
>  static inline bool temp_is_copy(TCGArg arg)
>  {
> -    return temps[arg].state == TCG_TEMP_COPY;
> +    return temps[arg].is_copy;
>  }
>  
>  /* Reset TEMP's state to TCG_TEMP_UNDEF.  If TEMP only had one copy, remove
> @@ -68,13 +63,14 @@ static void reset_temp(TCGArg temp)
>  {
>      if (temp_is_copy(temp)) {
>          if (temps[temp].prev_copy == temps[temp].next_copy) {
> -            temps[temps[temp].next_copy].state = TCG_TEMP_UNDEF;
> +            temps[temps[temp].next_copy].is_copy = false;
>          } else {
>              temps[temps[temp].next_copy].prev_copy = temps[temp].prev_copy;
>              temps[temps[temp].prev_copy].next_copy = temps[temp].next_copy;
>          }
>      }
> -    temps[temp].state = TCG_TEMP_UNDEF;
> +    temps[temp].is_const = false;
> +    temps[temp].is_copy = false;
>      temps[temp].mask = -1;
>  }
>  
> @@ -88,7 +84,8 @@ static void reset_all_temps(int nb_temps)
>  static void init_temp_info(TCGArg temp)
>  {
>      if (!test_bit(temp, temps_used.l)) {
> -        temps[temp].state = TCG_TEMP_UNDEF;
> +        temps[temp].is_const = false;
> +        temps[temp].is_copy = false;
>          temps[temp].mask = -1;
>          set_bit(temp, temps_used.l);
>      }
> @@ -218,7 +215,8 @@ static void tcg_opt_gen_movi(TCGContext *s, TCGOp *op, 
> TCGArg *args,
>      op->opc = new_op;
>  
>      reset_temp(dst);
> -    temps[dst].state = TCG_TEMP_CONST;
> +    temps[dst].is_const = true;
> +    temps[dst].is_copy = false;
>      temps[dst].val = val;
>      mask = val;
>      if (TCG_TARGET_REG_BITS > 32 && new_op == INDEX_op_movi_i32) {
> @@ -261,11 +259,13 @@ static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, 
> TCGArg *args,
>  
>      if (s->temps[src].type == s->temps[dst].type) {
>          if (!temp_is_copy(src)) {
> -            temps[src].state = TCG_TEMP_COPY;
> +            temps[src].is_const = false;
> +            temps[src].is_copy = true;
>              temps[src].next_copy = src;
>              temps[src].prev_copy = src;
>          }
> -        temps[dst].state = TCG_TEMP_COPY;
> +        temps[dst].is_const = false;
> +        temps[dst].is_copy = true;
>          temps[dst].next_copy = temps[src].next_copy;
>          temps[dst].prev_copy = src;
>          temps[temps[dst].next_copy].prev_copy = dst;

-- 
Alex Bennée