Re: [Qemu-devel] [PATCH v2 for-2.5 11/12] tcg/optimize: do not remember garbage high bits for 32-bit ops

[Aurelien Jarno <aurelien@aurel32.net>]

On 2015-07-27 12:56, Aurelien Jarno wrote:
> Now that we have real size changing ops, we don't need to mark high
> bits of the destination as garbage. The goal of the optimizer is to
> predict the value of the temps (and not of the registers) and do
> simplifications when possible. The problem there is therefore not the
> fact that those bits are not counted as garbage, but that a size
> changing op is replaced by a move.
> 
> This patch is basically a revert of 24666baf, including the changes that
> have been made since then.
> 
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  tcg/optimize.c | 41 +++++++++++------------------------------
>  1 file changed, 11 insertions(+), 30 deletions(-)
> 
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index 8f33755..166074e 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -937,17 +922,13 @@ void tcg_optimize(TCGContext *s)
>              break;
>          }
>  
> -        /* 32-bit ops generate 32-bit results.  For the result is zero test
> -           below, we can ignore high bits, but for further optimizations we
> -           need to record that the high bits contain garbage.  */
> -        partmask = mask;
> +        /* 32-bit ops generate 32-bit results.  */
>          if (!(def->flags & TCG_OPF_64BIT)) {
> -            mask |= ~(tcg_target_ulong)0xffffffffu;
> -            partmask &= 0xffffffffu;
> +            mask &= 0xffffffffu;
>              affected &= 0xffffffffu;
>          }
>  
> -        if (partmask == 0) {
> +        if (mask == 0) {
>              assert(nb_oargs == 1);
>              tcg_opt_gen_movi(s, op, args, args[0], 0);
>              continue;

Answering to myself, this actually doesn't work as the current code
wrongly assumes that all ops writing a 64-bit temp will have the
TCG_OPF_64BIT flag set. This is wrong for at least call.

I still haven't decided what is the best way to fix that, either by
special casing these ops, or by actually looking at the temp type. I
guess performances will decide. In early version of this patchset, I
tried to access the temp type at other places, and it has some
performances impact.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net