Re: [Qemu-devel] [PATCH 2.3 7/8] tcg: Implement insert_op_before

[Bastian Koppelmann <kbastian@mail.uni-paderborn.de>]

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