Re: [Qemu-devel] [PATCH] TCG: fix copy propagation

[Blue Swirl]

Thanks for testing, applied.

On Sun, Aug 7, 2011 at 6:48 AM, Stefan Weil <address@hidden> wrote:
> Am 06.08.2011 23:26, schrieb Blue Swirl:
>>
>> Copy propagation introduced in 22613af4a6d9602001e6d0e7b6d98aa40aa018dc
>> considered only global registers. However, register temps and stack
>> allocated locals must be handled differently because register temps
>> don't survive across brcond.
>>
>> Fix by propagating only within same class of temps.
>>
>> Signed-off-by: Blue Swirl<address@hidden>
>> ---
>>  tcg/optimize.c |   15 +++++++++------
>>  tcg/tcg.h      |    5 +++++
>>  2 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/tcg/optimize.c b/tcg/optimize.c
>> index a3bfa5e..7eb5eb1 100644
>> --- a/tcg/optimize.c
>> +++ b/tcg/optimize.c
>> @@ -185,12 +185,15 @@ static int op_to_movi(int op)
>>      }
>>  }
>>
>> -static void tcg_opt_gen_mov(TCGArg *gen_args, TCGArg dst, TCGArg src,
>> -                            int nb_temps, int nb_globals)
>> +static void tcg_opt_gen_mov(TCGContext *s, TCGArg *gen_args, TCGArg dst,
>> +                            TCGArg src, int nb_temps, int nb_globals)
>>  {
>>          reset_temp(dst, nb_temps, nb_globals);
>>          assert(temps[src].state != TCG_TEMP_COPY);
>> -        if (src>= nb_globals) {
>> +        /* Don't try to copy if one of temps is a global or either one
>> +           is local and another is register */
>> +        if (src>= nb_globals&&  dst>= nb_globals&&
>> +            tcg_arg_is_local(s, src) == tcg_arg_is_local(s, dst)) {
>>              assert(temps[src].state != TCG_TEMP_CONST);
>>              if (temps[src].state != TCG_TEMP_HAS_COPY) {
>>                  temps[src].state = TCG_TEMP_HAS_COPY;
>> @@ -474,7 +477,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
>> uint16_t *tcg_opc_ptr,
>>                      gen_opc_buf[op_index] = INDEX_op_nop;
>>                  } else {
>>                      gen_opc_buf[op_index] = op_to_mov(op);
>> -                    tcg_opt_gen_mov(gen_args, args[0], args[1],
>> +                    tcg_opt_gen_mov(s, gen_args, args[0], args[1],
>>                                      nb_temps, nb_globals);
>>                      gen_args += 2;
>>                      args += 3;
>> @@ -500,7 +503,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
>> uint16_t *tcg_opc_ptr,
>>                      gen_opc_buf[op_index] = INDEX_op_nop;
>>                  } else {
>>                      gen_opc_buf[op_index] = op_to_mov(op);
>> -                    tcg_opt_gen_mov(gen_args, args[0], args[1], nb_temps,
>> +                    tcg_opt_gen_mov(s, gen_args, args[0], args[1],
>> nb_temps,
>>                                      nb_globals);
>>                      gen_args += 2;
>>                      args += 3;
>> @@ -523,7 +526,7 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
>> uint16_t *tcg_opc_ptr,
>>                  break;
>>              }
>>              if (temps[args[1]].state != TCG_TEMP_CONST) {
>> -                tcg_opt_gen_mov(gen_args, args[0], args[1],
>> +                tcg_opt_gen_mov(s, gen_args, args[0], args[1],
>>                                  nb_temps, nb_globals);
>>                  gen_args += 2;
>>                  args += 2;
>> diff --git a/tcg/tcg.h b/tcg/tcg.h
>> index e76f9af..e2a7095 100644
>> --- a/tcg/tcg.h
>> +++ b/tcg/tcg.h
>> @@ -410,6 +410,11 @@ static inline TCGv_i64 tcg_temp_local_new_i64(void)
>>  void tcg_temp_free_i64(TCGv_i64 arg);
>>  char *tcg_get_arg_str_i64(TCGContext *s, char *buf, int buf_size,
>> TCGv_i64 arg);
>>
>> +static inline bool tcg_arg_is_local(TCGContext *s, TCGArg arg)
>> +{
>> +    return s->temps[arg].temp_local;
>> +}
>> +
>>  #if defined(CONFIG_DEBUG_TCG)
>>  /* If you call tcg_clear_temp_count() at the start of a section of
>>   * code which is not supposed to leak any TCG temporaries, then
>
> This fixes qemu-system-x86_64 and qemu-system-mips64(el) on 32 bit hosts.
>
> Tested-by: Stefan Weil <address@hidden>
>
>