Re: [Qemu-devel] [PATCH v2 03/14] tcg: Tidy temporary allocation

[Aurelien Jarno <aurelien@aurel32.net>]

On 2015-12-17 12:00, Richard Henderson wrote:
> In particular, make sure the memory is memset before use.
> Continues the increased use of TCGTemp pointers instead of
> integer indices where appropriate.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/tcg.c | 123 ++++++++++++++++++++++++++++----------------------------------
>  1 file changed, 56 insertions(+), 67 deletions(-)
> 
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index b1864d3..0a6edfb 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -429,32 +429,45 @@ void tcg_func_start(TCGContext *s)
>      s->be = tcg_malloc(sizeof(TCGBackendData));
>  }
>  
> -static inline void tcg_temp_alloc(TCGContext *s, int n)
> +static inline int temp_idx(TCGContext *s, TCGTemp *ts)
>  {
> -    if (n > TCG_MAX_TEMPS)
> -        tcg_abort();
> +    ptrdiff_t n = ts - s->temps;
> +    tcg_debug_assert(n >= 0 && n < s->nb_temps);
> +    return n;
> +}
> +
> +static inline TCGTemp *tcg_temp_alloc(TCGContext *s)
> +{
> +    int n = s->nb_temps++;
> +    tcg_debug_assert(n < TCG_MAX_TEMPS);
> +    return memset(&s->temps[n], 0, sizeof(TCGTemp));
> +}
> +
> +static inline TCGTemp *tcg_global_alloc(TCGContext *s)
> +{
> +    tcg_debug_assert(s->nb_globals == s->nb_temps);
> +    s->nb_globals++;
> +    return tcg_temp_alloc(s);
>  }

This is transforming an abort() which can happen all the time in an
assert which can happen only when TCG debug is enabled. Is it really
something we want? Maybe we should add a tcg_assert() function.

Otherwise it looks fine.

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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