[Qemu-devel] [PATCH v3 0/2] tcg: improve dead computation

[Qemu-devel] [PATCH v3 0/2] tcg: improve dead computation

[Aurelien Jarno]

This patchset is composed of two patches. The last one that has already
been sent and reviewed on the mailing list. However, while I believe it
is correct, it trigger a bug in the register allocation, causing some
regressions, especially on 32-bit hosts.

The first patch therefore fix the bug triggered by the second one.

Aurelien Jarno (2):
  tcg: fix register allocation with two aliased dead inputs
  tcg: fix dead computation for repeated input arguments

 tcg/tcg.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH v3 1/2] tcg: fix register allocation with two aliased dead inputs

[Aurelien Jarno]

For TCG ops with two outputs registers (add2, sub2, div2, div2u), when
the same input temp is used for the two inputs aliased to the two
outputs, and when these inputs are both dead, the register allocation
code wrongly assigned the same register to the same output.

This happens for example with sub2 t1, t2, t3, t3, t4, t5, when t3 is
not used anymore after the TCG op.  In that case the same register is
used for t1, t2 and t3.

The fix is to look for already allocated aliased input when allocating
a dead aliased input and check that the register is not already
used.

Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/tcg.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 8b43bbb..c5e2ce9 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1998,6 +1998,16 @@ static void tcg_reg_alloc_op(TCGContext *s,
                 if (!IS_DEAD_ARG(i)) {
                     goto allocate_in_reg;
                 }
+                /* check if the current register has already been allocated
+                   for another input aliased to an output */
+                int k2, i2;
+                for (k2 = 0 ; k2 < k ; k2++) {
+                    i2 = def->sorted_args[nb_oargs + k2];
+                    if ((def->args_ct[i2].ct & TCG_CT_IALIAS) &&
+                        (new_args[i2] == ts->reg)) {
+                        goto allocate_in_reg;
+                    }
+                }
             }
         }
         reg = ts->reg;
-- 
2.1.4

Re: [Qemu-devel] [PATCH v3 1/2] tcg: fix register allocation with two aliased dead inputs

[Richard Henderson]

On 06/04/2015 12:47 PM, Aurelien Jarno wrote:
> For TCG ops with two outputs registers (add2, sub2, div2, div2u), when
> the same input temp is used for the two inputs aliased to the two
> outputs, and when these inputs are both dead, the register allocation
> code wrongly assigned the same register to the same output.
> 
> This happens for example with sub2 t1, t2, t3, t3, t4, t5, when t3 is
> not used anymore after the TCG op.  In that case the same register is
> used for t1, t2 and t3.
> 
> The fix is to look for already allocated aliased input when allocating
> a dead aliased input and check that the register is not already
> used.
> 
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  tcg/tcg.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH v3 2/2] tcg: fix dead computation for repeated input arguments

[Aurelien Jarno]

When the same temp is used twice or more as an input argument to a TCG
instruction, the dead computation code doesn't recognize the second use
as a dead temp. This is because the temp is marked as live in the same
loop where dead inputs are checked.

The fix is to split the loop in two parts. This avoid emitting a move
and using a register for the movcond instruction when used as "move if
true" on x86-64. This might bring more improvements on RISC TCG targets
which don't have outputs aliased to inputs.

Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/tcg.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index c5e2ce9..4aa82c9 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1378,16 +1378,20 @@ static void tcg_liveness_analysis(TCGContext *s)
                         memset(dead_temps, 1, s->nb_globals);
                     }
 
-                    /* input args are live */
+                    /* record arguments that die in this helper */
                     for (i = nb_oargs; i < nb_iargs + nb_oargs; i++) {
                         arg = args[i];
                         if (arg != TCG_CALL_DUMMY_ARG) {
                             if (dead_temps[arg]) {
                                 dead_args |= (1 << i);
                             }
-                            dead_temps[arg] = 0;
                         }
                     }
+                    /* input arguments are live for preceeding opcodes */
+                    for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) {
+                        arg = args[i];
+                        dead_temps[arg] = 0;
+                    }
                     s->op_dead_args[oi] = dead_args;
                     s->op_sync_args[oi] = sync_args;
                 }
@@ -1522,12 +1526,16 @@ static void tcg_liveness_analysis(TCGContext *s)
                     memset(mem_temps, 1, s->nb_globals);
                 }
 
-                /* input args are live */
+                /* record arguments that die in this opcode */
                 for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) {
                     arg = args[i];
                     if (dead_temps[arg]) {
                         dead_args |= (1 << i);
                     }
+                }
+                /* input arguments are live for preceeding opcodes */
+                for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) {
+                    arg = args[i];
                     dead_temps[arg] = 0;
                 }
                 s->op_dead_args[oi] = dead_args;
-- 
2.1.4