[Qemu-devel] [PATCH v1 0/5] tcg/optimize: small cleanups

[Qemu-devel] [PATCH v1 0/5] tcg/optimize: small cleanups

[Aurelien Jarno]

This patch series does some small cleanups to make the optimizer code
more readable. It doesn't bring any functional change. 

Aurelien Jarno (5):
  tcg/optimize: remove opc argument from tcg_opt_gen_movi
  tcg/optimize: remove opc argument from tcg_opt_gen_mov
  tcg/optimize: fold temp copies test in tcg_opt_gen_mov
  tcg/optimize: fold constant test in tcg_opt_gen_mov
  tcg/optimize: rename tcg_constant_folding

 tcg/optimize.c | 149 +++++++++++++++++++++++----------------------------------
 1 file changed, 59 insertions(+), 90 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH v1 1/5] tcg/optimize: remove opc argument from tcg_opt_gen_movi

[Aurelien Jarno]

We can get the opcode using the TCGOp pointer. It needs to be
dereferenced, but it's anyway done a few lines below to write
the new value.

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

diff --git a/tcg/optimize.c b/tcg/optimize.c
index 585f1ed..0bfc9a5 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -229,9 +229,9 @@ static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, TCGArg *args,
 }
 
 static void tcg_opt_gen_movi(TCGContext *s, TCGOp *op, TCGArg *args,
-                             TCGOpcode old_op, TCGArg dst, TCGArg val)
+                             TCGArg dst, TCGArg val)
 {
-    TCGOpcode new_op = op_to_movi(old_op);
+    TCGOpcode new_op = op_to_movi(op->opc);
     tcg_target_ulong mask;
 
     op->opc = new_op;
@@ -670,7 +670,7 @@ static void tcg_constant_folding(TCGContext *s)
         CASE_OP_32_64(rotr):
             if (temps[args[1]].state == TCG_TEMP_CONST
                 && temps[args[1]].val == 0) {
-                tcg_opt_gen_movi(s, op, args, opc, args[0], 0);
+                tcg_opt_gen_movi(s, op, args, args[0], 0);
                 continue;
             }
             break;
@@ -942,7 +942,7 @@ static void tcg_constant_folding(TCGContext *s)
 
         if (partmask == 0) {
             assert(nb_oargs == 1);
-            tcg_opt_gen_movi(s, op, args, opc, args[0], 0);
+            tcg_opt_gen_movi(s, op, args, args[0], 0);
             continue;
         }
         if (affected == 0) {
@@ -952,7 +952,7 @@ static void tcg_constant_folding(TCGContext *s)
             } else if (temps[args[1]].state != TCG_TEMP_CONST) {
                 tcg_opt_gen_mov(s, op, args, opc, args[0], args[1]);
             } else {
-                tcg_opt_gen_movi(s, op, args, opc,
+                tcg_opt_gen_movi(s, op, args,
                                  args[0], temps[args[1]].val);
             }
             continue;
@@ -966,7 +966,7 @@ static void tcg_constant_folding(TCGContext *s)
         CASE_OP_32_64(mulsh):
             if ((temps[args[2]].state == TCG_TEMP_CONST
                 && temps[args[2]].val == 0)) {
-                tcg_opt_gen_movi(s, op, args, opc, args[0], 0);
+                tcg_opt_gen_movi(s, op, args, args[0], 0);
                 continue;
             }
             break;
@@ -984,7 +984,7 @@ static void tcg_constant_folding(TCGContext *s)
                 } else if (temps[args[1]].state != TCG_TEMP_CONST) {
                     tcg_opt_gen_mov(s, op, args, opc, args[0], args[1]);
                 } else {
-                    tcg_opt_gen_movi(s, op, args, opc,
+                    tcg_opt_gen_movi(s, op, args,
                                      args[0], temps[args[1]].val);
                 }
                 continue;
@@ -1000,7 +1000,7 @@ static void tcg_constant_folding(TCGContext *s)
         CASE_OP_32_64(sub):
         CASE_OP_32_64(xor):
             if (temps_are_copies(args[1], args[2])) {
-                tcg_opt_gen_movi(s, op, args, opc, args[0], 0);
+                tcg_opt_gen_movi(s, op, args, args[0], 0);
                 continue;
             }
             break;
@@ -1026,7 +1026,7 @@ static void tcg_constant_folding(TCGContext *s)
             args[1] = temps[args[1]].val;
             /* fallthrough */
         CASE_OP_32_64(movi):
-            tcg_opt_gen_movi(s, op, args, opc, args[0], args[1]);
+            tcg_opt_gen_movi(s, op, args, args[0], args[1]);
             break;
 
         CASE_OP_32_64(not):
@@ -1039,7 +1039,7 @@ static void tcg_constant_folding(TCGContext *s)
         case INDEX_op_ext32u_i64:
             if (temps[args[1]].state == TCG_TEMP_CONST) {
                 tmp = do_constant_folding(opc, temps[args[1]].val, 0);
-                tcg_opt_gen_movi(s, op, args, opc, args[0], tmp);
+                tcg_opt_gen_movi(s, op, args, args[0], tmp);
                 break;
             }
             goto do_default;
@@ -1047,7 +1047,7 @@ static void tcg_constant_folding(TCGContext *s)
         case INDEX_op_trunc_shr_i32:
             if (temps[args[1]].state == TCG_TEMP_CONST) {
                 tmp = do_constant_folding(opc, temps[args[1]].val, args[2]);
-                tcg_opt_gen_movi(s, op, args, opc, args[0], tmp);
+                tcg_opt_gen_movi(s, op, args, args[0], tmp);
                 break;
             }
             goto do_default;
@@ -1078,7 +1078,7 @@ static void tcg_constant_folding(TCGContext *s)
                 && temps[args[2]].state == TCG_TEMP_CONST) {
                 tmp = do_constant_folding(opc, temps[args[1]].val,
                                           temps[args[2]].val);
-                tcg_opt_gen_movi(s, op, args, opc, args[0], tmp);
+                tcg_opt_gen_movi(s, op, args, args[0], tmp);
                 break;
             }
             goto do_default;
@@ -1088,7 +1088,7 @@ static void tcg_constant_folding(TCGContext *s)
                 && temps[args[2]].state == TCG_TEMP_CONST) {
                 tmp = deposit64(temps[args[1]].val, args[3], args[4],
                                 temps[args[2]].val);
-                tcg_opt_gen_movi(s, op, args, opc, args[0], tmp);
+                tcg_opt_gen_movi(s, op, args, args[0], tmp);
                 break;
             }
             goto do_default;
