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(-)

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>
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.
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;