Re: [PATCH 01/16] tcg: Add temp_readonly

[Philippe Mathieu-Daudé]

On 4/18/20 6:18 PM, Richard Henderson wrote:
> In most, but not all, places that we check for TEMP_FIXED,
> we are really testing that we do not modify the temporary.
> 
> Signed-off-by: Richard Henderson <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
>  include/tcg/tcg.h |  5 +++++
>  tcg/tcg.c         | 21 ++++++++++-----------
>  2 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
> index 3534dce77f..27e1b509a6 100644
> --- a/include/tcg/tcg.h
> +++ b/include/tcg/tcg.h
> @@ -678,6 +678,11 @@ struct TCGContext {
>      target_ulong gen_insn_data[TCG_MAX_INSNS][TARGET_INSN_START_WORDS];
>  };
>  
> +static inline bool temp_readonly(TCGTemp *ts)
> +{
> +    return ts->kind == TEMP_FIXED;
> +}
> +
>  extern TCGContext tcg_init_ctx;
>  extern __thread TCGContext *tcg_ctx;
>  extern TCGv_env cpu_env;
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index eaf81397a3..92b3767097 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -3132,7 +3132,7 @@ static void temp_load(TCGContext *, TCGTemp *, 
> TCGRegSet, TCGRegSet, TCGRegSet);
>     mark it free; otherwise mark it dead.  */
>  static void temp_free_or_dead(TCGContext *s, TCGTemp *ts, int free_or_dead)
>  {
> -    if (ts->kind == TEMP_FIXED) {
> +    if (temp_readonly(ts)) {
>          return;
>      }
>      if (ts->val_type == TEMP_VAL_REG) {
> @@ -3156,7 +3156,7 @@ static inline void temp_dead(TCGContext *s, TCGTemp *ts)
>  static void temp_sync(TCGContext *s, TCGTemp *ts, TCGRegSet allocated_regs,
>                        TCGRegSet preferred_regs, int free_or_dead)
>  {
> -    if (ts->kind == TEMP_FIXED) {
> +    if (temp_readonly(ts)) {
>          return;
>      }
>      if (!ts->mem_coherent) {
> @@ -3314,8 +3314,7 @@ static void temp_save(TCGContext *s, TCGTemp *ts, 
> TCGRegSet allocated_regs)
>  {
>      /* The liveness analysis already ensures that globals are back
>         in memory. Keep an tcg_debug_assert for safety. */
> -    tcg_debug_assert(ts->val_type == TEMP_VAL_MEM
> -                     || ts->kind == TEMP_FIXED);
> +    tcg_debug_assert(ts->val_type == TEMP_VAL_MEM || temp_readonly(ts));
>  }
>  
>  /* save globals to their canonical location and assume they can be
> @@ -3373,7 +3372,7 @@ static void tcg_reg_alloc_do_movi(TCGContext *s, 
> TCGTemp *ots,
>                                    TCGRegSet preferred_regs)
>  {
>      /* ENV should not be modified.  */
> -    tcg_debug_assert(ots->kind != TEMP_FIXED);
> +    tcg_debug_assert(!temp_readonly(ots));
>  
>      /* The movi is not explicitly generated here.  */
>      if (ots->val_type == TEMP_VAL_REG) {
> @@ -3413,7 +3412,7 @@ static void tcg_reg_alloc_mov(TCGContext *s, const 
> TCGOp *op)
>      ts = arg_temp(op->args[1]);
>  
>      /* ENV should not be modified.  */
> -    tcg_debug_assert(ots->kind != TEMP_FIXED);
> +    tcg_debug_assert(!temp_readonly(ots));
>  
>      /* Note that otype != itype for no-op truncation.  */
>      otype = ots->type;
> @@ -3474,7 +3473,7 @@ static void tcg_reg_alloc_mov(TCGContext *s, const 
> TCGOp *op)
>                   * Store the source register into the destination slot
>                   * and leave the destination temp as TEMP_VAL_MEM.
>                   */
> -                assert(ots->kind != TEMP_FIXED);
> +                assert(!temp_readonly(ots));
>                  if (!ts->mem_allocated) {
>                      temp_allocate_frame(s, ots);
>                  }
> @@ -3511,7 +3510,7 @@ static void tcg_reg_alloc_dup(TCGContext *s, const 
> TCGOp *op)
>      its = arg_temp(op->args[1]);
>  
>      /* ENV should not be modified.  */
> -    tcg_debug_assert(ots->kind != TEMP_FIXED);
> +    tcg_debug_assert(!temp_readonly(ots));
>  
>      itype = its->type;
>      vece = TCGOP_VECE(op);
> @@ -3742,7 +3741,7 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp 
> *op)
>              ts = arg_temp(arg);
>  
>              /* ENV should not be modified.  */
> -            tcg_debug_assert(ts->kind != TEMP_FIXED);
> +            tcg_debug_assert(!temp_readonly(ts));
>  
>              if ((arg_ct->ct & TCG_CT_ALIAS)
>                  && !const_args[arg_ct->alias_index]) {
> @@ -3784,7 +3783,7 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp 
> *op)
>          ts = arg_temp(op->args[i]);
>  
>          /* ENV should not be modified.  */
> -        tcg_debug_assert(ts->kind != TEMP_FIXED);
> +        tcg_debug_assert(!temp_readonly(ts));
>  
>          if (NEED_SYNC_ARG(i)) {
>              temp_sync(s, ts, o_allocated_regs, 0, IS_DEAD_ARG(i));
> @@ -3916,7 +3915,7 @@ static void tcg_reg_alloc_call(TCGContext *s, TCGOp *op)
>          ts = arg_temp(arg);
>  
>          /* ENV should not be modified.  */
> -        tcg_debug_assert(ts->kind != TEMP_FIXED);
> +        tcg_debug_assert(!temp_readonly(ts));
>  
>          reg = tcg_target_call_oarg_regs[i];
>          tcg_debug_assert(s->reg_to_temp[reg] == NULL);
>