Re: [Qemu-devel] [PATCH for-2.5 04/10] tcg/optimize: allow constant to have copies

[Alex Bennée]

Aurelien Jarno <address@hidden> writes:

> Now that copies and constants are tracked separately, we can allow
> constant to have copies, deferring the choice to use a register or a
> constant to the register allocation pass. This prevent this kind of
> regular constant reloading:
>
> -OUT: [size=338]
> +OUT: [size=298]
>    mov    -0x4(%r14),%ebp
>    test   %ebp,%ebp
>    jne    0x7ffbe9cb0ed6
>    mov    $0x40002219f8,%rbp
>    mov    %rbp,(%r14)
> -  mov    $0x40002219f8,%rbp
>    mov    $0x4000221a20,%rbx
>    mov    %rbp,(%rbx)
>    mov    $0x4000000000,%rbp
>    mov    %rbp,(%r14)
> -  mov    $0x4000000000,%rbp
>    mov    $0x4000221d38,%rbx
>    mov    %rbp,(%rbx)
>    mov    $0x40002221a8,%rbp
>    mov    %rbp,(%r14)
> -  mov    $0x40002221a8,%rbp
>    mov    $0x4000221d40,%rbx
>    mov    %rbp,(%rbx)
>    mov    $0x4000019170,%rbp
>    mov    %rbp,(%r14)
> -  mov    $0x4000019170,%rbp
>    mov    $0x4000221d48,%rbx
>    mov    %rbp,(%rbx)
>    mov    $0x40000049ee,%rbp
>    mov    %rbp,0x80(%r14)
>    mov    %r14,%rdi
>    callq  0x7ffbe99924d0
>    mov    $0x4000001680,%rbp
>    mov    %rbp,0x30(%r14)
>    mov    0x10(%r14),%rbp
>    mov    $0x4000001680,%rbp
>    mov    %rbp,0x30(%r14)
>    mov    0x10(%r14),%rbp
>    shl    $0x20,%rbp
>    mov    (%r14),%rbx
>    mov    %ebx,%ebx
>    mov    %rbx,(%r14)
>    or     %rbx,%rbp
>    mov    %rbp,0x10(%r14)
>    mov    %rbp,0x90(%r14)
>    mov    0x60(%r14),%rbx
>    mov    %rbx,0x38(%r14)
>    mov    0x28(%r14),%rbx
>    mov    $0x4000220e60,%r12
>    mov    %rbx,(%r12)
>    mov    $0x40002219c8,%rbx
>    mov    %rbp,(%rbx)
>    mov    0x20(%r14),%rbp
>    sub    $0x8,%rbp
>    mov    $0x4000004a16,%rbx
>    mov    %rbx,0x0(%rbp)
>    mov    %rbp,0x20(%r14)
>    mov    $0x19,%ebp
>    mov    %ebp,0xa8(%r14)
>    mov    $0x4000015110,%rbp
>    mov    %rbp,0x80(%r14)
>    xor    %eax,%eax
>    jmpq   0x7ffbebcae426
>    lea    -0x5f6d72a(%rip),%rax        # 0x7ffbe3d437b3
>    jmpq   0x7ffbebcae426
>
> Cc: Richard Henderson <address@hidden>
> Signed-off-by: Aurelien Jarno <address@hidden>
> ---
>  tcg/optimize.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index f16eb1e..48103b2 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -237,11 +237,6 @@ static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, 
> TCGArg *args,
>          return;
>      }
>  
> -    if (temp_is_const(src)) {
> -        tcg_opt_gen_movi(s, op, args, dst, temps[src].val);
> -        return;
> -    }
> -

That looks suspicious, surely we only want to drop the move if we
already have the const somewhere else?

>      TCGOpcode new_op = op_to_mov(op->opc);
>      tcg_target_ulong mask;
>  
> @@ -255,17 +250,15 @@ static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op, 
> TCGArg *args,
>      }
>      temps[dst].mask = mask;
>  
> -    assert(!temp_is_const(src));
> -
>      if (s->temps[src].type == s->temps[dst].type) {
>          if (!temp_is_copy(src)) {
> -            temps[src].is_const = false;
>              temps[src].is_copy = true;
>              temps[src].next_copy = src;
>              temps[src].prev_copy = src;
>          }
> -        temps[dst].is_const = false;
> +        temps[dst].is_const = temps[src].is_const;
>          temps[dst].is_copy = true;
> +        temps[dst].val = temps[src].val;
>          temps[dst].next_copy = temps[src].next_copy;
>          temps[dst].prev_copy = src;
>          temps[temps[dst].next_copy].prev_copy = dst;

-- 
Alex Bennée