All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/2] tcg: improve dead computation
@ 2015-06-04 19:47 Aurelien Jarno
  2015-06-04 19:47 ` [Qemu-devel] [PATCH v3 1/2] tcg: fix register allocation with two aliased dead inputs Aurelien Jarno
  2015-06-04 19:47 ` [Qemu-devel] [PATCH v3 2/2] tcg: fix dead computation for repeated input arguments Aurelien Jarno
  0 siblings, 2 replies; 4+ messages in thread
From: Aurelien Jarno @ 2015-06-04 19:47 UTC (permalink / raw
  To: qemu-devel; +Cc: Aurelien Jarno, Richard Henderson

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH v3 1/2] tcg: fix register allocation with two aliased dead inputs
  2015-06-04 19:47 [Qemu-devel] [PATCH v3 0/2] tcg: improve dead computation Aurelien Jarno
@ 2015-06-04 19:47 ` Aurelien Jarno
  2015-06-04 21:17   ` Richard Henderson
  2015-06-04 19:47 ` [Qemu-devel] [PATCH v3 2/2] tcg: fix dead computation for repeated input arguments Aurelien Jarno
  1 sibling, 1 reply; 4+ messages in thread
From: Aurelien Jarno @ 2015-06-04 19:47 UTC (permalink / raw
  To: qemu-devel; +Cc: Aurelien Jarno, Richard Henderson

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH v3 2/2] tcg: fix dead computation for repeated input arguments
  2015-06-04 19:47 [Qemu-devel] [PATCH v3 0/2] tcg: improve dead computation Aurelien Jarno
  2015-06-04 19:47 ` [Qemu-devel] [PATCH v3 1/2] tcg: fix register allocation with two aliased dead inputs Aurelien Jarno
@ 2015-06-04 19:47 ` Aurelien Jarno
  1 sibling, 0 replies; 4+ messages in thread
From: Aurelien Jarno @ 2015-06-04 19:47 UTC (permalink / raw
  To: qemu-devel; +Cc: Aurelien Jarno, Richard Henderson

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/2] tcg: fix register allocation with two aliased dead inputs
  2015-06-04 19:47 ` [Qemu-devel] [PATCH v3 1/2] tcg: fix register allocation with two aliased dead inputs Aurelien Jarno
@ 2015-06-04 21:17   ` Richard Henderson
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2015-06-04 21:17 UTC (permalink / raw
  To: Aurelien Jarno, qemu-devel

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~

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-06-04 21:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-04 19:47 [Qemu-devel] [PATCH v3 0/2] tcg: improve dead computation Aurelien Jarno
2015-06-04 19:47 ` [Qemu-devel] [PATCH v3 1/2] tcg: fix register allocation with two aliased dead inputs Aurelien Jarno
2015-06-04 21:17   ` Richard Henderson
2015-06-04 19:47 ` [Qemu-devel] [PATCH v3 2/2] tcg: fix dead computation for repeated input arguments Aurelien Jarno

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.