diff mbox series

[v3,41/48] tcg/optimize: Sink commutative operand swapping into fold functions

Message ID 20211021210539.825582-42-richard.henderson@linaro.org
State New
Headers show
Series tcg: optimize redundant sign extensions | expand

Commit Message

Richard Henderson Oct. 21, 2021, 9:05 p.m. UTC 
Most of these are handled by creating a fold_const2_commutative
to handle all of the binary operators.  The rest were already
handled on a case-by-case basis in the switch, and have their
own fold function in which to place the call.

We now have only one major switch on TCGOpcode.

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

---
 tcg/optimize.c | 128 ++++++++++++++++++++++---------------------------
 1 file changed, 56 insertions(+), 72 deletions(-)

-- 
2.25.1

Comments

Alex Bennée Oct. 26, 2021, 4:27 p.m. UTC | #1 
Richard Henderson <richard.henderson@linaro.org> writes:

> Most of these are handled by creating a fold_const2_commutative

> to handle all of the binary operators.  The rest were already

> handled on a case-by-case basis in the switch, and have their

> own fold function in which to place the call.

>

> We now have only one major switch on TCGOpcode.

>

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

> ---

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

>  1 file changed, 56 insertions(+), 72 deletions(-)

>

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

> index ba068e7d3e..92b35a8c3f 100644

> --- a/tcg/optimize.c

> +++ b/tcg/optimize.c

> @@ -696,6 +696,12 @@ static bool fold_const2(OptContext *ctx, TCGOp *op)

>      return false;

>  }

>  

> +static bool fold_const2_commutative(OptContext *ctx, TCGOp *op)

> +{

> +    swap_commutative(op->args[0], &op->args[1], &op->args[2]);

> +    return fold_const2(ctx, op);

> +}

> +

>  static bool fold_masks(OptContext *ctx, TCGOp *op)

