diff mbox series

[v2,17/36] tcg/optimize: Adjust TempOptInfo allocation

Message ID 20200422011722.13287-18-richard.henderson@linaro.org
State Superseded
Headers show
Series tcg 5.1 omnibus patch set | expand

Commit Message

Richard Henderson April 22, 2020, 1:17 a.m. UTC
Do not allocate a large block for indexing.  Instead, allocate
for each temporary as they are seen.

In general, this will use less memory, if we consider that most
TBs do not touch every target register.  This also allows us to
allocate TempOptInfo for new temps created during optimization.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 tcg/optimize.c | 60 ++++++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 26 deletions(-)

-- 
2.20.1

Comments

Alex Bennée April 22, 2020, 5:53 p.m. UTC | #1
Richard Henderson <richard.henderson@linaro.org> writes:

> Do not allocate a large block for indexing.  Instead, allocate

> for each temporary as they are seen.

>

> In general, this will use less memory, if we consider that most

> TBs do not touch every target register.  This also allows us to

> allocate TempOptInfo for new temps created during optimization.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


> ---

>  tcg/optimize.c | 60 ++++++++++++++++++++++++++++----------------------

>  1 file changed, 34 insertions(+), 26 deletions(-)

>

> diff --git a/tcg/optimize.c b/tcg/optimize.c

> index b86bf3d707..d36d7e1d7f 100644

> --- a/tcg/optimize.c

> +++ b/tcg/optimize.c

> @@ -89,35 +89,41 @@ static void reset_temp(TCGArg arg)

>  }

>  

>  /* Initialize and activate a temporary.  */

> -static void init_ts_info(TempOptInfo *infos,

> -                         TCGTempSet *temps_used, TCGTemp *ts)

> +static void init_ts_info(TCGTempSet *temps_used, TCGTemp *ts)

