[Qemu-devel] [PATCH v2 0/3] Rework copy propagation

[Qemu-devel] [PATCH v2 0/3] Rework copy propagation

[Aurelien Jarno]

This is the second attempt to rework the copy propagation and restore
it to its original state.

In the first version, optimization was possible around set_label, causing
a TCG crash in some cases. The first patch of this new series fix that.
The two others are unchanged.

Aurelien Jarno (3):
  tcg: mark set_label with TCG_OPF_BB_END flag
  tcg/optimize: fix end of basic bloc detection
  revert "TCG: fix copy propagation"

 tcg/optimize.c |   38 +++++++++++++++-----------------------
 tcg/tcg-opc.h  |    2 +-
 tcg/tcg.c      |    5 -----
 tcg/tcg.h      |    5 -----
 4 files changed, 16 insertions(+), 34 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH v2 1/3] tcg: mark set_label with TCG_OPF_BB_END flag

[Aurelien Jarno]

set_label is effectively the end of a basic bloc, as no optimization
can be made accross it. It was treated as such in the liveness analysis
code, but as a special case.

Mark it with TCG_OPF_BB_END flag so that this information can be used
by other parts of the TCG code, and remove the special case in the liveness
analysis code.

Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/tcg-opc.h |    2 +-
 tcg/tcg.c     |    5 -----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h
index 8e06d03..d12e8d0 100644
--- a/tcg/tcg-opc.h
+++ b/tcg/tcg-opc.h
@@ -36,7 +36,7 @@ DEF(nopn, 0, 0, 1, 0) /* variable number of parameters */
 
 DEF(discard, 1, 0, 0, 0)
 
-DEF(set_label, 0, 0, 1, 0)
+DEF(set_label, 0, 0, 1, TCG_OPF_BB_END)
 DEF(call, 0, 1, 2, TCG_OPF_SIDE_EFFECTS) /* variable number of parameters */
 DEF(jmp, 0, 1, 0, TCG_OPF_BB_END | TCG_OPF_SIDE_EFFECTS)
 DEF(br, 0, 0, 1, TCG_OPF_BB_END | TCG_OPF_SIDE_EFFECTS)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 8386b70..c002a88 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1297,11 +1297,6 @@ static void tcg_liveness_analysis(TCGContext *s)
                 args--;
             }
             break;
-        case INDEX_op_set_label:
-            args--;
-            /* mark end of basic block */
-            tcg_la_bb_end(s, dead_temps);
-            break;
         case INDEX_op_debug_insn_start:
             args -= def->nb_args;
             break;
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 2/3] tcg/optimize: fix end of basic bloc detection

[Aurelien Jarno]

Commit e31b0a7c050711884ad570fe73df806520953618 fixed copy propagation on
32-bit host by restricting the copy between different types. This was the
wrong fix.

The real problem is that the all temps states should be reset at the end
of a basic bloc. This was done by adding such operations in the switch,
but brcond2 was forgotten (that's why the crash was only observed on 32-bit
hosts).

Fix that by looking at the TCG_OPF_BB_END instead. We need to keep the case
for op_set_label as temps might be modified through another path.

Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/optimize.c |   23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/tcg/optimize.c b/tcg/optimize.c
index 9c65474..64aa35b 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -487,22 +487,17 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr,
                 i--;
             }
             break;
-        case INDEX_op_set_label:
-        case INDEX_op_jmp:
-        case INDEX_op_br:
-        CASE_OP_32_64(brcond):
-            memset(temps, 0, nb_temps * sizeof(struct tcg_temp_info));
-            for (i = 0; i < def->nb_args; i++) {
-                *gen_args = *args;
-                args++;
-                gen_args++;
-            }
-            break;
         default:
             /* Default case: we do know nothing about operation so no
-               propagation is done.  We only trash output args.  */
-            for (i = 0; i < def->nb_oargs; i++) {
-                reset_temp(args[i], nb_temps, nb_globals);
+               propagation is done.  We trash everything if the operation
+               is the end of a basic block, otherwise we only trash the
+               output args.  */
+            if (def->flags & TCG_OPF_BB_END) {
+                memset(temps, 0, nb_temps * sizeof(struct tcg_temp_info));
+            } else {
+                for (i = 0; i < def->nb_oargs; i++) {
+                    reset_temp(args[i], nb_temps, nb_globals);
+                }
             }
             for (i = 0; i < def->nb_args; i++) {
                 gen_args[i] = args[i];
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 3/3] revert "TCG: fix copy propagation"

[Aurelien Jarno]

Given the copy propagation breakage on 32-bit hosts has been fixed
commit e31b0a7c050711884ad570fe73df806520953618 can be reverted.

Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/optimize.c |   15 ++++++---------
 tcg/tcg.h      |    5 -----
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/tcg/optimize.c b/tcg/optimize.c
index 64aa35b..c312c8f 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -107,15 +107,12 @@ static TCGOpcode op_to_movi(TCGOpcode op)
     }
 }
 
-static void tcg_opt_gen_mov(TCGContext *s, TCGArg *gen_args, TCGArg dst,
-                            TCGArg src, int nb_temps, int nb_globals)
+static void tcg_opt_gen_mov(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);
-        /* 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)) {
+        if (src >= nb_globals) {
             assert(temps[src].state != TCG_TEMP_CONST);
             if (temps[src].state != TCG_TEMP_HAS_COPY) {
                 temps[src].state = TCG_TEMP_HAS_COPY;
@@ -344,7 +341,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(s, gen_args, args[0], args[1],
+                    tcg_opt_gen_mov(gen_args, args[0], args[1],
                                     nb_temps, nb_globals);
                     gen_args += 2;
                     args += 3;
@@ -370,7 +367,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(s, gen_args, args[0], args[1], nb_temps,
+                    tcg_opt_gen_mov(gen_args, args[0], args[1], nb_temps,
                                     nb_globals);
                     gen_args += 2;
                     args += 3;
@@ -395,7 +392,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(s, gen_args, args[0], args[1],
+                tcg_opt_gen_mov(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 d710694..8fbbc81 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -458,11 +458,6 @@ 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.10.4