Patchwork TCG: fix copy propagation

login
register
mail settings
Submitter Blue Swirl
Date Aug. 6, 2011, 9:26 p.m.
Message ID <CAAu8pHssiKJ48NGcJNe8X4K_hxk9Yq4Gi1hQw1YBvAsVEnbcYw@mail.gmail.com>
Download mbox | patch
Permalink /patch/108798/
State New
Headers show

Comments

Blue Swirl - Aug. 6, 2011, 9:26 p.m.
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 <blauwirbel@gmail.com>
---
 tcg/optimize.c |   15 +++++++++------
 tcg/tcg.h      |    5 +++++
 2 files changed, 14 insertions(+), 6 deletions(-)

  * code which is not supposed to leak any TCG temporaries, then
Stefan Weil - Aug. 7, 2011, 6:48 a.m.
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<blauwirbel@gmail.com>
> ---
>   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 <weil@mail.berlios.de>
Blue Swirl - Aug. 7, 2011, 9:36 a.m.
Thanks for testing, applied.

On Sun, Aug 7, 2011 at 6:48 AM, Stefan Weil <weil@mail.berlios.de> 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<blauwirbel@gmail.com>
>> ---
>>  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 <weil@mail.berlios.de>
>
>

Patch

From c06136b49409ba1c68457ce3ceffb5eeb355ec47 Mon Sep 17 00:00:00 2001
Message-Id: <c06136b49409ba1c68457ce3ceffb5eeb355ec47.1312665937.git.blauwirbel@gmail.com>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Sat, 6 Aug 2011 13:58:47 +0000
Subject: [PATCH] TCG: fix copy propagation

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 <blauwirbel@gmail.com>
---
 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
-- 
1.7.2.5