>  {

>      size_t idx = temp_idx(ts);

> -    if (!test_bit(idx, temps_used->l)) {

> -        TempOptInfo *ti = &infos[idx];

> +    TempOptInfo *ti;

>  

> +    if (test_bit(idx, temps_used->l)) {

> +        return;

> +    }

> +    set_bit(idx, temps_used->l);

> +

> +    ti = ts->state_ptr;

> +    if (ti == NULL) {

> +        ti = tcg_malloc(sizeof(TempOptInfo));

>          ts->state_ptr = ti;

> -        ti->next_copy = ts;

> -        ti->prev_copy = ts;

> -        if (ts->kind == TEMP_CONST) {

> -            ti->is_const = true;

> -            ti->val = ti->mask = ts->val;

> -            if (TCG_TARGET_REG_BITS > 32 && ts->type == TCG_TYPE_I32) {

> -                /* High bits of a 32-bit quantity are garbage.  */

> -                ti->mask |= ~0xffffffffull;

> -            }

> -        } else {

> -            ti->is_const = false;

> -            ti->mask = -1;

> +    }

> +

> +    ti->next_copy = ts;

> +    ti->prev_copy = ts;

> +    if (ts->kind == TEMP_CONST) {

> +        ti->is_const = true;

> +        ti->val = ts->val;

> +        ti->mask = ts->val;

> +        if (TCG_TARGET_REG_BITS > 32 && ts->type == TCG_TYPE_I32) {

> +            /* High bits of a 32-bit quantity are garbage.  */

> +            ti->mask |= ~0xffffffffull;

>          }

> -        set_bit(idx, temps_used->l);

> +    } else {

> +        ti->is_const = false;

> +        ti->mask = -1;

>      }

>  }

>  

> -static void init_arg_info(TempOptInfo *infos,

> -                          TCGTempSet *temps_used, TCGArg arg)

> +static void init_arg_info(TCGTempSet *temps_used, TCGArg arg)

>  {

> -    init_ts_info(infos, temps_used, arg_temp(arg));

> +    init_ts_info(temps_used, arg_temp(arg));

>  }

>  

>  static TCGTemp *find_better_copy(TCGContext *s, TCGTemp *ts)

> @@ -603,9 +609,8 @@ static bool swap_commutative2(TCGArg *p1, TCGArg *p2)

>  /* Propagate constants and copies, fold constant expressions. */

>  void tcg_optimize(TCGContext *s)

>  {

> -    int nb_temps, nb_globals;

> +    int nb_temps, nb_globals, i;

>      TCGOp *op, *op_next, *prev_mb = NULL;

> -    TempOptInfo *infos;

>      TCGTempSet temps_used;

>  

>      /* Array VALS has an element for each temp.

> @@ -615,12 +620,15 @@ void tcg_optimize(TCGContext *s)

>  

>      nb_temps = s->nb_temps;

>      nb_globals = s->nb_globals;

> +

>      bitmap_zero(temps_used.l, nb_temps);

> -    infos = tcg_malloc(sizeof(TempOptInfo) * nb_temps);

> +    for (i = 0; i < nb_temps; ++i) {

> +        s->temps[i].state_ptr = NULL;

> +    }

>  

>      QTAILQ_FOREACH_SAFE(op, &s->ops, link, op_next) {

>          tcg_target_ulong mask, partmask, affected;

> -        int nb_oargs, nb_iargs, i;

> +        int nb_oargs, nb_iargs;

>          TCGArg tmp;

>          TCGOpcode opc = op->opc;

>          const TCGOpDef *def = &tcg_op_defs[opc];

> @@ -633,14 +641,14 @@ void tcg_optimize(TCGContext *s)

>              for (i = 0; i < nb_oargs + nb_iargs; i++) {

>                  TCGTemp *ts = arg_temp(op->args[i]);

>                  if (ts) {

> -                    init_ts_info(infos, &temps_used, ts);

> +                    init_ts_info(&temps_used, ts);

>                  }

>              }

>          } else {

>              nb_oargs = def->nb_oargs;

>              nb_iargs = def->nb_iargs;

>              for (i = 0; i < nb_oargs + nb_iargs; i++) {

> -                init_arg_info(infos, &temps_used, op->args[i]);

> +                init_arg_info(&temps_used, op->args[i]);

>              }

>          }



-- 
Alex Bennée
Alex Bennée April 22, 2020, 6:28 p.m. UTC | #2
Alex Bennée <alex.bennee@linaro.org> writes:

> Richard Henderson <richard.henderson@linaro.org> writes:

>

>> Do not allocate a large block for indexing.  Instead, allocate

>> for each temporary as they are seen.

>>

>> In general, this will use less memory, if we consider that most

>> TBs do not touch every target register.  This also allows us to

>> allocate TempOptInfo for new temps created during optimization.

>>

>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

>

> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

<snip>
>>  

>> -static void init_arg_info(TempOptInfo *infos,

>> -                          TCGTempSet *temps_used, TCGArg arg)

>> +static void init_arg_info(TCGTempSet *temps_used, TCGArg arg)

>>  {

>> -    init_ts_info(infos, temps_used, arg_temp(arg));

>> +    init_ts_info(temps_used, arg_temp(arg));

>>  }


Although I've noticed this function which is only called once where as
others call init_ts_info directly. Any reason to keep it around?

-- 
Alex Bennée
diff mbox series

Patch

diff --git a/tcg/optimize.c b/tcg/optimize.c
index b86bf3d707..d36d7e1d7f 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -89,35 +89,41 @@  static void reset_temp(TCGArg arg)
 }
 
 /* Initialize and activate a temporary.  */
-static void init_ts_info(TempOptInfo *infos,
-                         TCGTempSet *temps_used, TCGTemp *ts)
+static void init_ts_info(TCGTempSet *temps_used, TCGTemp *ts)
 {
     size_t idx = temp_idx(ts);
-    if (!test_bit(idx, temps_used->l)) {
-        TempOptInfo *ti = &infos[idx];
+    TempOptInfo *ti;
 
+    if (test_bit(idx, temps_used->l)) {
+        return;
+    }
+    set_bit(idx, temps_used->l);
+
+    ti = ts->state_ptr;
+    if (ti == NULL) {
+        ti = tcg_malloc(sizeof(TempOptInfo));
         ts->state_ptr = ti;
-        ti->next_copy = ts;
-        ti->prev_copy = ts;
-        if (ts->kind == TEMP_CONST) {
-            ti->is_const = true;
-            ti->val = ti->mask = ts->val;
-            if (TCG_TARGET_REG_BITS > 32 && ts->type == TCG_TYPE_I32) {
-                /* High bits of a 32-bit quantity are garbage.  */
-                ti->mask |= ~0xffffffffull;
-            }
-        } else {
-            ti->is_const = false;
-            ti->mask = -1;
+    }
+
+    ti->next_copy = ts;
+    ti->prev_copy = ts;
+    if (ts->kind == TEMP_CONST) {
+        ti->is_const = true;
+        ti->val = ts->val;
+        ti->mask = ts->val;
+        if (TCG_TARGET_REG_BITS > 32 && ts->type == TCG_TYPE_I32) {
+            /* High bits of a 32-bit quantity are garbage.  */
+            ti->mask |= ~0xffffffffull;
         }
-        set_bit(idx, temps_used->l);
+    } else {
+        ti->is_const = false;
+        ti->mask = -1;
     }
 }
 
-static void init_arg_info(TempOptInfo *infos,
-                          TCGTempSet *temps_used, TCGArg arg)
+static void init_arg_info(TCGTempSet *temps_used, TCGArg arg)
 {
-    init_ts_info(infos, temps_used, arg_temp(arg));
+    init_ts_info(temps_used, arg_temp(arg));
 }
 
 static TCGTemp *find_better_copy(TCGContext *s, TCGTemp *ts)
@@ -603,9 +609,8 @@  static bool swap_commutative2(TCGArg *p1, TCGArg *p2)
 /* Propagate constants and copies, fold constant expressions. */
 void tcg_optimize(TCGContext *s)
 {
-    int nb_temps, nb_globals;
+    int nb_temps, nb_globals, i;
     TCGOp *op, *op_next, *prev_mb = NULL;
-    TempOptInfo *infos;
     TCGTempSet temps_used;
 
     /* Array VALS has an element for each temp.
@@ -615,12 +620,15 @@  void tcg_optimize(TCGContext *s)
 
     nb_temps = s->nb_temps;
     nb_globals = s->nb_globals;
+
     bitmap_zero(temps_used.l, nb_temps);
-    infos = tcg_malloc(sizeof(TempOptInfo) * nb_temps);
+    for (i = 0; i < nb_temps; ++i) {
+        s->temps[i].state_ptr = NULL;
+    }
 
     QTAILQ_FOREACH_SAFE(op, &s->ops, link, op_next) {
         tcg_target_ulong mask, partmask, affected;
-        int nb_oargs, nb_iargs, i;
+        int nb_oargs, nb_iargs;
         TCGArg tmp;
         TCGOpcode opc = op->opc;
         const TCGOpDef *def = &tcg_op_defs[opc];
@@ -633,14 +641,14 @@  void tcg_optimize(TCGContext *s)
             for (i = 0; i < nb_oargs + nb_iargs; i++) {
                 TCGTemp *ts = arg_temp(op->args[i]);
                 if (ts) {
-                    init_ts_info(infos, &temps_used, ts);
+                    init_ts_info(&temps_used, ts);
                 }
             }
         } else {
             nb_oargs = def->nb_oargs;
             nb_iargs = def->nb_iargs;
             for (i = 0; i < nb_oargs + nb_iargs; i++) {
-                init_arg_info(infos, &temps_used, op->args[i]);
+                init_arg_info(&temps_used, op->args[i]);
             }
         }