>  {

>      uint64_t a_mask = ctx->a_mask;

> @@ -832,7 +838,7 @@ static bool fold_xx_to_x(OptContext *ctx, TCGOp *op)

>  

>  static bool fold_add(OptContext *ctx, TCGOp *op)

>  {

> -    if (fold_const2(ctx, op) ||

> +    if (fold_const2_commutative(ctx, op) ||

>          fold_xi_to_x(ctx, op, 0)) {

>          return true;

>      }

> @@ -891,6 +897,9 @@ static bool fold_addsub2(OptContext *ctx, TCGOp *op, bool add)

>  

>  static bool fold_add2(OptContext *ctx, TCGOp *op)

>  {

> +    swap_commutative(op->args[0], &op->args[2], &op->args[4]);

> +    swap_commutative(op->args[1], &op->args[3], &op->args[5]);

> +

>      return fold_addsub2(ctx, op, true);

>  }

>  

> @@ -898,7 +907,7 @@ static bool fold_and(OptContext *ctx, TCGOp *op)

>  {

>      uint64_t z1, z2;

>  

> -    if (fold_const2(ctx, op) ||

> +    if (fold_const2_commutative(ctx, op) ||

>          fold_xi_to_i(ctx, op, 0) ||

>          fold_xi_to_x(ctx, op, -1) ||

>          fold_xx_to_x(ctx, op)) {

> @@ -950,8 +959,13 @@ static bool fold_andc(OptContext *ctx, TCGOp *op)

>  static bool fold_brcond(OptContext *ctx, TCGOp *op)

>  {

>      TCGCond cond = op->args[2];

> -    int i = do_constant_folding_cond(ctx->type, op->args[0], op->args[1], cond);

> +    int i;

>  

> +    if (swap_commutative(-1, &op->args[0], &op->args[1])) {


I find this a bit magical. I couldn't find anything about TCGArg except
it's type:

  typedef tcg_target_ulong TCGArg;

so I'm not sure what to make of -1 in this case. I guess it just means
we never have a (sum == 0 && dest == a2) leg but it's not obvious
reading the fold code.
>  

> +    if (swap_commutative(-1, &op->args[1], &op->args[2])) {

> +        op->args[5] = cond = tcg_swap_cond(cond);

> +    }


Also here.

<snip>

-- 
Alex Bennée
Richard Henderson Oct. 26, 2021, 7:33 p.m. UTC | #2 
On 10/26/21 9:27 AM, Alex Bennée wrote:
> I find this a bit magical. I couldn't find anything about TCGArg except

> it's type:

> 

>    typedef tcg_target_ulong TCGArg;


For an argument that contains a temp,

static inline TCGArg temp_arg(TCGTemp *ts)
{
     return (uintptr_t)ts;
}

static inline TCGTemp *arg_temp(TCGArg a)
{
     return (TCGTemp *)(uintptr_t)a;
}

i.e. the TCGArg is in fact a pointer.

> so I'm not sure what to make of -1 in this case. I guess it just means

> we never have a (sum == 0 && dest == a2) leg but it's not obvious

> reading the fold code.


Indeed.  The use of -1 goes back quite a ways.  How about

#define NO_DEST  temp_arg(NULL)

which will also fail to match in that expression?


r~
Alex Bennée Oct. 27, 2021, 1:22 p.m. UTC | #3 
Richard Henderson <richard.henderson@linaro.org> writes:

> On 10/26/21 9:27 AM, Alex Bennée wrote:

>> I find this a bit magical. I couldn't find anything about TCGArg except

>> it's type:

>>    typedef tcg_target_ulong TCGArg;

>

> For an argument that contains a temp,

>

> static inline TCGArg temp_arg(TCGTemp *ts)

> {

>     return (uintptr_t)ts;

> }

>

> static inline TCGTemp *arg_temp(TCGArg a)

> {

>     return (TCGTemp *)(uintptr_t)a;

> }

>

> i.e. the TCGArg is in fact a pointer.

>

>> so I'm not sure what to make of -1 in this case. I guess it just means

>> we never have a (sum == 0 && dest == a2) leg but it's not obvious

>> reading the fold code.

>

> Indeed.  The use of -1 goes back quite a ways.  How about

>

> #define NO_DEST  temp_arg(NULL)

>

> which will also fail to match in that expression?


Sounds good to me. It would be nice to document the details of what
TCGArg can be in another patch.

-- 
Alex Bennée
diff mbox series

Patch

diff --git a/tcg/optimize.c b/tcg/optimize.c
index ba068e7d3e..92b35a8c3f 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -696,6 +696,12 @@  static bool fold_const2(OptContext *ctx, TCGOp *op)
     return false;
 }
 
+static bool fold_const2_commutative(OptContext *ctx, TCGOp *op)
+{
+    swap_commutative(op->args[0], &op->args[1], &op->args[2]);
+    return fold_const2(ctx, op);
+}
+
 static bool fold_masks(OptContext *ctx, TCGOp *op)
 {
     uint64_t a_mask = ctx->a_mask;
@@ -832,7 +838,7 @@  static bool fold_xx_to_x(OptContext *ctx, TCGOp *op)
 
 static bool fold_add(OptContext *ctx, TCGOp *op)
 {
-    if (fold_const2(ctx, op) ||
+    if (fold_const2_commutative(ctx, op) ||
         fold_xi_to_x(ctx, op, 0)) {
         return true;
     }
@@ -891,6 +897,9 @@  static bool fold_addsub2(OptContext *ctx, TCGOp *op, bool add)
 
 static bool fold_add2(OptContext *ctx, TCGOp *op)
 {
+    swap_commutative(op->args[0], &op->args[2], &op->args[4]);
+    swap_commutative(op->args[1], &op->args[3], &op->args[5]);
+
     return fold_addsub2(ctx, op, true);
 }
 
@@ -898,7 +907,7 @@  static bool fold_and(OptContext *ctx, TCGOp *op)
 {
     uint64_t z1, z2;
 
-    if (fold_const2(ctx, op) ||
+    if (fold_const2_commutative(ctx, op) ||
         fold_xi_to_i(ctx, op, 0) ||
         fold_xi_to_x(ctx, op, -1) ||
         fold_xx_to_x(ctx, op)) {
@@ -950,8 +959,13 @@  static bool fold_andc(OptContext *ctx, TCGOp *op)
 static bool fold_brcond(OptContext *ctx, TCGOp *op)
 {
     TCGCond cond = op->args[2];
-    int i = do_constant_folding_cond(ctx->type, op->args[0], op->args[1], cond);
+    int i;
 
+    if (swap_commutative(-1, &op->args[0], &op->args[1])) {
+        op->args[2] = cond = tcg_swap_cond(cond);
+    }
+
+    i = do_constant_folding_cond(ctx->type, op->args[0], op->args[1], cond);
     if (i == 0) {
         tcg_op_remove(ctx->tcg, op);
         return true;
@@ -966,10 +980,14 @@  static bool fold_brcond(OptContext *ctx, TCGOp *op)
 static bool fold_brcond2(OptContext *ctx, TCGOp *op)
 {
     TCGCond cond = op->args[4];
-    int i = do_constant_folding_cond2(&op->args[0], &op->args[2], cond);
     TCGArg label = op->args[5];
-    int inv = 0;
+    int i, inv = 0;
 
+    if (swap_commutative2(&op->args[0], &op->args[2])) {
+        op->args[4] = cond = tcg_swap_cond(cond);
+    }
+
+    i = do_constant_folding_cond2(&op->args[0], &op->args[2], cond);
     if (i >= 0) {
         goto do_brcond_const;
     }
@@ -1214,7 +1232,7 @@  static bool fold_dup2(OptContext *ctx, TCGOp *op)
 
 static bool fold_eqv(OptContext *ctx, TCGOp *op)
 {
-    if (fold_const2(ctx, op) ||
+    if (fold_const2_commutative(ctx, op) ||
         fold_xi_to_x(ctx, op, -1) ||
         fold_xi_to_not(ctx, op, 0)) {
         return true;
@@ -1376,8 +1394,20 @@  static bool fold_mov(OptContext *ctx, TCGOp *op)
 static bool fold_movcond(OptContext *ctx, TCGOp *op)
 {
     TCGCond cond = op->args[5];
-    int i = do_constant_folding_cond(ctx->type, op->args[1], op->args[2], cond);
+    int i;
 
+    if (swap_commutative(-1, &op->args[1], &op->args[2])) {
+        op->args[5] = cond = tcg_swap_cond(cond);
+    }
+    /*
+     * Canonicalize the "false" input reg to match the destination reg so
+     * that the tcg backend can implement a "move if true" operation.
+     */
+    if (swap_commutative(op->args[0], &op->args[4], &op->args[3])) {
+        op->args[5] = cond = tcg_invert_cond(cond);
+    }
+
+    i = do_constant_folding_cond(ctx->type, op->args[1], op->args[2], cond);
     if (i >= 0) {
         return tcg_opt_gen_mov(ctx, op, op->args[0], op->args[4 - i]);
     }
@@ -1414,7 +1444,7 @@  static bool fold_movcond(OptContext *ctx, TCGOp *op)
 
 static bool fold_multiply(OptContext *ctx, TCGOp *op)
 {
-    if (fold_const2(ctx, op) ||
+    if (fold_const2_commutative(ctx, op) ||
         fold_xi_to_i(ctx, op, 0)) {
         return true;
     }
@@ -1423,6 +1453,8 @@  static bool fold_multiply(OptContext *ctx, TCGOp *op)
 
 static bool fold_multiply2(OptContext *ctx, TCGOp *op)
 {
+    swap_commutative(op->args[0], &op->args[2], &op->args[3]);
+
     if (arg_is_const(op->args[2]) && arg_is_const(op->args[3])) {
         uint64_t a = arg_info(op->args[2])->val;
         uint64_t b = arg_info(op->args[3])->val;
@@ -1466,7 +1498,7 @@  static bool fold_multiply2(OptContext *ctx, TCGOp *op)
 
 static bool fold_nand(OptContext *ctx, TCGOp *op)
 {
-    if (fold_const2(ctx, op) ||
+    if (fold_const2_commutative(ctx, op) ||
         fold_xi_to_not(ctx, op, -1)) {
         return true;
     }
@@ -1495,7 +1527,7 @@  static bool fold_neg(OptContext *ctx, TCGOp *op)
 
 static bool fold_nor(OptContext *ctx, TCGOp *op)
 {
-    if (fold_const2(ctx, op) ||
+    if (fold_const2_commutative(ctx, op) ||
         fold_xi_to_not(ctx, op, 0)) {
         return true;
     }
@@ -1515,7 +1547,7 @@  static bool fold_not(OptContext *ctx, TCGOp *op)
 
 static bool fold_or(OptContext *ctx, TCGOp *op)
 {
-    if (fold_const2(ctx, op) ||
+    if (fold_const2_commutative(ctx, op) ||
         fold_xi_to_x(ctx, op, 0) ||
         fold_xx_to_x(ctx, op)) {
         return true;
@@ -1561,8 +1593,13 @@  static bool fold_qemu_st(OptContext *ctx, TCGOp *op)
 static bool fold_setcond(OptContext *ctx, TCGOp *op)
 {
     TCGCond cond = op->args[3];
-    int i = do_constant_folding_cond(ctx->type, op->args[1], op->args[2], cond);
+    int i;
 
+    if (swap_commutative(op->args[0], &op->args[1], &op->args[2])) {
+        op->args[3] = cond = tcg_swap_cond(cond);
+    }
+
+    i = do_constant_folding_cond(ctx->type, op->args[1], op->args[2], cond);
     if (i >= 0) {
         return tcg_opt_gen_movi(ctx, op, op->args[0], i);
     }
@@ -1574,9 +1611,13 @@  static bool fold_setcond(OptContext *ctx, TCGOp *op)
 static bool fold_setcond2(OptContext *ctx, TCGOp *op)
 {
     TCGCond cond = op->args[5];
-    int i = do_constant_folding_cond2(&op->args[1], &op->args[3], cond);
-    int inv = 0;
+    int i, inv = 0;
 
+    if (swap_commutative2(&op->args[1], &op->args[3])) {
+        op->args[5] = cond = tcg_swap_cond(cond);
+    }
+
+    i = do_constant_folding_cond2(&op->args[1], &op->args[3], cond);
     if (i >= 0) {
         goto do_setcond_const;
     }
@@ -1754,7 +1795,7 @@  static bool fold_tcg_ld(OptContext *ctx, TCGOp *op)
 
 static bool fold_xor(OptContext *ctx, TCGOp *op)
 {
-    if (fold_const2(ctx, op) ||
+    if (fold_const2_commutative(ctx, op) ||
         fold_xx_to_i(ctx, op, 0) ||
         fold_xi_to_x(ctx, op, 0) ||
         fold_xi_to_not(ctx, op, -1)) {
@@ -1807,63 +1848,6 @@  void tcg_optimize(TCGContext *s)
             ctx.type = TCG_TYPE_I32;
         }
 
-        /* For commutative operations make constant second argument */
-        switch (opc) {
-        CASE_OP_32_64_VEC(add):
-        CASE_OP_32_64_VEC(mul):
-        CASE_OP_32_64_VEC(and):
-        CASE_OP_32_64_VEC(or):
-        CASE_OP_32_64_VEC(xor):
-        CASE_OP_32_64(eqv):
-        CASE_OP_32_64(nand):
-        CASE_OP_32_64(nor):
-        CASE_OP_32_64(muluh):
-        CASE_OP_32_64(mulsh):
-            swap_commutative(op->args[0], &op->args[1], &op->args[2]);
-            break;
-        CASE_OP_32_64(brcond):
-            if (swap_commutative(-1, &op->args[0], &op->args[1])) {
-                op->args[2] = tcg_swap_cond(op->args[2]);
-            }
-            break;
-        CASE_OP_32_64(setcond):
-            if (swap_commutative(op->args[0], &op->args[1], &op->args[2])) {
-                op->args[3] = tcg_swap_cond(op->args[3]);
-            }
-            break;
-        CASE_OP_32_64(movcond):
-            if (swap_commutative(-1, &op->args[1], &op->args[2])) {
-                op->args[5] = tcg_swap_cond(op->args[5]);
-            }
-            /* For movcond, we canonicalize the "false" input reg to match
-               the destination reg so that the tcg backend can implement
-               a "move if true" operation.  */
-            if (swap_commutative(op->args[0], &op->args[4], &op->args[3])) {
-                op->args[5] = tcg_invert_cond(op->args[5]);
-            }
-            break;
-        CASE_OP_32_64(add2):
-            swap_commutative(op->args[0], &op->args[2], &op->args[4]);
-            swap_commutative(op->args[1], &op->args[3], &op->args[5]);
-            break;
-        CASE_OP_32_64(mulu2):
-        CASE_OP_32_64(muls2):
-            swap_commutative(op->args[0], &op->args[2], &op->args[3]);
-            break;
-        case INDEX_op_brcond2_i32:
-            if (swap_commutative2(&op->args[0], &op->args[2])) {
-                op->args[4] = tcg_swap_cond(op->args[4]);
-            }
-            break;
-        case INDEX_op_setcond2_i32:
-            if (swap_commutative2(&op->args[1], &op->args[3])) {
-                op->args[5] = tcg_swap_cond(op->args[5]);
-            }
-            break;
-        default:
-            break;
-        }
-
         /* Assume all bits affected, and no bits known zero. */
         ctx.a_mask = -1;
         ctx.z_mask = -1;