Re: [PATCH 02/10] target/arm: Add missing TCG temp free in do_2shift_env_64()

[Peter Maydell]

On Thu, 11 Jun 2020 at 16:43, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 6/11/20 7:45 AM, Peter Maydell wrote:
> > In commit 37bfce81b10450071 we accidentally introduced a leak of a TCG
> > temporary in do_2shift_env_64(); free it.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > My test setup wasn't looking for temporary-leak warnings (they are
> > not as easy to get at as they used to be because they only appear
> > if you enable qemu_log tracing for some other purpose). This is the
> > only one that snuck through, though.
> > ---
> >  target/arm/translate-neon.inc.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/target/arm/translate-neon.inc.c 
> > b/target/arm/translate-neon.inc.c
> > index 7c4888a80c9..f2c241a87e9 100644
> > --- a/target/arm/translate-neon.inc.c
> > +++ b/target/arm/translate-neon.inc.c
> > @@ -1329,6 +1329,7 @@ static bool do_2shift_env_64(DisasContext *s, 
> > arg_2reg_shift *a,
> >          neon_load_reg64(tmp, a->vm + pass);
> >          fn(tmp, cpu_env, tmp, constimm);
> >          neon_store_reg64(tmp, a->vd + pass);
> > +        tcg_temp_free_i64(tmp);
>
> Huh.  I thought all the a32 stores magically freed their inputs.  I guess
> that's just the general-purpose registers.

Confusingly, neon_load_reg()/neon_store_reg() do the "create
new TCG temp for the load, free the store input" thing, but
neon_load_reg64()/neon_store_reg64()/neon_load_reg32()/neon_store_reg32()
do not. I'd kind of like to get rid of this weird proliferation
of load and store functions, but neon_load_reg()/neon_store_reg() are
also the only ones which take a (regno, pass) pair of inputs,
so it's not a completely trivial substitution.

thanks
-- PMM