@@ -1096,7 +1096,7 @@ static void tcg_constant_folding(TCGContext *s)
         CASE_OP_32_64(setcond):
             tmp = do_constant_folding_cond(opc, args[1], args[2], args[3]);
             if (tmp != 2) {
-                tcg_opt_gen_movi(s, op, args, opc, args[0], tmp);
+                tcg_opt_gen_movi(s, op, args, args[0], tmp);
                 break;
             }
             goto do_default;
@@ -1121,7 +1121,7 @@ static void tcg_constant_folding(TCGContext *s)
                 if (temps_are_copies(args[0], args[4-tmp])) {
                     tcg_op_remove(s, op);
                 } else if (temps[args[4-tmp]].state == TCG_TEMP_CONST) {
-                    tcg_opt_gen_movi(s, op, args, opc,
+                    tcg_opt_gen_movi(s, op, args,
                                      args[0], temps[args[4-tmp]].val);
                 } else {
                     tcg_opt_gen_mov(s, op, args, opc, args[0], args[4-tmp]);
@@ -1154,8 +1154,8 @@ static void tcg_constant_folding(TCGContext *s)
 
                 rl = args[0];
                 rh = args[1];
-                tcg_opt_gen_movi(s, op, args, opc, rl, (uint32_t)a);
-                tcg_opt_gen_movi(s, op2, args2, opc, rh, (uint32_t)(a >> 32));
+                tcg_opt_gen_movi(s, op, args, rl, (uint32_t)a);
+                tcg_opt_gen_movi(s, op2, args2, rh, (uint32_t)(a >> 32));
 
                 /* We've done all we need to do with the movi.  Skip it.  */
                 oi_next = op2->next;
@@ -1175,8 +1175,8 @@ static void tcg_constant_folding(TCGContext *s)
 
                 rl = args[0];
                 rh = args[1];
-                tcg_opt_gen_movi(s, op, args, opc, rl, (uint32_t)r);
-                tcg_opt_gen_movi(s, op2, args2, opc, rh, (uint32_t)(r >> 32));
+                tcg_opt_gen_movi(s, op, args, rl, (uint32_t)r);
+                tcg_opt_gen_movi(s, op2, args2, rh, (uint32_t)(r >> 32));
 
                 /* We've done all we need to do with the movi.  Skip it.  */
                 oi_next = op2->next;
@@ -1260,7 +1260,7 @@ static void tcg_constant_folding(TCGContext *s)
             tmp = do_constant_folding_cond2(&args[1], &args[3], args[5]);
             if (tmp != 2) {
             do_setcond_const:
-                tcg_opt_gen_movi(s, op, args, opc, args[0], tmp);
+                tcg_opt_gen_movi(s, op, args, args[0], tmp);
             } else if ((args[5] == TCG_COND_LT || args[5] == TCG_COND_GE)
                        && temps[args[3]].state == TCG_TEMP_CONST
                        && temps[args[4]].state == TCG_TEMP_CONST
-- 
2.1.4

[Qemu-devel] [PATCH v1 2/5] tcg/optimize: remove opc argument from tcg_opt_gen_mov

[Aurelien Jarno]

We can get the opcode using the TCGOp pointer. It needs to be
dereferenced, but it's anyway done a few lines below to write
the new value.

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

diff --git a/tcg/optimize.c b/tcg/optimize.c
index 0bfc9a5..db15976 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -194,9 +194,9 @@ static bool temps_are_copies(TCGArg arg1, TCGArg arg2)
 }
 
 static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, TCGArg *args,
-                            TCGOpcode old_op, TCGArg dst, TCGArg src)
+                            TCGArg dst, TCGArg src)
 {
-    TCGOpcode new_op = op_to_mov(old_op);
+    TCGOpcode new_op = op_to_mov(op->opc);
     tcg_target_ulong mask;
 
     op->opc = new_op;
@@ -791,7 +791,7 @@ static void tcg_constant_folding(TCGContext *s)
             if (temps_are_copies(args[0], args[1])) {
                 tcg_op_remove(s, op);
             } else {
-                tcg_opt_gen_mov(s, op, args, opc, args[0], args[1]);
+                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
             }
             continue;
         default:
@@ -950,7 +950,7 @@ static void tcg_constant_folding(TCGContext *s)
             if (temps_are_copies(args[0], args[1])) {
                 tcg_op_remove(s, op);
             } else if (temps[args[1]].state != TCG_TEMP_CONST) {
-                tcg_opt_gen_mov(s, op, args, opc, args[0], args[1]);
+                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
             } else {
                 tcg_opt_gen_movi(s, op, args,
                                  args[0], temps[args[1]].val);
@@ -982,7 +982,7 @@ static void tcg_constant_folding(TCGContext *s)
                 if (temps_are_copies(args[0], args[1])) {
                     tcg_op_remove(s, op);
                 } else if (temps[args[1]].state != TCG_TEMP_CONST) {
-                    tcg_opt_gen_mov(s, op, args, opc, args[0], args[1]);
+                    tcg_opt_gen_mov(s, op, args, args[0], args[1]);
                 } else {
                     tcg_opt_gen_movi(s, op, args,
                                      args[0], temps[args[1]].val);
@@ -1018,7 +1018,7 @@ static void tcg_constant_folding(TCGContext *s)
                 break;
             }
             if (temps[args[1]].state != TCG_TEMP_CONST) {
-                tcg_opt_gen_mov(s, op, args, opc, args[0], args[1]);
+                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
                 break;
             }
             /* Source argument is constant.  Rewrite the operation and
@@ -1124,7 +1124,7 @@ static void tcg_constant_folding(TCGContext *s)
                     tcg_opt_gen_movi(s, op, args,
                                      args[0], temps[args[4-tmp]].val);
                 } else {
-                    tcg_opt_gen_mov(s, op, args, opc, args[0], args[4-tmp]);
+                    tcg_opt_gen_mov(s, op, args, args[0], args[4-tmp]);
                 }
                 break;
             }
-- 
2.1.4

[Qemu-devel] [PATCH v1 3/5] tcg/optimize: fold temp copies test in tcg_opt_gen_mov

[Aurelien Jarno]

Each call to tcg_opt_gen_mov is preceeded by a test to check if the
source and destination temps are copies. Fold that into the
tcg_opt_gen_mov function.

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

diff --git a/tcg/optimize.c b/tcg/optimize.c
index db15976..d5c0398 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -196,6 +196,11 @@ static bool temps_are_copies(TCGArg arg1, TCGArg arg2)
 static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, TCGArg *args,
                             TCGArg dst, TCGArg src)
 {
+    if (temps_are_copies(dst, src)) {
+        tcg_op_remove(s, op);
+        return;
+    }
+
     TCGOpcode new_op = op_to_mov(op->opc);
     tcg_target_ulong mask;
 
@@ -788,11 +793,7 @@ static void tcg_constant_folding(TCGContext *s)
             }
             break;
         do_mov3:
-            if (temps_are_copies(args[0], args[1])) {
-                tcg_op_remove(s, op);
-            } else {
-                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
-            }
+            tcg_opt_gen_mov(s, op, args, args[0], args[1]);
             continue;
         default:
             break;
@@ -947,9 +948,7 @@ static void tcg_constant_folding(TCGContext *s)
         }
         if (affected == 0) {
             assert(nb_oargs == 1);
-            if (temps_are_copies(args[0], args[1])) {
-                tcg_op_remove(s, op);
-            } else if (temps[args[1]].state != TCG_TEMP_CONST) {
+            if (temps[args[1]].state != TCG_TEMP_CONST) {
                 tcg_opt_gen_mov(s, op, args, args[0], args[1]);
             } else {
                 tcg_opt_gen_movi(s, op, args,
@@ -979,9 +978,7 @@ static void tcg_constant_folding(TCGContext *s)
         CASE_OP_32_64(or):
         CASE_OP_32_64(and):
             if (temps_are_copies(args[1], args[2])) {
-                if (temps_are_copies(args[0], args[1])) {
-                    tcg_op_remove(s, op);
-                } else if (temps[args[1]].state != TCG_TEMP_CONST) {
+                if (temps[args[1]].state != TCG_TEMP_CONST) {
                     tcg_opt_gen_mov(s, op, args, args[0], args[1]);
                 } else {
                     tcg_opt_gen_movi(s, op, args,
@@ -1013,10 +1010,6 @@ static void tcg_constant_folding(TCGContext *s)
            allocator where needed and possible.  Also detect copies. */
         switch (opc) {
         CASE_OP_32_64(mov):
-            if (temps_are_copies(args[0], args[1])) {
-                tcg_op_remove(s, op);
-                break;
-            }
             if (temps[args[1]].state != TCG_TEMP_CONST) {
                 tcg_opt_gen_mov(s, op, args, args[0], args[1]);
                 break;
@@ -1118,9 +1111,7 @@ static void tcg_constant_folding(TCGContext *s)
         CASE_OP_32_64(movcond):
             tmp = do_constant_folding_cond(opc, args[1], args[2], args[5]);
             if (tmp != 2) {
-                if (temps_are_copies(args[0], args[4-tmp])) {
-                    tcg_op_remove(s, op);
-                } else if (temps[args[4-tmp]].state == TCG_TEMP_CONST) {
+                if (temps[args[4-tmp]].state == TCG_TEMP_CONST) {
                     tcg_opt_gen_movi(s, op, args,
                                      args[0], temps[args[4-tmp]].val);
                 } else {
-- 
2.1.4

[Qemu-devel] [PATCH v1 4/5] tcg/optimize: fold constant test in tcg_opt_gen_mov

[Aurelien Jarno]

Most of the calls to tcg_opt_gen_mov are preceeded by a test to check if
the source temp is a constant. Fold that into the tcg_opt_gen_mov
function.

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

diff --git a/tcg/optimize.c b/tcg/optimize.c
index d5c0398..a8dac85 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -193,9 +193,36 @@ static bool temps_are_copies(TCGArg arg1, TCGArg arg2)
     return false;
 }
 
+static void tcg_opt_gen_movi(TCGContext *s, TCGOp *op, TCGArg *args,
+                             TCGArg dst, TCGArg val)
+{
+    TCGOpcode new_op = op_to_movi(op->opc);
+    tcg_target_ulong mask;
+
+    op->opc = new_op;
+
+    reset_temp(dst);
+    temps[dst].state = TCG_TEMP_CONST;
+    temps[dst].val = val;
+    mask = val;
+    if (TCG_TARGET_REG_BITS > 32 && new_op == INDEX_op_mov_i32) {
+        /* High bits of the destination are now garbage.  */
+        mask |= ~0xffffffffull;
+    }
+    temps[dst].mask = mask;
+
+    args[0] = dst;
+    args[1] = val;
+}
+
 static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, TCGArg *args,
                             TCGArg dst, TCGArg src)
 {
+    if (temps[src].state == TCG_TEMP_CONST) {
+        tcg_opt_gen_movi(s, op, args, dst, temps[src].val);
+        return;
+    }
+
     if (temps_are_copies(dst, src)) {
         tcg_op_remove(s, op);
         return;
@@ -233,28 +260,6 @@ static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, TCGArg *args,
     args[1] = src;
 }
 
-static void tcg_opt_gen_movi(TCGContext *s, TCGOp *op, TCGArg *args,
-                             TCGArg dst, TCGArg val)
-{
-    TCGOpcode new_op = op_to_movi(op->opc);
-    tcg_target_ulong mask;
-
-    op->opc = new_op;
-
-    reset_temp(dst);
-    temps[dst].state = TCG_TEMP_CONST;
-    temps[dst].val = val;
-    mask = val;
-    if (TCG_TARGET_REG_BITS > 32 && new_op == INDEX_op_mov_i32) {
-        /* High bits of the destination are now garbage.  */
-        mask |= ~0xffffffffull;
-    }
-    temps[dst].mask = mask;
-
-    args[0] = dst;
-    args[1] = val;
-}
-
 static TCGArg do_constant_folding_2(TCGOpcode op, TCGArg x, TCGArg y)
 {
     uint64_t l64, h64;
@@ -780,7 +785,8 @@ static void tcg_constant_folding(TCGContext *s)
             if (temps[args[1]].state != TCG_TEMP_CONST
                 && temps[args[2]].state == TCG_TEMP_CONST
                 && temps[args[2]].val == 0) {
-                goto do_mov3;
+                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
+                continue;
             }
             break;
         CASE_OP_32_64(and):
@@ -789,12 +795,10 @@ static void tcg_constant_folding(TCGContext *s)
             if (temps[args[1]].state != TCG_TEMP_CONST
                 && temps[args[2]].state == TCG_TEMP_CONST
                 && temps[args[2]].val == -1) {
-                goto do_mov3;
+                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
+                continue;
             }
             break;
-        do_mov3:
-            tcg_opt_gen_mov(s, op, args, args[0], args[1]);
-            continue;
         default:
             break;
         }
@@ -948,12 +952,7 @@ static void tcg_constant_folding(TCGContext *s)
         }
         if (affected == 0) {
             assert(nb_oargs == 1);
-            if (temps[args[1]].state != TCG_TEMP_CONST) {
-                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
-            } else {
-                tcg_opt_gen_movi(s, op, args,
-                                 args[0], temps[args[1]].val);
-            }
+            tcg_opt_gen_mov(s, op, args, args[0], args[1]);
             continue;
         }
 
@@ -978,12 +977,7 @@ static void tcg_constant_folding(TCGContext *s)
         CASE_OP_32_64(or):
         CASE_OP_32_64(and):
             if (temps_are_copies(args[1], args[2])) {
-                if (temps[args[1]].state != TCG_TEMP_CONST) {
-                    tcg_opt_gen_mov(s, op, args, args[0], args[1]);
-                } else {
-                    tcg_opt_gen_movi(s, op, args,
-                                     args[0], temps[args[1]].val);
-                }
+                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
                 continue;
             }
             break;
@@ -1010,14 +1004,8 @@ static void tcg_constant_folding(TCGContext *s)
            allocator where needed and possible.  Also detect copies. */
         switch (opc) {
         CASE_OP_32_64(mov):
-            if (temps[args[1]].state != TCG_TEMP_CONST) {
-                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
-                break;
-            }
-            /* Source argument is constant.  Rewrite the operation and
-               let movi case handle it. */
-            args[1] = temps[args[1]].val;
-            /* fallthrough */
+            tcg_opt_gen_mov(s, op, args, args[0], args[1]);
+            break;
         CASE_OP_32_64(movi):
             tcg_opt_gen_movi(s, op, args, args[0], args[1]);
             break;
@@ -1111,12 +1099,7 @@ static void tcg_constant_folding(TCGContext *s)
         CASE_OP_32_64(movcond):
             tmp = do_constant_folding_cond(opc, args[1], args[2], args[5]);
             if (tmp != 2) {
-                if (temps[args[4-tmp]].state == TCG_TEMP_CONST) {
-                    tcg_opt_gen_movi(s, op, args,
-                                     args[0], temps[args[4-tmp]].val);
-                } else {
-                    tcg_opt_gen_mov(s, op, args, args[0], args[4-tmp]);
-                }
+                tcg_opt_gen_mov(s, op, args, args[0], args[4-tmp]);
                 break;
             }
             goto do_default;
-- 
2.1.4

[Qemu-devel] [PATCH v1 5/5] tcg/optimize: rename tcg_constant_folding

[Aurelien Jarno]

The tcg_constant_folding folding ends up doing all the optimizations
(which is a good thing to avoid looping on all ops multiple time), so
make it clear and just rename it tcg_optimize.

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

diff --git a/tcg/optimize.c b/tcg/optimize.c
index a8dac85..026d70b 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -574,7 +574,7 @@ static bool swap_commutative2(TCGArg *p1, TCGArg *p2)
 }
 
 /* Propagate constants and copies, fold constant expressions. */
-static void tcg_constant_folding(TCGContext *s)
+void tcg_optimize(TCGContext *s)
 {
     int oi, oi_next, nb_temps, nb_globals;
 
@@ -1328,8 +1328,3 @@ static void tcg_constant_folding(TCGContext *s)
         }
     }
 }
-
-void tcg_optimize(TCGContext *s)
-{
-    tcg_constant_folding(s);
-}
-- 
2.1.4

Re: [Qemu-devel] [PATCH v1 4/5] tcg/optimize: fold constant test in tcg_opt_gen_mov

[Richard Henderson]

On 06/04/2015 12:53 PM, Aurelien Jarno wrote:
> +
>  static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, TCGArg *args,
>                              TCGArg dst, TCGArg src)
>  {
> +    if (temps[src].state == TCG_TEMP_CONST) {
> +        tcg_opt_gen_movi(s, op, args, dst, temps[src].val);
> +        return;
> +    }
> +
>      if (temps_are_copies(dst, src)) {
>          tcg_op_remove(s, op);
>          return;

I was going to comment re patch 3/5, but here's better, since it actually
tidies up all the places I would have pointed out.

I think you should check for copies first, and only then do the TEMP_CONST
check.  That way we get to remove an op.


r~

Re: [Qemu-devel] [PATCH v1 0/5] tcg/optimize: small cleanups

[Richard Henderson]

On 06/04/2015 12:53 PM, Aurelien Jarno wrote:
> This patch series does some small cleanups to make the optimizer code
> more readable. It doesn't bring any functional change. 
> 
> Aurelien Jarno (5):
>   tcg/optimize: remove opc argument from tcg_opt_gen_movi
>   tcg/optimize: remove opc argument from tcg_opt_gen_mov
>   tcg/optimize: fold temp copies test in tcg_opt_gen_mov
>   tcg/optimize: fold constant test in tcg_opt_gen_mov
>   tcg/optimize: rename tcg_constant_folding
> 
>  tcg/optimize.c | 149 +++++++++++++++++++++++----------------------------------
>  1 file changed, 59 insertions(+), 90 deletions(-)

1, 2, 3 & 5 get

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

4 can have it as well with the trivial change mentioned.


r~

Re: [Qemu-devel] [PATCH v1 4/5] tcg/optimize: fold constant test in tcg_opt_gen_mov

[Aurelien Jarno]

On 2015-06-04 14:07, Richard Henderson wrote:
> On 06/04/2015 12:53 PM, Aurelien Jarno wrote:
> > +
> >  static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, TCGArg *args,
> >                              TCGArg dst, TCGArg src)
> >  {
> > +    if (temps[src].state == TCG_TEMP_CONST) {
> > +        tcg_opt_gen_movi(s, op, args, dst, temps[src].val);
> > +        return;
> > +    }
> > +
> >      if (temps_are_copies(dst, src)) {
> >          tcg_op_remove(s, op);
> >          return;
> 
> I was going to comment re patch 3/5, but here's better, since it actually
> tidies up all the places I would have pointed out.
> 
> I think you should check for copies first, and only then do the TEMP_CONST
> check.  That way we get to remove an op.

If the state of a temp is TCG_TEMP_CONST, it can't be a copy as its
state should be TCG_TEMP_COPY for that. But it's true that the original
code is checking first for copies and then for constants.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH v1 4/5] tcg/optimize: fold constant test in tcg_opt_gen_mov

[Richard Henderson]

On 06/04/2015 02:44 PM, Aurelien Jarno wrote:
> On 2015-06-04 14:07, Richard Henderson wrote:
>> I think you should check for copies first, and only then do the TEMP_CONST
>> check.  That way we get to remove an op.
>
> If the state of a temp is TCG_TEMP_CONST, it can't be a copy as its
> state should be TCG_TEMP_COPY for that.

Ah, true.  I'd forgotten how that copy ring was represented.
The existing patch is fine, if you prefer.


r~

[Qemu-devel] [PATCH v2] tcg/optimize: fold constant test in tcg_opt_gen_mov

[Aurelien Jarno]

Most of the calls to tcg_opt_gen_mov are preceeded by a test to check if
the source temp is a constant. Fold that into the tcg_opt_gen_mov
function.

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

A version with the check for constants and copies swapped in case you
prefer to merge this one.

diff --git a/tcg/optimize.c b/tcg/optimize.c
index d5c0398..859a6c1 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -193,6 +193,28 @@ static bool temps_are_copies(TCGArg arg1, TCGArg arg2)
     return false;
 }
 
+static void tcg_opt_gen_movi(TCGContext *s, TCGOp *op, TCGArg *args,
+                             TCGArg dst, TCGArg val)
+{
+    TCGOpcode new_op = op_to_movi(op->opc);
+    tcg_target_ulong mask;
+
+    op->opc = new_op;
+
+    reset_temp(dst);
+    temps[dst].state = TCG_TEMP_CONST;
+    temps[dst].val = val;
+    mask = val;
+    if (TCG_TARGET_REG_BITS > 32 && new_op == INDEX_op_mov_i32) {
+        /* High bits of the destination are now garbage.  */
+        mask |= ~0xffffffffull;
+    }
+    temps[dst].mask = mask;
+
+    args[0] = dst;
+    args[1] = val;
+}
+
 static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, TCGArg *args,
                             TCGArg dst, TCGArg src)
 {
@@ -201,6 +223,11 @@ static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, TCGArg *args,
         return;
     }
 
+    if (temps[src].state == TCG_TEMP_CONST) {
+        tcg_opt_gen_movi(s, op, args, dst, temps[src].val);
+        return;
+    }
+
     TCGOpcode new_op = op_to_mov(op->opc);
     tcg_target_ulong mask;
 
@@ -233,28 +260,6 @@ static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, TCGArg *args,
     args[1] = src;
 }
 
-static void tcg_opt_gen_movi(TCGContext *s, TCGOp *op, TCGArg *args,
-                             TCGArg dst, TCGArg val)
-{
-    TCGOpcode new_op = op_to_movi(op->opc);
-    tcg_target_ulong mask;
-
-    op->opc = new_op;
-
-    reset_temp(dst);
-    temps[dst].state = TCG_TEMP_CONST;
-    temps[dst].val = val;
-    mask = val;
-    if (TCG_TARGET_REG_BITS > 32 && new_op == INDEX_op_mov_i32) {
-        /* High bits of the destination are now garbage.  */
-        mask |= ~0xffffffffull;
-    }
-    temps[dst].mask = mask;
-
-    args[0] = dst;
-    args[1] = val;
-}
-
 static TCGArg do_constant_folding_2(TCGOpcode op, TCGArg x, TCGArg y)
 {
     uint64_t l64, h64;
@@ -780,7 +785,8 @@ static void tcg_constant_folding(TCGContext *s)
             if (temps[args[1]].state != TCG_TEMP_CONST
                 && temps[args[2]].state == TCG_TEMP_CONST
                 && temps[args[2]].val == 0) {
-                goto do_mov3;
+                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
+                continue;
             }
             break;
         CASE_OP_32_64(and):
@@ -789,12 +795,10 @@ static void tcg_constant_folding(TCGContext *s)
             if (temps[args[1]].state != TCG_TEMP_CONST
                 && temps[args[2]].state == TCG_TEMP_CONST
                 && temps[args[2]].val == -1) {
-                goto do_mov3;
+                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
+                continue;
             }
             break;
-        do_mov3:
-            tcg_opt_gen_mov(s, op, args, args[0], args[1]);
-            continue;
         default:
             break;
         }
@@ -948,12 +952,7 @@ static void tcg_constant_folding(TCGContext *s)
         }
         if (affected == 0) {
             assert(nb_oargs == 1);
-            if (temps[args[1]].state != TCG_TEMP_CONST) {
-                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
-            } else {
-                tcg_opt_gen_movi(s, op, args,
-                                 args[0], temps[args[1]].val);
-            }
+            tcg_opt_gen_mov(s, op, args, args[0], args[1]);
             continue;
         }
 
