diff mbox

[2.3,7/8] tcg: Implement insert_op_before

Message ID 1415723092-4088-8-git-send-email-rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson Nov. 11, 2014, 4:24 p.m. UTC 
Rather reserving space in the op stream for optimization,
let the optimizer add ops as necessary.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/optimize.c | 57 +++++++++++++++++++++++++++++++++++----------------------
 tcg/tcg-op.c   | 16 ----------------
 2 files changed, 35 insertions(+), 38 deletions(-)

Comments

Richard Henderson Nov. 14, 2014, 2:46 p.m. UTC | #1 
On 11/14/2014 04:25 PM, Bastian Koppelmann wrote:
> 
> On 11/11/2014 04:24 PM, Richard Henderson wrote:
>> Rather reserving space in the op stream for optimization,
>> let the optimizer add ops as necessary.
>>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>> ---
>>   tcg/optimize.c | 57 +++++++++++++++++++++++++++++++++++----------------------
>>   tcg/tcg-op.c   | 16 ----------------
>>   2 files changed, 35 insertions(+), 38 deletions(-)
>>
>> diff --git a/tcg/optimize.c b/tcg/optimize.c
>> index 973fbb4..067917c 100644
>> --- a/tcg/optimize.c
>> +++ b/tcg/optimize.c
>> @@ -67,6 +67,37 @@ static void reset_temp(TCGArg temp)
>>       temps[temp].mask = -1;
>>   }
>>   +static TCGOp *insert_op_before(TCGContext *s, TCGOp *old_op,
>> +                                TCGOpcode opc, int nargs)
>> +{
>> +    int oi = s->gen_next_op_idx;
>> +    int pi = s->gen_next_parm_idx;
>> +    int prev = old_op->prev;
>> +    int next = old_op - s->gen_op_buf;
>> +    TCGOp *new_op;
>> +
>> +    tcg_debug_assert(oi < OPC_BUF_SIZE);
>> +    tcg_debug_assert(pi + nargs <= OPPARAM_BUF_SIZE);
> I thinks it is better to assure these assertion always hold, e.g.
> 
>     if (oi < OPC_BUF_SIZE || pi + nargs <= OPPARAM_BUF_SIZE) {
>         return NULL;
>     }
>     ...
>     TCGOp *op2 = insert_op_before(s, op, INDEX_op_movi_i32, 2);
>     if (op2) {
>         *args2 = &s->gen_opparam_buf[op2->args];
>     }
> 
> Or how do we know they always hold?

For the same reason we don't bother checking during initial generation of the
opcodes.  We simply assume there's enough space.  Not a good answer but...

> All references on tcg_gen_op0 are gone, so lets remove it.

Fair enough.


r~
Bastian Koppelmann Nov. 14, 2014, 3:25 p.m. UTC | #2 
On 11/11/2014 04:24 PM, Richard Henderson wrote:
> Rather reserving space in the op stream for optimization,
> let the optimizer add ops as necessary.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>   tcg/optimize.c | 57 +++++++++++++++++++++++++++++++++++----------------------
>   tcg/tcg-op.c   | 16 ----------------
>   2 files changed, 35 insertions(+), 38 deletions(-)
>
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index 973fbb4..067917c 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -67,6 +67,37 @@ static void reset_temp(TCGArg temp)
>       temps[temp].mask = -1;
>   }
>   
> +static TCGOp *insert_op_before(TCGContext *s, TCGOp *old_op,
> +                                TCGOpcode opc, int nargs)
> +{
> +    int oi = s->gen_next_op_idx;
> +    int pi = s->gen_next_parm_idx;
> +    int prev = old_op->prev;
> +    int next = old_op - s->gen_op_buf;
> +    TCGOp *new_op;
> +
> +    tcg_debug_assert(oi < OPC_BUF_SIZE);
> +    tcg_debug_assert(pi + nargs <= OPPARAM_BUF_SIZE);
I thinks it is better to assure these assertion always hold, e.g.

     if (oi < OPC_BUF_SIZE || pi + nargs <= OPPARAM_BUF_SIZE) {
         return NULL;
     }
     ...
     TCGOp *op2 = insert_op_before(s, op, INDEX_op_movi_i32, 2);
     if (op2) {
         *args2 = &s->gen_opparam_buf[op2->args];
     }

Or how do we know they always hold?

> diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
> index fbd82bd..8de259a 100644
> --- a/tcg/tcg-op.c
> +++ b/tcg/tcg-op.c
> @@ -571,8 +571,6 @@ void tcg_gen_add2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al,
>   {
>       if (TCG_TARGET_HAS_add2_i32) {
>           tcg_gen_op6_i32(INDEX_op_add2_i32, rl, rh, al, ah, bl, bh);
> -        /* Allow the optimizer room to replace add2 with two moves.  */
> -        tcg_gen_op0(&tcg_ctx, INDEX_op_nop);
>
All references on tcg_gen_op0 are gone, so lets remove it.

Cheers,
Bastian
diff mbox

Patch

diff --git a/tcg/optimize.c b/tcg/optimize.c
index 973fbb4..067917c 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -67,6 +67,37 @@  static void reset_temp(TCGArg temp)
     temps[temp].mask = -1;
 }
 
+static TCGOp *insert_op_before(TCGContext *s, TCGOp *old_op,
+                                TCGOpcode opc, int nargs)
+{
+    int oi = s->gen_next_op_idx;
+    int pi = s->gen_next_parm_idx;
+    int prev = old_op->prev;
+    int next = old_op - s->gen_op_buf;
+    TCGOp *new_op;
+
+    tcg_debug_assert(oi < OPC_BUF_SIZE);
+    tcg_debug_assert(pi + nargs <= OPPARAM_BUF_SIZE);
+    s->gen_next_op_idx = oi + 1;
+    s->gen_next_parm_idx = pi + nargs;
+
+    new_op = &s->gen_op_buf[oi];
+    *new_op = (TCGOp){
+        .opc = opc,
+        .args = pi,
+        .prev = prev,
+        .next = next
+    };
+    if (prev >= 0) {
+        s->gen_op_buf[prev].next = oi;
+    } else {
+        s->gen_first_op_idx = oi;
+    }
+    old_op->prev = oi;
+
+    return new_op;
+}
+
 /* Reset all temporaries, given that there are NB_TEMPS of them.  */
 static void reset_all_temps(int nb_temps)
 {
@@ -1108,8 +1139,8 @@  static void tcg_constant_folding(TCGContext *s)
                 uint64_t a = ((uint64_t)ah << 32) | al;
                 uint64_t b = ((uint64_t)bh << 32) | bl;
                 TCGArg rl, rh;
-                TCGOp *op2;
-                TCGArg *args2;
+                TCGOp *op2 = insert_op_before(s, op, INDEX_op_movi_i32, 2);
+                TCGArg *args2 = &s->gen_opparam_buf[op2->args];
 
                 if (opc == INDEX_op_add2_i32) {
                     a += b;
@@ -1117,15 +1148,6 @@  static void tcg_constant_folding(TCGContext *s)
                     a -= b;
                 }
 
-                /* We emit the extra nop when we emit the add2/sub2.  */
-                op2 = &s->gen_op_buf[oi_next];
-                assert(op2->opc == INDEX_op_nop);
-
-                /* But we still have to allocate args for the op.  */
-                op2->args = s->gen_next_parm_idx;
-                s->gen_next_parm_idx += 2;
-                args2 = &s->gen_opparam_buf[op2->args];
-
                 rl = args[0];
                 rh = args[1];
                 tcg_opt_gen_movi(s, op, args, opc, rl, (uint32_t)a);
@@ -1144,17 +1166,8 @@  static void tcg_constant_folding(TCGContext *s)
                 uint32_t b = temps[args[3]].val;
                 uint64_t r = (uint64_t)a * b;
                 TCGArg rl, rh;
-                TCGOp *op2;
-                TCGArg *args2;
-
-                /* We emit the extra nop when we emit the mulu2.  */
-                op2 = &s->gen_op_buf[oi_next];
-                assert(op2->opc == INDEX_op_nop);
-
-                /* But we still have to allocate args for the op.  */
-                op2->args = s->gen_next_parm_idx;
-                s->gen_next_parm_idx += 2;
-                args2 = &s->gen_opparam_buf[op2->args];
+                TCGOp *op2 = insert_op_before(s, op, INDEX_op_movi_i32, 2);
+                TCGArg *args2 = &s->gen_opparam_buf[op2->args];
 
                 rl = args[0];
                 rh = args[1];
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index fbd82bd..8de259a 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -571,8 +571,6 @@  void tcg_gen_add2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al,
 {
     if (TCG_TARGET_HAS_add2_i32) {
         tcg_gen_op6_i32(INDEX_op_add2_i32, rl, rh, al, ah, bl, bh);
-        /* Allow the optimizer room to replace add2 with two moves.  */
-        tcg_gen_op0(&tcg_ctx, INDEX_op_nop);
     } else {
         TCGv_i64 t0 = tcg_temp_new_i64();
         TCGv_i64 t1 = tcg_temp_new_i64();
@@ -590,8 +588,6 @@  void tcg_gen_sub2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 al,
 {
     if (TCG_TARGET_HAS_sub2_i32) {
         tcg_gen_op6_i32(INDEX_op_sub2_i32, rl, rh, al, ah, bl, bh);
-        /* Allow the optimizer room to replace sub2 with two moves.  */
-        tcg_gen_op0(&tcg_ctx, INDEX_op_nop);
     } else {
         TCGv_i64 t0 = tcg_temp_new_i64();
         TCGv_i64 t1 = tcg_temp_new_i64();
@@ -608,8 +604,6 @@  void tcg_gen_mulu2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 arg1, TCGv_i32 arg2)
 {
     if (TCG_TARGET_HAS_mulu2_i32) {
         tcg_gen_op4_i32(INDEX_op_mulu2_i32, rl, rh, arg1, arg2);
-        /* Allow the optimizer room to replace mulu2 with two moves.  */
-        tcg_gen_op0(&tcg_ctx, INDEX_op_nop);
     } else if (TCG_TARGET_HAS_muluh_i32) {
         TCGv_i32 t = tcg_temp_new_i32();
         tcg_gen_op3_i32(INDEX_op_mul_i32, t, arg1, arg2);
@@ -632,8 +626,6 @@  void tcg_gen_muls2_i32(TCGv_i32 rl, TCGv_i32 rh, TCGv_i32 arg1, TCGv_i32 arg2)
 {
     if (TCG_TARGET_HAS_muls2_i32) {
         tcg_gen_op4_i32(INDEX_op_muls2_i32, rl, rh, arg1, arg2);
-        /* Allow the optimizer room to replace muls2 with two moves.  */
-        tcg_gen_op0(&tcg_ctx, INDEX_op_nop);
     } else if (TCG_TARGET_HAS_mulsh_i32) {
         TCGv_i32 t = tcg_temp_new_i32();
         tcg_gen_op3_i32(INDEX_op_mul_i32, t, arg1, arg2);
@@ -1648,8 +1640,6 @@  void tcg_gen_add2_i64(TCGv_i64 rl, TCGv_i64 rh, TCGv_i64 al,
 {
     if (TCG_TARGET_HAS_add2_i64) {
         tcg_gen_op6_i64(INDEX_op_add2_i64, rl, rh, al, ah, bl, bh);
-        /* Allow the optimizer room to replace add2 with two moves.  */
-        tcg_gen_op0(&tcg_ctx, INDEX_op_nop);
     } else {
         TCGv_i64 t0 = tcg_temp_new_i64();
         TCGv_i64 t1 = tcg_temp_new_i64();
@@ -1668,8 +1658,6 @@  void tcg_gen_sub2_i64(TCGv_i64 rl, TCGv_i64 rh, TCGv_i64 al,
 {
     if (TCG_TARGET_HAS_sub2_i64) {
         tcg_gen_op6_i64(INDEX_op_sub2_i64, rl, rh, al, ah, bl, bh);
-        /* Allow the optimizer room to replace sub2 with two moves.  */
-        tcg_gen_op0(&tcg_ctx, INDEX_op_nop);
     } else {
         TCGv_i64 t0 = tcg_temp_new_i64();
         TCGv_i64 t1 = tcg_temp_new_i64();
@@ -1687,8 +1675,6 @@  void tcg_gen_mulu2_i64(TCGv_i64 rl, TCGv_i64 rh, TCGv_i64 arg1, TCGv_i64 arg2)
 {
     if (TCG_TARGET_HAS_mulu2_i64) {
         tcg_gen_op4_i64(INDEX_op_mulu2_i64, rl, rh, arg1, arg2);
-        /* Allow the optimizer room to replace mulu2 with two moves.  */
-        tcg_gen_op0(&tcg_ctx, INDEX_op_nop);
     } else if (TCG_TARGET_HAS_muluh_i64) {
         TCGv_i64 t = tcg_temp_new_i64();
         tcg_gen_op3_i64(INDEX_op_mul_i64, t, arg1, arg2);
@@ -1708,8 +1694,6 @@  void tcg_gen_muls2_i64(TCGv_i64 rl, TCGv_i64 rh, TCGv_i64 arg1, TCGv_i64 arg2)
 {
     if (TCG_TARGET_HAS_muls2_i64) {
         tcg_gen_op4_i64(INDEX_op_muls2_i64, rl, rh, arg1, arg2);
-        /* Allow the optimizer room to replace muls2 with two moves.  */
-        tcg_gen_op0(&tcg_ctx, INDEX_op_nop);
     } else if (TCG_TARGET_HAS_mulsh_i64) {
         TCGv_i64 t = tcg_temp_new_i64();
         tcg_gen_op3_i64(INDEX_op_mul_i64, t, arg1, arg2);