@@ -978,12 +977,7 @@ static void tcg_constant_folding(TCGContext *s)
         CASE_OP_32_64(or):
         CASE_OP_32_64(and):
             if (temps_are_copies(args[1], args[2])) {
-                if (temps[args[1]].state != TCG_TEMP_CONST) {
-                    tcg_opt_gen_mov(s, op, args, args[0], args[1]);
-                } else {
-                    tcg_opt_gen_movi(s, op, args,
-                                     args[0], temps[args[1]].val);
-                }
+                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
                 continue;
             }
             break;
@@ -1010,14 +1004,8 @@ static void tcg_constant_folding(TCGContext *s)
            allocator where needed and possible.  Also detect copies. */
         switch (opc) {
         CASE_OP_32_64(mov):
-            if (temps[args[1]].state != TCG_TEMP_CONST) {
-                tcg_opt_gen_mov(s, op, args, args[0], args[1]);
-                break;
-            }
-            /* Source argument is constant.  Rewrite the operation and
-               let movi case handle it. */
-            args[1] = temps[args[1]].val;
-            /* fallthrough */
+            tcg_opt_gen_mov(s, op, args, args[0], args[1]);
+            break;
         CASE_OP_32_64(movi):
             tcg_opt_gen_movi(s, op, args, args[0], args[1]);
             break;
@@ -1111,12 +1099,7 @@ static void tcg_constant_folding(TCGContext *s)
         CASE_OP_32_64(movcond):
             tmp = do_constant_folding_cond(opc, args[1], args[2], args[5]);
             if (tmp != 2) {
-                if (temps[args[4-tmp]].state == TCG_TEMP_CONST) {
-                    tcg_opt_gen_movi(s, op, args,
-                                     args[0], temps[args[4-tmp]].val);
-                } else {
-                    tcg_opt_gen_mov(s, op, args, args[0], args[4-tmp]);
-                }
+                tcg_opt_gen_mov(s, op, args, args[0], args[4-tmp]);
                 break;
             }
             goto do_default;
-- 
2.1.4