diff mbox

[v2,RESEND] target-arm: cleanup internal resource leaks

Message ID 1256213856-76026-1-git-send-email-juha.riihimaki@nokia.com
State New
Headers show

Commit Message

Juha.Riihimaki@nokia.com Oct. 22, 2009, 12:17 p.m. UTC
From: Juha Riihimäki <juha.riihimaki@nokia.com>

Revised patch for getting rid of tcg temporary variable leaks in
target-arm/translate.c. This version also includes the leak patch for
gen_set_cpsr macro, now converted as a static inline function, which I
sent earlier as a separate patch on top of this patch.

Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
---
 target-arm/translate.c |  116 ++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 89 insertions(+), 27 deletions(-)

Comments

Laurent Desnogues Oct. 22, 2009, 10:11 p.m. UTC | #1
On Thu, Oct 22, 2009 at 2:17 PM,  <juha.riihimaki@nokia.com> wrote:
> From: Juha Riihimäki <juha.riihimaki@nokia.com>
>
> Revised patch for getting rid of tcg temporary variable leaks in
> target-arm/translate.c. This version also includes the leak patch for
> gen_set_cpsr macro, now converted as a static inline function, which I
> sent earlier as a separate patch on top of this patch.
>
> Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>

Acked-by: Laurent Desnogues <laurent.desnogues@gmail.com>

> ---
>  target-arm/translate.c |  116 ++++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 89 insertions(+), 27 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index e56082b..813f661 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -184,7 +184,12 @@ static void store_reg(DisasContext *s, int reg, TCGv var)
>  #define gen_uxtb16(var) gen_helper_uxtb16(var, var)
>
>
> -#define gen_set_cpsr(var, mask) gen_helper_cpsr_write(var, tcg_const_i32(mask))
> +static inline void gen_set_cpsr(TCGv var, uint32_t mask)
> +{
> +    TCGv tmp_mask = tcg_const_i32(mask);
> +    gen_helper_cpsr_write(var, tmp_mask);
> +    tcg_temp_free_i32(tmp_mask);
> +}
>  /* Set NZCV flags from the high 4 bits of var.  */
>  #define gen_set_nzcv(var) gen_set_cpsr(var, CPSR_NZCV)
>
> @@ -287,6 +292,7 @@ static TCGv_i64 gen_mulu_i64_i32(TCGv a, TCGv b)
>     tcg_gen_extu_i32_i64(tmp2, b);
>     dead_tmp(b);
>     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
> +    tcg_temp_free_i64(tmp2);
>     return tmp1;
>  }
>
> @@ -300,6 +306,7 @@ static TCGv_i64 gen_muls_i64_i32(TCGv a, TCGv b)
>     tcg_gen_ext_i32_i64(tmp2, b);
>     dead_tmp(b);
>     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
> +    tcg_temp_free_i64(tmp2);
>     return tmp1;
>  }
>
> @@ -312,9 +319,11 @@ static void gen_mull(TCGv a, TCGv b)
>     tcg_gen_extu_i32_i64(tmp1, a);
>     tcg_gen_extu_i32_i64(tmp2, b);
>     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
> +    tcg_temp_free_i64(tmp2);
>     tcg_gen_trunc_i64_i32(a, tmp1);
>     tcg_gen_shri_i64(tmp1, tmp1, 32);
>     tcg_gen_trunc_i64_i32(b, tmp1);
> +    tcg_temp_free_i64(tmp1);
>  }
>
>  /* Signed 32x32->64 multiply.  */
> @@ -326,9 +335,11 @@ static void gen_imull(TCGv a, TCGv b)
>     tcg_gen_ext_i32_i64(tmp1, a);
>     tcg_gen_ext_i32_i64(tmp2, b);
>     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
> +    tcg_temp_free_i64(tmp2);
>     tcg_gen_trunc_i64_i32(a, tmp1);
>     tcg_gen_shri_i64(tmp1, tmp1, 32);
>     tcg_gen_trunc_i64_i32(b, tmp1);
> +    tcg_temp_free_i64(tmp1);
>  }
>
>  /* Swap low and high halfwords.  */
> @@ -542,11 +553,13 @@ static void gen_arm_parallel_addsub(int op1, int op2, TCGv a, TCGv b)
>         tmp = tcg_temp_new_ptr();
>         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
>         PAS_OP(s)
> +        tcg_temp_free_ptr(tmp);
>         break;
>     case 5:
>         tmp = tcg_temp_new_ptr();
>         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
>         PAS_OP(u)
> +        tcg_temp_free_ptr(tmp);
>         break;
>  #undef gen_pas_helper
>  #define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
> @@ -587,11 +600,13 @@ static void gen_thumb2_parallel_addsub(int op1, int op2, TCGv a, TCGv b)
>         tmp = tcg_temp_new_ptr();
>         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
>         PAS_OP(s)
> +        tcg_temp_free_ptr(tmp);
>         break;
>     case 4:
>         tmp = tcg_temp_new_ptr();
>         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
>         PAS_OP(u)
> +        tcg_temp_free_ptr(tmp);
>         break;
>  #undef gen_pas_helper
>  #define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
> @@ -995,10 +1010,12 @@ static inline void gen_vfp_tosiz(int dp)
>  #define VFP_GEN_FIX(name) \
>  static inline void gen_vfp_##name(int dp, int shift) \
>  { \
> +    TCGv tmp_shift = tcg_const_i32(shift); \
>     if (dp) \
> -        gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tcg_const_i32(shift), cpu_env);\
> +        gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tmp_shift, cpu_env);\
>     else \
> -        gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tcg_const_i32(shift), cpu_env);\
> +        gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tmp_shift, cpu_env);\
> +    tcg_temp_free_i32(tmp_shift); \
>  }
>  VFP_GEN_FIX(tosh)
>  VFP_GEN_FIX(tosl)
> @@ -2399,7 +2416,7 @@ static int disas_dsp_insn(CPUState *env, DisasContext *s, uint32_t insn)
>    instruction is not defined.  */
>  static int disas_cp_insn(CPUState *env, DisasContext *s, uint32_t insn)
>  {
> -    TCGv tmp;
> +    TCGv tmp, tmp2;
>     uint32_t rd = (insn >> 12) & 0xf;
>     uint32_t cp = (insn >> 8) & 0xf;
>     if (IS_USER(s)) {
> @@ -2411,14 +2428,18 @@ static int disas_cp_insn(CPUState *env, DisasContext *s, uint32_t insn)
>             return 1;
>         gen_set_pc_im(s->pc);
>         tmp = new_tmp();
> -        gen_helper_get_cp(tmp, cpu_env, tcg_const_i32(insn));
> +        tmp2 = tcg_const_i32(insn);
> +        gen_helper_get_cp(tmp, cpu_env, tmp2);
> +        tcg_temp_free(tmp2);
>         store_reg(s, rd, tmp);
>     } else {
>         if (!env->cp[cp].cp_write)
>             return 1;
>         gen_set_pc_im(s->pc);
>         tmp = load_reg(s, rd);
> -        gen_helper_set_cp(cpu_env, tcg_const_i32(insn), tmp);
> +        tmp2 = tcg_const_i32(insn);
> +        gen_helper_set_cp(cpu_env, tmp2, tmp);
> +        tcg_temp_free(tmp2);
>         dead_tmp(tmp);
>     }
>     return 0;
> @@ -2449,7 +2470,7 @@ static int cp15_user_ok(uint32_t insn)
>  static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
>  {
>     uint32_t rd;
> -    TCGv tmp;
> +    TCGv tmp, tmp2;
>
>     /* M profile cores use memory mapped registers instead of cp15.  */
>     if (arm_feature(env, ARM_FEATURE_M))
> @@ -2478,9 +2499,10 @@ static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
>         return 0;
>     }
>     rd = (insn >> 12) & 0xf;
> +    tmp2 = tcg_const_i32(insn);
>     if (insn & ARM_CP_RW_BIT) {
>         tmp = new_tmp();
> -        gen_helper_get_cp15(tmp, cpu_env, tcg_const_i32(insn));
> +        gen_helper_get_cp15(tmp, cpu_env, tmp2);
>         /* If the destination register is r15 then sets condition codes.  */
>         if (rd != 15)
>             store_reg(s, rd, tmp);
> @@ -2488,7 +2510,7 @@ static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
>             dead_tmp(tmp);
>     } else {
>         tmp = load_reg(s, rd);
> -        gen_helper_set_cp15(cpu_env, tcg_const_i32(insn), tmp);
> +        gen_helper_set_cp15(cpu_env, tmp2, tmp);
>         dead_tmp(tmp);
>         /* Normally we would always end the TB here, but Linux
>          * arch/arm/mach-pxa/sleep.S expects two instructions following
> @@ -2497,6 +2519,7 @@ static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
>                 (insn & 0x0fff0fff) != 0x0e010f10)
>             gen_lookup_tb(s);
>     }
> +    tcg_temp_free_i32(tmp2);
>     return 0;
>  }
>
> @@ -4058,9 +4081,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>     int u;
>     int n;
>     uint32_t imm;
> -    TCGv tmp;
> -    TCGv tmp2;
> -    TCGv tmp3;
> +    TCGv tmp, tmp2, tmp3, tmp4, tmp5;
>     TCGv_i64 tmp64;
>
>     if (!vfp_enabled(env))
> @@ -4676,12 +4697,18 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                             gen_neon_narrow_satu(size - 1, tmp, cpu_V0);
>                     }
>                     if (pass == 0) {
> +                        if (size != 3) {
> +                            dead_tmp(tmp2);
> +                        }
>                         tmp2 = tmp;
>                     } else {
>                         neon_store_reg(rd, 0, tmp2);
>                         neon_store_reg(rd, 1, tmp);
>                     }
>                 } /* for pass */
> +                if (size == 3) {
> +                    tcg_temp_free_i64(tmp64);
> +                }
>             } else if (op == 10) {
>                 /* VSHLL */
>                 if (q || size == 3)
> @@ -5147,6 +5174,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                     tcg_gen_shli_i64(cpu_V1, cpu_V1, 64 - (imm * 8));
>                     tcg_gen_shri_i64(tmp64, tmp64, imm * 8);
>                     tcg_gen_or_i64(cpu_V1, cpu_V1, tmp64);
> +                    tcg_temp_free_i64(tmp64);
>                 } else {
>                     /* BUGFIX */
>                     neon_load_reg64(cpu_V0, rn);
> @@ -5511,8 +5539,9 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                     tcg_gen_movi_i32(tmp, 0);
>                 }
>                 tmp2 = neon_load_reg(rm, 0);
> -                gen_helper_neon_tbl(tmp2, tmp2, tmp, tcg_const_i32(rn),
> -                                    tcg_const_i32(n));
> +                tmp4 = tcg_const_i32(rn);
> +                tmp5 = tcg_const_i32(n);
> +                gen_helper_neon_tbl(tmp2, tmp2, tmp, tmp4, tmp5);
>                 dead_tmp(tmp);
>                 if (insn & (1 << 6)) {
>                     tmp = neon_load_reg(rd, 1);
> @@ -5521,8 +5550,9 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                     tcg_gen_movi_i32(tmp, 0);
>                 }
>                 tmp3 = neon_load_reg(rm, 1);
> -                gen_helper_neon_tbl(tmp3, tmp3, tmp, tcg_const_i32(rn),
> -                                    tcg_const_i32(n));
> +                gen_helper_neon_tbl(tmp3, tmp3, tmp, tmp4, tmp5);
> +                dead_tmp(tmp5);
> +                dead_tmp(tmp4);
>                 neon_store_reg(rd, 0, tmp2);
>                 neon_store_reg(rd, 1, tmp3);
>                 dead_tmp(tmp);
> @@ -5685,6 +5715,7 @@ static void gen_addq_lo(DisasContext *s, TCGv_i64 val, int rlow)
>     tcg_gen_extu_i32_i64(tmp, tmp2);
>     dead_tmp(tmp2);
>     tcg_gen_add_i64(val, val, tmp);
> +    tcg_temp_free_i64(tmp);
>  }
>
>  /* load and add a 64-bit value from a register pair.  */
> @@ -5702,6 +5733,7 @@ static void gen_addq(DisasContext *s, TCGv_i64 val, int rlow, int rhigh)
>     dead_tmp(tmpl);
>     dead_tmp(tmph);
>     tcg_gen_add_i64(val, val, tmp);
> +    tcg_temp_free_i64(tmp);
>  }
>
>  /* Set N and Z flags from a 64-bit value.  */
> @@ -5785,7 +5817,9 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                 addr = load_reg(s, 13);
>             } else {
>                 addr = new_tmp();
> -                gen_helper_get_r13_banked(addr, cpu_env, tcg_const_i32(op1));
> +                tmp = tcg_const_i32(op1);
> +                gen_helper_get_r13_banked(addr, cpu_env, tmp);
> +                tcg_temp_free_i32(tmp);
>             }
>             i = (insn >> 23) & 3;
>             switch (i) {
> @@ -5816,7 +5850,9 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                 if (op1 == (env->uncached_cpsr & CPSR_M)) {
>                     store_reg(s, 13, addr);
>                 } else {
> -                    gen_helper_set_r13_banked(cpu_env, tcg_const_i32(op1), addr);
> +                    tmp = tcg_const_i32(op1);
> +                    gen_helper_set_r13_banked(cpu_env, tmp, addr);
> +                    tcg_temp_free_i32(tmp);
>                     dead_tmp(addr);
>                 }
>             } else {
> @@ -6058,6 +6094,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                 tcg_gen_shri_i64(tmp64, tmp64, 16);
>                 tmp = new_tmp();
>                 tcg_gen_trunc_i64_i32(tmp, tmp64);
> +                tcg_temp_free_i64(tmp64);
>                 if ((sh & 2) == 0) {
>                     tmp2 = load_reg(s, rn);
>                     gen_helper_add_setq(tmp, tmp, tmp2);
> @@ -6076,6 +6113,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                     dead_tmp(tmp);
>                     gen_addq(s, tmp64, rn, rd);
>                     gen_storeq_reg(s, rn, rd, tmp64);
> +                    tcg_temp_free_i64(tmp64);
>                 } else {
>                     if (op1 == 0) {
>                         tmp2 = load_reg(s, rn);
> @@ -6326,6 +6364,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                         if (insn & (1 << 20))
>                             gen_logicq_cc(tmp64);
>                         gen_storeq_reg(s, rn, rd, tmp64);
> +                        tcg_temp_free_i64(tmp64);
>                         break;
>                     }
>                 } else {
> @@ -6545,10 +6584,12 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                         }
>                         sh = (insn >> 16) & 0x1f;
>                         if (sh != 0) {
> +                            tmp2 = tcg_const_i32(sh);
>                             if (insn & (1 << 22))
> -                                gen_helper_usat(tmp, tmp, tcg_const_i32(sh));
> +                                gen_helper_usat(tmp, tmp, tmp2);
>                             else
> -                                gen_helper_ssat(tmp, tmp, tcg_const_i32(sh));
> +                                gen_helper_ssat(tmp, tmp, tmp2);
> +                            tcg_temp_free_i32(tmp2);
>                         }
>                         store_reg(s, rd, tmp);
>                     } else if ((insn & 0x00300fe0) == 0x00200f20) {
> @@ -6556,10 +6597,12 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                         tmp = load_reg(s, rm);
>                         sh = (insn >> 16) & 0x1f;
>                         if (sh != 0) {
> +                            tmp2 = tcg_const_i32(sh);
>                             if (insn & (1 << 22))
> -                                gen_helper_usat16(tmp, tmp, tcg_const_i32(sh));
> +                                gen_helper_usat16(tmp, tmp, tmp2);
>                             else
> -                                gen_helper_ssat16(tmp, tmp, tcg_const_i32(sh));
> +                                gen_helper_ssat16(tmp, tmp, tmp2);
> +                            tcg_temp_free_i32(tmp2);
>                         }
>                         store_reg(s, rd, tmp);
>                     } else if ((insn & 0x00700fe0) == 0x00000fa0) {
> @@ -6631,6 +6674,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                         tcg_gen_shri_i64(tmp64, tmp64, 32);
>                         tmp = new_tmp();
>                         tcg_gen_trunc_i64_i32(tmp, tmp64);
> +                        tcg_temp_free_i64(tmp64);
>                         if (rd != 15) {
>                             tmp2 = load_reg(s, rd);
>                             if (insn & (1 << 6)) {
> @@ -6659,6 +6703,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                             dead_tmp(tmp);
>                             gen_addq(s, tmp64, rd, rn);
>                             gen_storeq_reg(s, rd, rn, tmp64);
> +                            tcg_temp_free_i64(tmp64);
>                         } else {
>                             /* smuad, smusd, smlad, smlsd */
>                             if (rd != 15)
> @@ -6831,7 +6876,9 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                             if (i == 15) {
>                                 gen_bx(s, tmp);
>                             } else if (user) {
> -                                gen_helper_set_user_reg(tcg_const_i32(i), tmp);
> +                                tmp2 = tcg_const_i32(i);
> +                                gen_helper_set_user_reg(tmp2, tmp);
> +                                tcg_temp_free_i32(tmp2);
>                                 dead_tmp(tmp);
>                             } else if (i == rn) {
>                                 loaded_var = tmp;
> @@ -6848,7 +6895,9 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                                 tcg_gen_movi_i32(tmp, val);
>                             } else if (user) {
>                                 tmp = new_tmp();
> -                                gen_helper_get_user_reg(tmp, tcg_const_i32(i));
> +                                tmp2 = tcg_const_i32(i);
> +                                gen_helper_get_user_reg(tmp, tmp2);
> +                                tcg_temp_free_i32(tmp2);
>                             } else {
>                                 tmp = load_reg(s, i);
>                             }
> @@ -7264,7 +7313,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                         addr = load_reg(s, 13);
>                     } else {
>                         addr = new_tmp();
> -                        gen_helper_get_r13_banked(addr, cpu_env, tcg_const_i32(op));
> +                        tmp = tcg_const_i32(op);
> +                        gen_helper_get_r13_banked(addr, cpu_env, tmp);
> +                        tcg_temp_free_i32(tmp);
>                     }
>                     if ((insn & (1 << 24)) == 0) {
>                         tcg_gen_addi_i32(addr, addr, -8);
> @@ -7284,8 +7335,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                         if (op == (env->uncached_cpsr & CPSR_M)) {
>                             store_reg(s, 13, addr);
>                         } else {
> -                            gen_helper_set_r13_banked(cpu_env,
> -                                tcg_const_i32(op), addr);
> +                            tmp = tcg_const_i32(op);
> +                            gen_helper_set_r13_banked(cpu_env, tmp, addr);
> +                            tcg_temp_free_i32(tmp);
>                         }
>                     } else {
>                         dead_tmp(addr);
> @@ -7515,6 +7567,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                 tcg_gen_shri_i64(tmp64, tmp64, 16);
>                 tmp = new_tmp();
>                 tcg_gen_trunc_i64_i32(tmp, tmp64);
> +                tcg_temp_free_i64(tmp64);
>                 if (rs != 15)
>                   {
>                     tmp2 = load_reg(s, rs);
> @@ -7584,6 +7637,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                 dead_tmp(tmp);
>                 gen_addq(s, tmp64, rs, rd);
>                 gen_storeq_reg(s, rs, rd, tmp64);
> +                tcg_temp_free_i64(tmp64);
>             } else {
>                 if (op & 0x20) {
>                     /* Unsigned 64-bit multiply  */
> @@ -7610,6 +7664,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                     gen_addq(s, tmp64, rs, rd);
>                 }
>                 gen_storeq_reg(s, rs, rd, tmp64);
> +                tcg_temp_free_i64(tmp64);
>             }
>             break;
>         }
> @@ -7673,6 +7728,8 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                             tmp = load_reg(s, rn);
>                             addr = tcg_const_i32(insn & 0xff);
>                             gen_helper_v7m_msr(cpu_env, addr, tmp);
> +                            tcg_temp_free_i32(addr);
> +                            dead_tmp(tmp);
>                             gen_lookup_tb(s);
>                             break;
>                         }
> @@ -7742,6 +7799,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                         if (IS_M(env)) {
>                             addr = tcg_const_i32(insn & 0xff);
>                             gen_helper_v7m_mrs(tmp, cpu_env, addr);
> +                            tcg_temp_free_i32(addr);
>                         } else {
>                             gen_helper_cpsr_read(tmp);
>                         }
> @@ -7842,6 +7900,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                             else
>                                 gen_helper_ssat(tmp, tmp, tmp2);
>                         }
> +                        tcg_temp_free_i32(tmp2);
>                         break;
>                     }
>                     store_reg(s, rd, tmp);
> @@ -8614,12 +8673,15 @@ static void disas_thumb_insn(CPUState *env, DisasContext *s)
>                 if (insn & 1) {
>                     addr = tcg_const_i32(16);
>                     gen_helper_v7m_msr(cpu_env, addr, tmp);
> +                    tcg_temp_free_i32(addr);
>                 }
>                 /* FAULTMASK */
>                 if (insn & 2) {
>                     addr = tcg_const_i32(17);
>                     gen_helper_v7m_msr(cpu_env, addr, tmp);
> +                    tcg_temp_free_i32(addr);
>                 }
> +                tcg_temp_free_i32(tmp);
>                 gen_lookup_tb(s);
>             } else {
>                 if (insn & (1 << 4))
> --
> 1.6.1
>
>
>
>
Laurent Desnogues Oct. 26, 2009, 10:46 a.m. UTC | #2
On Thu, Oct 22, 2009 at 1:17 PM,  <juha.riihimaki@nokia.com> wrote:
> From: Juha Riihimäki <juha.riihimaki@nokia.com>
>
> Revised patch for getting rid of tcg temporary variable leaks in
> target-arm/translate.c. This version also includes the leak patch for
> gen_set_cpsr macro, now converted as a static inline function, which I
> sent earlier as a separate patch on top of this patch.
>
> Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
> ---
>  target-arm/translate.c |  116 ++++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 89 insertions(+), 27 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index e56082b..813f661 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -184,7 +184,12 @@ static void store_reg(DisasContext *s, int reg, TCGv var)
>  #define gen_uxtb16(var) gen_helper_uxtb16(var, var)
>
>
> -#define gen_set_cpsr(var, mask) gen_helper_cpsr_write(var, tcg_const_i32(mask))
> +static inline void gen_set_cpsr(TCGv var, uint32_t mask)
> +{
> +    TCGv tmp_mask = tcg_const_i32(mask);
> +    gen_helper_cpsr_write(var, tmp_mask);
> +    tcg_temp_free_i32(tmp_mask);
> +}
>  /* Set NZCV flags from the high 4 bits of var.  */
>  #define gen_set_nzcv(var) gen_set_cpsr(var, CPSR_NZCV)
>
> @@ -287,6 +292,7 @@ static TCGv_i64 gen_mulu_i64_i32(TCGv a, TCGv b)
>     tcg_gen_extu_i32_i64(tmp2, b);
>     dead_tmp(b);
>     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
> +    tcg_temp_free_i64(tmp2);
>     return tmp1;
>  }
>
> @@ -300,6 +306,7 @@ static TCGv_i64 gen_muls_i64_i32(TCGv a, TCGv b)
>     tcg_gen_ext_i32_i64(tmp2, b);
>     dead_tmp(b);
>     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
> +    tcg_temp_free_i64(tmp2);
>     return tmp1;
>  }
>
> @@ -312,9 +319,11 @@ static void gen_mull(TCGv a, TCGv b)
>     tcg_gen_extu_i32_i64(tmp1, a);
>     tcg_gen_extu_i32_i64(tmp2, b);
>     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
> +    tcg_temp_free_i64(tmp2);
>     tcg_gen_trunc_i64_i32(a, tmp1);
>     tcg_gen_shri_i64(tmp1, tmp1, 32);
>     tcg_gen_trunc_i64_i32(b, tmp1);
> +    tcg_temp_free_i64(tmp1);
>  }
>
>  /* Signed 32x32->64 multiply.  */
> @@ -326,9 +335,11 @@ static void gen_imull(TCGv a, TCGv b)
>     tcg_gen_ext_i32_i64(tmp1, a);
>     tcg_gen_ext_i32_i64(tmp2, b);
>     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
> +    tcg_temp_free_i64(tmp2);
>     tcg_gen_trunc_i64_i32(a, tmp1);
>     tcg_gen_shri_i64(tmp1, tmp1, 32);
>     tcg_gen_trunc_i64_i32(b, tmp1);
> +    tcg_temp_free_i64(tmp1);
>  }
>
>  /* Swap low and high halfwords.  */
> @@ -542,11 +553,13 @@ static void gen_arm_parallel_addsub(int op1, int op2, TCGv a, TCGv b)
>         tmp = tcg_temp_new_ptr();
>         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
>         PAS_OP(s)
> +        tcg_temp_free_ptr(tmp);
>         break;
>     case 5:
>         tmp = tcg_temp_new_ptr();
>         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
>         PAS_OP(u)
> +        tcg_temp_free_ptr(tmp);
>         break;
>  #undef gen_pas_helper
>  #define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
> @@ -587,11 +600,13 @@ static void gen_thumb2_parallel_addsub(int op1, int op2, TCGv a, TCGv b)
>         tmp = tcg_temp_new_ptr();
>         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
>         PAS_OP(s)
> +        tcg_temp_free_ptr(tmp);
>         break;
>     case 4:
>         tmp = tcg_temp_new_ptr();
>         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
>         PAS_OP(u)
> +        tcg_temp_free_ptr(tmp);
>         break;
>  #undef gen_pas_helper
>  #define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
> @@ -995,10 +1010,12 @@ static inline void gen_vfp_tosiz(int dp)
>  #define VFP_GEN_FIX(name) \
>  static inline void gen_vfp_##name(int dp, int shift) \
>  { \
> +    TCGv tmp_shift = tcg_const_i32(shift); \
>     if (dp) \
> -        gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tcg_const_i32(shift), cpu_env);\
> +        gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tmp_shift, cpu_env);\
>     else \
> -        gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tcg_const_i32(shift), cpu_env);\
> +        gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tmp_shift, cpu_env);\
> +    tcg_temp_free_i32(tmp_shift); \
>  }
>  VFP_GEN_FIX(tosh)
>  VFP_GEN_FIX(tosl)
> @@ -2399,7 +2416,7 @@ static int disas_dsp_insn(CPUState *env, DisasContext *s, uint32_t insn)
>    instruction is not defined.  */
>  static int disas_cp_insn(CPUState *env, DisasContext *s, uint32_t insn)
>  {
> -    TCGv tmp;
> +    TCGv tmp, tmp2;
>     uint32_t rd = (insn >> 12) & 0xf;
>     uint32_t cp = (insn >> 8) & 0xf;
>     if (IS_USER(s)) {
> @@ -2411,14 +2428,18 @@ static int disas_cp_insn(CPUState *env, DisasContext *s, uint32_t insn)
>             return 1;
>         gen_set_pc_im(s->pc);
>         tmp = new_tmp();
> -        gen_helper_get_cp(tmp, cpu_env, tcg_const_i32(insn));
> +        tmp2 = tcg_const_i32(insn);
> +        gen_helper_get_cp(tmp, cpu_env, tmp2);
> +        tcg_temp_free(tmp2);
>         store_reg(s, rd, tmp);
>     } else {
>         if (!env->cp[cp].cp_write)
>             return 1;
>         gen_set_pc_im(s->pc);
>         tmp = load_reg(s, rd);
> -        gen_helper_set_cp(cpu_env, tcg_const_i32(insn), tmp);
> +        tmp2 = tcg_const_i32(insn);
> +        gen_helper_set_cp(cpu_env, tmp2, tmp);
> +        tcg_temp_free(tmp2);
>         dead_tmp(tmp);
>     }
>     return 0;
> @@ -2449,7 +2470,7 @@ static int cp15_user_ok(uint32_t insn)
>  static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
>  {
>     uint32_t rd;
> -    TCGv tmp;
> +    TCGv tmp, tmp2;
>
>     /* M profile cores use memory mapped registers instead of cp15.  */
>     if (arm_feature(env, ARM_FEATURE_M))
> @@ -2478,9 +2499,10 @@ static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
>         return 0;
>     }
>     rd = (insn >> 12) & 0xf;
> +    tmp2 = tcg_const_i32(insn);
>     if (insn & ARM_CP_RW_BIT) {
>         tmp = new_tmp();
> -        gen_helper_get_cp15(tmp, cpu_env, tcg_const_i32(insn));
> +        gen_helper_get_cp15(tmp, cpu_env, tmp2);
>         /* If the destination register is r15 then sets condition codes.  */
>         if (rd != 15)
>             store_reg(s, rd, tmp);
> @@ -2488,7 +2510,7 @@ static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
>             dead_tmp(tmp);
>     } else {
>         tmp = load_reg(s, rd);
> -        gen_helper_set_cp15(cpu_env, tcg_const_i32(insn), tmp);
> +        gen_helper_set_cp15(cpu_env, tmp2, tmp);
>         dead_tmp(tmp);
>         /* Normally we would always end the TB here, but Linux
>          * arch/arm/mach-pxa/sleep.S expects two instructions following
> @@ -2497,6 +2519,7 @@ static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
>                 (insn & 0x0fff0fff) != 0x0e010f10)
>             gen_lookup_tb(s);
>     }
> +    tcg_temp_free_i32(tmp2);
>     return 0;
>  }
>
> @@ -4058,9 +4081,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>     int u;
>     int n;
>     uint32_t imm;
> -    TCGv tmp;
> -    TCGv tmp2;
> -    TCGv tmp3;
> +    TCGv tmp, tmp2, tmp3, tmp4, tmp5;
>     TCGv_i64 tmp64;
>
>     if (!vfp_enabled(env))
> @@ -4676,12 +4697,18 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                             gen_neon_narrow_satu(size - 1, tmp, cpu_V0);
>                     }
>                     if (pass == 0) {
> +                        if (size != 3) {
> +                            dead_tmp(tmp2);
> +                        }
>                         tmp2 = tmp;
>                     } else {
>                         neon_store_reg(rd, 0, tmp2);
>                         neon_store_reg(rd, 1, tmp);
>                     }
>                 } /* for pass */
> +                if (size == 3) {
> +                    tcg_temp_free_i64(tmp64);
> +                }
>             } else if (op == 10) {
>                 /* VSHLL */
>                 if (q || size == 3)
> @@ -5147,6 +5174,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                     tcg_gen_shli_i64(cpu_V1, cpu_V1, 64 - (imm * 8));
>                     tcg_gen_shri_i64(tmp64, tmp64, imm * 8);
>                     tcg_gen_or_i64(cpu_V1, cpu_V1, tmp64);
> +                    tcg_temp_free_i64(tmp64);
>                 } else {
>                     /* BUGFIX */
>                     neon_load_reg64(cpu_V0, rn);
> @@ -5511,8 +5539,9 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                     tcg_gen_movi_i32(tmp, 0);
>                 }
>                 tmp2 = neon_load_reg(rm, 0);
> -                gen_helper_neon_tbl(tmp2, tmp2, tmp, tcg_const_i32(rn),
> -                                    tcg_const_i32(n));
> +                tmp4 = tcg_const_i32(rn);
> +                tmp5 = tcg_const_i32(n);
> +                gen_helper_neon_tbl(tmp2, tmp2, tmp, tmp4, tmp5);
>                 dead_tmp(tmp);
>                 if (insn & (1 << 6)) {
>                     tmp = neon_load_reg(rd, 1);
> @@ -5521,8 +5550,9 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>                     tcg_gen_movi_i32(tmp, 0);
>                 }
>                 tmp3 = neon_load_reg(rm, 1);
> -                gen_helper_neon_tbl(tmp3, tmp3, tmp, tcg_const_i32(rn),
> -                                    tcg_const_i32(n));
> +                gen_helper_neon_tbl(tmp3, tmp3, tmp, tmp4, tmp5);
> +                dead_tmp(tmp5);
> +                dead_tmp(tmp4);

I missed this mistake when I acked the patch:  you're using
dead_tmp instead of tcg_temp_free...  One more reason
to drop dead_tmp :-)

Sorry,

Laurent

>                 neon_store_reg(rd, 0, tmp2);
>                 neon_store_reg(rd, 1, tmp3);
>                 dead_tmp(tmp);
> @@ -5685,6 +5715,7 @@ static void gen_addq_lo(DisasContext *s, TCGv_i64 val, int rlow)
>     tcg_gen_extu_i32_i64(tmp, tmp2);
>     dead_tmp(tmp2);
>     tcg_gen_add_i64(val, val, tmp);
> +    tcg_temp_free_i64(tmp);
>  }
>
>  /* load and add a 64-bit value from a register pair.  */
> @@ -5702,6 +5733,7 @@ static void gen_addq(DisasContext *s, TCGv_i64 val, int rlow, int rhigh)
>     dead_tmp(tmpl);
>     dead_tmp(tmph);
>     tcg_gen_add_i64(val, val, tmp);
> +    tcg_temp_free_i64(tmp);
>  }
>
>  /* Set N and Z flags from a 64-bit value.  */
> @@ -5785,7 +5817,9 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                 addr = load_reg(s, 13);
>             } else {
>                 addr = new_tmp();
> -                gen_helper_get_r13_banked(addr, cpu_env, tcg_const_i32(op1));
> +                tmp = tcg_const_i32(op1);
> +                gen_helper_get_r13_banked(addr, cpu_env, tmp);
> +                tcg_temp_free_i32(tmp);
>             }
>             i = (insn >> 23) & 3;
>             switch (i) {
> @@ -5816,7 +5850,9 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                 if (op1 == (env->uncached_cpsr & CPSR_M)) {
>                     store_reg(s, 13, addr);
>                 } else {
> -                    gen_helper_set_r13_banked(cpu_env, tcg_const_i32(op1), addr);
> +                    tmp = tcg_const_i32(op1);
> +                    gen_helper_set_r13_banked(cpu_env, tmp, addr);
> +                    tcg_temp_free_i32(tmp);
>                     dead_tmp(addr);
>                 }
>             } else {
> @@ -6058,6 +6094,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                 tcg_gen_shri_i64(tmp64, tmp64, 16);
>                 tmp = new_tmp();
>                 tcg_gen_trunc_i64_i32(tmp, tmp64);
> +                tcg_temp_free_i64(tmp64);
>                 if ((sh & 2) == 0) {
>                     tmp2 = load_reg(s, rn);
>                     gen_helper_add_setq(tmp, tmp, tmp2);
> @@ -6076,6 +6113,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                     dead_tmp(tmp);
>                     gen_addq(s, tmp64, rn, rd);
>                     gen_storeq_reg(s, rn, rd, tmp64);
> +                    tcg_temp_free_i64(tmp64);
>                 } else {
>                     if (op1 == 0) {
>                         tmp2 = load_reg(s, rn);
> @@ -6326,6 +6364,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                         if (insn & (1 << 20))
>                             gen_logicq_cc(tmp64);
>                         gen_storeq_reg(s, rn, rd, tmp64);
> +                        tcg_temp_free_i64(tmp64);
>                         break;
>                     }
>                 } else {
> @@ -6545,10 +6584,12 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                         }
>                         sh = (insn >> 16) & 0x1f;
>                         if (sh != 0) {
> +                            tmp2 = tcg_const_i32(sh);
>                             if (insn & (1 << 22))
> -                                gen_helper_usat(tmp, tmp, tcg_const_i32(sh));
> +                                gen_helper_usat(tmp, tmp, tmp2);
>                             else
> -                                gen_helper_ssat(tmp, tmp, tcg_const_i32(sh));
> +                                gen_helper_ssat(tmp, tmp, tmp2);
> +                            tcg_temp_free_i32(tmp2);
>                         }
>                         store_reg(s, rd, tmp);
>                     } else if ((insn & 0x00300fe0) == 0x00200f20) {
> @@ -6556,10 +6597,12 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                         tmp = load_reg(s, rm);
>                         sh = (insn >> 16) & 0x1f;
>                         if (sh != 0) {
> +                            tmp2 = tcg_const_i32(sh);
>                             if (insn & (1 << 22))
> -                                gen_helper_usat16(tmp, tmp, tcg_const_i32(sh));
> +                                gen_helper_usat16(tmp, tmp, tmp2);
>                             else
> -                                gen_helper_ssat16(tmp, tmp, tcg_const_i32(sh));
> +                                gen_helper_ssat16(tmp, tmp, tmp2);
> +                            tcg_temp_free_i32(tmp2);
>                         }
>                         store_reg(s, rd, tmp);
>                     } else if ((insn & 0x00700fe0) == 0x00000fa0) {
> @@ -6631,6 +6674,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                         tcg_gen_shri_i64(tmp64, tmp64, 32);
>                         tmp = new_tmp();
>                         tcg_gen_trunc_i64_i32(tmp, tmp64);
> +                        tcg_temp_free_i64(tmp64);
>                         if (rd != 15) {
>                             tmp2 = load_reg(s, rd);
>                             if (insn & (1 << 6)) {
> @@ -6659,6 +6703,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                             dead_tmp(tmp);
>                             gen_addq(s, tmp64, rd, rn);
>                             gen_storeq_reg(s, rd, rn, tmp64);
> +                            tcg_temp_free_i64(tmp64);
>                         } else {
>                             /* smuad, smusd, smlad, smlsd */
>                             if (rd != 15)
> @@ -6831,7 +6876,9 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                             if (i == 15) {
>                                 gen_bx(s, tmp);
>                             } else if (user) {
> -                                gen_helper_set_user_reg(tcg_const_i32(i), tmp);
> +                                tmp2 = tcg_const_i32(i);
> +                                gen_helper_set_user_reg(tmp2, tmp);
> +                                tcg_temp_free_i32(tmp2);
>                                 dead_tmp(tmp);
>                             } else if (i == rn) {
>                                 loaded_var = tmp;
> @@ -6848,7 +6895,9 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
>                                 tcg_gen_movi_i32(tmp, val);
>                             } else if (user) {
>                                 tmp = new_tmp();
> -                                gen_helper_get_user_reg(tmp, tcg_const_i32(i));
> +                                tmp2 = tcg_const_i32(i);
> +                                gen_helper_get_user_reg(tmp, tmp2);
> +                                tcg_temp_free_i32(tmp2);
>                             } else {
>                                 tmp = load_reg(s, i);
>                             }
> @@ -7264,7 +7313,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                         addr = load_reg(s, 13);
>                     } else {
>                         addr = new_tmp();
> -                        gen_helper_get_r13_banked(addr, cpu_env, tcg_const_i32(op));
> +                        tmp = tcg_const_i32(op);
> +                        gen_helper_get_r13_banked(addr, cpu_env, tmp);
> +                        tcg_temp_free_i32(tmp);
>                     }
>                     if ((insn & (1 << 24)) == 0) {
>                         tcg_gen_addi_i32(addr, addr, -8);
> @@ -7284,8 +7335,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                         if (op == (env->uncached_cpsr & CPSR_M)) {
>                             store_reg(s, 13, addr);
>                         } else {
> -                            gen_helper_set_r13_banked(cpu_env,
> -                                tcg_const_i32(op), addr);
> +                            tmp = tcg_const_i32(op);
> +                            gen_helper_set_r13_banked(cpu_env, tmp, addr);
> +                            tcg_temp_free_i32(tmp);
>                         }
>                     } else {
>                         dead_tmp(addr);
> @@ -7515,6 +7567,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                 tcg_gen_shri_i64(tmp64, tmp64, 16);
>                 tmp = new_tmp();
>                 tcg_gen_trunc_i64_i32(tmp, tmp64);
> +                tcg_temp_free_i64(tmp64);
>                 if (rs != 15)
>                   {
>                     tmp2 = load_reg(s, rs);
> @@ -7584,6 +7637,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                 dead_tmp(tmp);
>                 gen_addq(s, tmp64, rs, rd);
>                 gen_storeq_reg(s, rs, rd, tmp64);
> +                tcg_temp_free_i64(tmp64);
>             } else {
>                 if (op & 0x20) {
>                     /* Unsigned 64-bit multiply  */
> @@ -7610,6 +7664,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                     gen_addq(s, tmp64, rs, rd);
>                 }
>                 gen_storeq_reg(s, rs, rd, tmp64);
> +                tcg_temp_free_i64(tmp64);
>             }
>             break;
>         }
> @@ -7673,6 +7728,8 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                             tmp = load_reg(s, rn);
>                             addr = tcg_const_i32(insn & 0xff);
>                             gen_helper_v7m_msr(cpu_env, addr, tmp);
> +                            tcg_temp_free_i32(addr);
> +                            dead_tmp(tmp);
>                             gen_lookup_tb(s);
>                             break;
>                         }
> @@ -7742,6 +7799,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                         if (IS_M(env)) {
>                             addr = tcg_const_i32(insn & 0xff);
>                             gen_helper_v7m_mrs(tmp, cpu_env, addr);
> +                            tcg_temp_free_i32(addr);
>                         } else {
>                             gen_helper_cpsr_read(tmp);
>                         }
> @@ -7842,6 +7900,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                             else
>                                 gen_helper_ssat(tmp, tmp, tmp2);
>                         }
> +                        tcg_temp_free_i32(tmp2);
>                         break;
>                     }
>                     store_reg(s, rd, tmp);
> @@ -8614,12 +8673,15 @@ static void disas_thumb_insn(CPUState *env, DisasContext *s)
>                 if (insn & 1) {
>                     addr = tcg_const_i32(16);
>                     gen_helper_v7m_msr(cpu_env, addr, tmp);
> +                    tcg_temp_free_i32(addr);
>                 }
>                 /* FAULTMASK */
>                 if (insn & 2) {
>                     addr = tcg_const_i32(17);
>                     gen_helper_v7m_msr(cpu_env, addr, tmp);
> +                    tcg_temp_free_i32(addr);
>                 }
> +                tcg_temp_free_i32(tmp);
>                 gen_lookup_tb(s);
>             } else {
>                 if (insn & (1 << 4))
> --
> 1.6.1
>
>
>
>
Juha.Riihimaki@nokia.com Oct. 26, 2009, 10:52 a.m. UTC | #3
On Oct 26, 2009, at 12:46, ext Laurent Desnogues wrote:

>> @@ -5511,8 +5539,9 @@ static int disas_neon_data_insn(CPUState *  
>> env, DisasContext *s, uint32_t insn)
>>                    tcg_gen_movi_i32(tmp, 0);
>>                }
>>                tmp2 = neon_load_reg(rm, 0);
>> -                gen_helper_neon_tbl(tmp2, tmp2, tmp, tcg_const_i32 
>> (rn),
>> -                                    tcg_const_i32(n));
>> +                tmp4 = tcg_const_i32(rn);
>> +                tmp5 = tcg_const_i32(n);
>> +                gen_helper_neon_tbl(tmp2, tmp2, tmp, tmp4, tmp5);
>>                dead_tmp(tmp);
>>                if (insn & (1 << 6)) {
>>                    tmp = neon_load_reg(rd, 1);
>> @@ -5521,8 +5550,9 @@ static int disas_neon_data_insn(CPUState *  
>> env, DisasContext *s, uint32_t insn)
>>                    tcg_gen_movi_i32(tmp, 0);
>>                }
>>                tmp3 = neon_load_reg(rm, 1);
>> -                gen_helper_neon_tbl(tmp3, tmp3, tmp, tcg_const_i32 
>> (rn),
>> -                                    tcg_const_i32(n));
>> +                gen_helper_neon_tbl(tmp3, tmp3, tmp, tmp4, tmp5);
>> +                dead_tmp(tmp5);
>> +                dead_tmp(tmp4);
>
> I missed this mistake when I acked the patch:  you're using
> dead_tmp instead of tcg_temp_free...  One more reason
> to drop dead_tmp :-)

Indeed. I'll fix it, didn't notice it either because I'm using other  
new_xxx/dead_xxx helpers myself to catch resource leaks. I should work  
around that to get more similar code and eliminate this kind of issues.

The bug will cause false resource leak reports, the actual freeing of  
the temporary is done correctly. Since this patch seems to have  
already been applied, I'll just provide a patch that fixes this  
specific error in the mainline instead of reposting a fixed version of  
the original patch.


Regards,
Juha
diff mbox

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index e56082b..813f661 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -184,7 +184,12 @@  static void store_reg(DisasContext *s, int reg, TCGv var)
 #define gen_uxtb16(var) gen_helper_uxtb16(var, var)
 
 
-#define gen_set_cpsr(var, mask) gen_helper_cpsr_write(var, tcg_const_i32(mask))
+static inline void gen_set_cpsr(TCGv var, uint32_t mask)
+{
+    TCGv tmp_mask = tcg_const_i32(mask);
+    gen_helper_cpsr_write(var, tmp_mask);
+    tcg_temp_free_i32(tmp_mask);
+}
 /* Set NZCV flags from the high 4 bits of var.  */
 #define gen_set_nzcv(var) gen_set_cpsr(var, CPSR_NZCV)
 
@@ -287,6 +292,7 @@  static TCGv_i64 gen_mulu_i64_i32(TCGv a, TCGv b)
     tcg_gen_extu_i32_i64(tmp2, b);
     dead_tmp(b);
     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+    tcg_temp_free_i64(tmp2);
     return tmp1;
 }
 
@@ -300,6 +306,7 @@  static TCGv_i64 gen_muls_i64_i32(TCGv a, TCGv b)
     tcg_gen_ext_i32_i64(tmp2, b);
     dead_tmp(b);
     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+    tcg_temp_free_i64(tmp2);
     return tmp1;
 }
 
@@ -312,9 +319,11 @@  static void gen_mull(TCGv a, TCGv b)
     tcg_gen_extu_i32_i64(tmp1, a);
     tcg_gen_extu_i32_i64(tmp2, b);
     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+    tcg_temp_free_i64(tmp2);
     tcg_gen_trunc_i64_i32(a, tmp1);
     tcg_gen_shri_i64(tmp1, tmp1, 32);
     tcg_gen_trunc_i64_i32(b, tmp1);
+    tcg_temp_free_i64(tmp1);
 }
 
 /* Signed 32x32->64 multiply.  */
@@ -326,9 +335,11 @@  static void gen_imull(TCGv a, TCGv b)
     tcg_gen_ext_i32_i64(tmp1, a);
     tcg_gen_ext_i32_i64(tmp2, b);
     tcg_gen_mul_i64(tmp1, tmp1, tmp2);
+    tcg_temp_free_i64(tmp2);
     tcg_gen_trunc_i64_i32(a, tmp1);
     tcg_gen_shri_i64(tmp1, tmp1, 32);
     tcg_gen_trunc_i64_i32(b, tmp1);
+    tcg_temp_free_i64(tmp1);
 }
 
 /* Swap low and high halfwords.  */
@@ -542,11 +553,13 @@  static void gen_arm_parallel_addsub(int op1, int op2, TCGv a, TCGv b)
         tmp = tcg_temp_new_ptr();
         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
         PAS_OP(s)
+        tcg_temp_free_ptr(tmp);
         break;
     case 5:
         tmp = tcg_temp_new_ptr();
         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
         PAS_OP(u)
+        tcg_temp_free_ptr(tmp);
         break;
 #undef gen_pas_helper
 #define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
@@ -587,11 +600,13 @@  static void gen_thumb2_parallel_addsub(int op1, int op2, TCGv a, TCGv b)
         tmp = tcg_temp_new_ptr();
         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
         PAS_OP(s)
+        tcg_temp_free_ptr(tmp);
         break;
     case 4:
         tmp = tcg_temp_new_ptr();
         tcg_gen_addi_ptr(tmp, cpu_env, offsetof(CPUState, GE));
         PAS_OP(u)
+        tcg_temp_free_ptr(tmp);
         break;
 #undef gen_pas_helper
 #define gen_pas_helper(name) glue(gen_helper_,name)(a, a, b)
@@ -995,10 +1010,12 @@  static inline void gen_vfp_tosiz(int dp)
 #define VFP_GEN_FIX(name) \
 static inline void gen_vfp_##name(int dp, int shift) \
 { \
+    TCGv tmp_shift = tcg_const_i32(shift); \
     if (dp) \
-        gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tcg_const_i32(shift), cpu_env);\
+        gen_helper_vfp_##name##d(cpu_F0d, cpu_F0d, tmp_shift, cpu_env);\
     else \
-        gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tcg_const_i32(shift), cpu_env);\
+        gen_helper_vfp_##name##s(cpu_F0s, cpu_F0s, tmp_shift, cpu_env);\
+    tcg_temp_free_i32(tmp_shift); \
 }
 VFP_GEN_FIX(tosh)
 VFP_GEN_FIX(tosl)
@@ -2399,7 +2416,7 @@  static int disas_dsp_insn(CPUState *env, DisasContext *s, uint32_t insn)
    instruction is not defined.  */
 static int disas_cp_insn(CPUState *env, DisasContext *s, uint32_t insn)
 {
-    TCGv tmp;
+    TCGv tmp, tmp2;
     uint32_t rd = (insn >> 12) & 0xf;
     uint32_t cp = (insn >> 8) & 0xf;
     if (IS_USER(s)) {
@@ -2411,14 +2428,18 @@  static int disas_cp_insn(CPUState *env, DisasContext *s, uint32_t insn)
             return 1;
         gen_set_pc_im(s->pc);
         tmp = new_tmp();
-        gen_helper_get_cp(tmp, cpu_env, tcg_const_i32(insn));
+        tmp2 = tcg_const_i32(insn);
+        gen_helper_get_cp(tmp, cpu_env, tmp2);
+        tcg_temp_free(tmp2);
         store_reg(s, rd, tmp);
     } else {
         if (!env->cp[cp].cp_write)
             return 1;
         gen_set_pc_im(s->pc);
         tmp = load_reg(s, rd);
-        gen_helper_set_cp(cpu_env, tcg_const_i32(insn), tmp);
+        tmp2 = tcg_const_i32(insn);
+        gen_helper_set_cp(cpu_env, tmp2, tmp);
+        tcg_temp_free(tmp2);
         dead_tmp(tmp);
     }
     return 0;
@@ -2449,7 +2470,7 @@  static int cp15_user_ok(uint32_t insn)
 static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
 {
     uint32_t rd;
-    TCGv tmp;
+    TCGv tmp, tmp2;
 
     /* M profile cores use memory mapped registers instead of cp15.  */
     if (arm_feature(env, ARM_FEATURE_M))
@@ -2478,9 +2499,10 @@  static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
         return 0;
     }
     rd = (insn >> 12) & 0xf;
+    tmp2 = tcg_const_i32(insn);
     if (insn & ARM_CP_RW_BIT) {
         tmp = new_tmp();
-        gen_helper_get_cp15(tmp, cpu_env, tcg_const_i32(insn));
+        gen_helper_get_cp15(tmp, cpu_env, tmp2);
         /* If the destination register is r15 then sets condition codes.  */
         if (rd != 15)
             store_reg(s, rd, tmp);
@@ -2488,7 +2510,7 @@  static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
             dead_tmp(tmp);
     } else {
         tmp = load_reg(s, rd);
-        gen_helper_set_cp15(cpu_env, tcg_const_i32(insn), tmp);
+        gen_helper_set_cp15(cpu_env, tmp2, tmp);
         dead_tmp(tmp);
         /* Normally we would always end the TB here, but Linux
          * arch/arm/mach-pxa/sleep.S expects two instructions following
@@ -2497,6 +2519,7 @@  static int disas_cp15_insn(CPUState *env, DisasContext *s, uint32_t insn)
                 (insn & 0x0fff0fff) != 0x0e010f10)
             gen_lookup_tb(s);
     }
+    tcg_temp_free_i32(tmp2);
     return 0;
 }
 
@@ -4058,9 +4081,7 @@  static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
     int u;
     int n;
     uint32_t imm;
-    TCGv tmp;
-    TCGv tmp2;
-    TCGv tmp3;
+    TCGv tmp, tmp2, tmp3, tmp4, tmp5;
     TCGv_i64 tmp64;
 
     if (!vfp_enabled(env))
@@ -4676,12 +4697,18 @@  static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
                             gen_neon_narrow_satu(size - 1, tmp, cpu_V0);
                     }
                     if (pass == 0) {
+                        if (size != 3) {
+                            dead_tmp(tmp2);
+                        }
                         tmp2 = tmp;
                     } else {
                         neon_store_reg(rd, 0, tmp2);
                         neon_store_reg(rd, 1, tmp);
                     }
                 } /* for pass */
+                if (size == 3) {
+                    tcg_temp_free_i64(tmp64);
+                }
             } else if (op == 10) {
                 /* VSHLL */
                 if (q || size == 3)
@@ -5147,6 +5174,7 @@  static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
                     tcg_gen_shli_i64(cpu_V1, cpu_V1, 64 - (imm * 8));
                     tcg_gen_shri_i64(tmp64, tmp64, imm * 8);
                     tcg_gen_or_i64(cpu_V1, cpu_V1, tmp64);
+                    tcg_temp_free_i64(tmp64);
                 } else {
                     /* BUGFIX */
                     neon_load_reg64(cpu_V0, rn);
@@ -5511,8 +5539,9 @@  static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
                     tcg_gen_movi_i32(tmp, 0);
                 }
                 tmp2 = neon_load_reg(rm, 0);
-                gen_helper_neon_tbl(tmp2, tmp2, tmp, tcg_const_i32(rn),
-                                    tcg_const_i32(n));
+                tmp4 = tcg_const_i32(rn);
+                tmp5 = tcg_const_i32(n);
+                gen_helper_neon_tbl(tmp2, tmp2, tmp, tmp4, tmp5);
                 dead_tmp(tmp);
                 if (insn & (1 << 6)) {
                     tmp = neon_load_reg(rd, 1);
@@ -5521,8 +5550,9 @@  static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
                     tcg_gen_movi_i32(tmp, 0);
                 }
                 tmp3 = neon_load_reg(rm, 1);
-                gen_helper_neon_tbl(tmp3, tmp3, tmp, tcg_const_i32(rn),
-                                    tcg_const_i32(n));
+                gen_helper_neon_tbl(tmp3, tmp3, tmp, tmp4, tmp5);
+                dead_tmp(tmp5);
+                dead_tmp(tmp4);
                 neon_store_reg(rd, 0, tmp2);
                 neon_store_reg(rd, 1, tmp3);
                 dead_tmp(tmp);
@@ -5685,6 +5715,7 @@  static void gen_addq_lo(DisasContext *s, TCGv_i64 val, int rlow)
     tcg_gen_extu_i32_i64(tmp, tmp2);
     dead_tmp(tmp2);
     tcg_gen_add_i64(val, val, tmp);
+    tcg_temp_free_i64(tmp);
 }
 
 /* load and add a 64-bit value from a register pair.  */
@@ -5702,6 +5733,7 @@  static void gen_addq(DisasContext *s, TCGv_i64 val, int rlow, int rhigh)
     dead_tmp(tmpl);
     dead_tmp(tmph);
     tcg_gen_add_i64(val, val, tmp);
+    tcg_temp_free_i64(tmp);
 }
 
 /* Set N and Z flags from a 64-bit value.  */
@@ -5785,7 +5817,9 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                 addr = load_reg(s, 13);
             } else {
                 addr = new_tmp();
-                gen_helper_get_r13_banked(addr, cpu_env, tcg_const_i32(op1));
+                tmp = tcg_const_i32(op1);
+                gen_helper_get_r13_banked(addr, cpu_env, tmp);
+                tcg_temp_free_i32(tmp);
             }
             i = (insn >> 23) & 3;
             switch (i) {
@@ -5816,7 +5850,9 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                 if (op1 == (env->uncached_cpsr & CPSR_M)) {
                     store_reg(s, 13, addr);
                 } else {
-                    gen_helper_set_r13_banked(cpu_env, tcg_const_i32(op1), addr);
+                    tmp = tcg_const_i32(op1);
+                    gen_helper_set_r13_banked(cpu_env, tmp, addr);
+                    tcg_temp_free_i32(tmp);
                     dead_tmp(addr);
                 }
             } else {
@@ -6058,6 +6094,7 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                 tcg_gen_shri_i64(tmp64, tmp64, 16);
                 tmp = new_tmp();
                 tcg_gen_trunc_i64_i32(tmp, tmp64);
+                tcg_temp_free_i64(tmp64);
                 if ((sh & 2) == 0) {
                     tmp2 = load_reg(s, rn);
                     gen_helper_add_setq(tmp, tmp, tmp2);
@@ -6076,6 +6113,7 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                     dead_tmp(tmp);
                     gen_addq(s, tmp64, rn, rd);
                     gen_storeq_reg(s, rn, rd, tmp64);
+                    tcg_temp_free_i64(tmp64);
                 } else {
                     if (op1 == 0) {
                         tmp2 = load_reg(s, rn);
@@ -6326,6 +6364,7 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                         if (insn & (1 << 20))
                             gen_logicq_cc(tmp64);
                         gen_storeq_reg(s, rn, rd, tmp64);
+                        tcg_temp_free_i64(tmp64);
                         break;
                     }
                 } else {
@@ -6545,10 +6584,12 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                         }
                         sh = (insn >> 16) & 0x1f;
                         if (sh != 0) {
+                            tmp2 = tcg_const_i32(sh);
                             if (insn & (1 << 22))
-                                gen_helper_usat(tmp, tmp, tcg_const_i32(sh));
+                                gen_helper_usat(tmp, tmp, tmp2);
                             else
-                                gen_helper_ssat(tmp, tmp, tcg_const_i32(sh));
+                                gen_helper_ssat(tmp, tmp, tmp2);
+                            tcg_temp_free_i32(tmp2);
                         }
                         store_reg(s, rd, tmp);
                     } else if ((insn & 0x00300fe0) == 0x00200f20) {
@@ -6556,10 +6597,12 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                         tmp = load_reg(s, rm);
                         sh = (insn >> 16) & 0x1f;
                         if (sh != 0) {
+                            tmp2 = tcg_const_i32(sh);
                             if (insn & (1 << 22))
-                                gen_helper_usat16(tmp, tmp, tcg_const_i32(sh));
+                                gen_helper_usat16(tmp, tmp, tmp2);
                             else
-                                gen_helper_ssat16(tmp, tmp, tcg_const_i32(sh));
+                                gen_helper_ssat16(tmp, tmp, tmp2);
+                            tcg_temp_free_i32(tmp2);
                         }
                         store_reg(s, rd, tmp);
                     } else if ((insn & 0x00700fe0) == 0x00000fa0) {
@@ -6631,6 +6674,7 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                         tcg_gen_shri_i64(tmp64, tmp64, 32);
                         tmp = new_tmp();
                         tcg_gen_trunc_i64_i32(tmp, tmp64);
+                        tcg_temp_free_i64(tmp64);
                         if (rd != 15) {
                             tmp2 = load_reg(s, rd);
                             if (insn & (1 << 6)) {
@@ -6659,6 +6703,7 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                             dead_tmp(tmp);
                             gen_addq(s, tmp64, rd, rn);
                             gen_storeq_reg(s, rd, rn, tmp64);
+                            tcg_temp_free_i64(tmp64);
                         } else {
                             /* smuad, smusd, smlad, smlsd */
                             if (rd != 15)
@@ -6831,7 +6876,9 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                             if (i == 15) {
                                 gen_bx(s, tmp);
                             } else if (user) {
-                                gen_helper_set_user_reg(tcg_const_i32(i), tmp);
+                                tmp2 = tcg_const_i32(i);
+                                gen_helper_set_user_reg(tmp2, tmp);
+                                tcg_temp_free_i32(tmp2);
                                 dead_tmp(tmp);
                             } else if (i == rn) {
                                 loaded_var = tmp;
@@ -6848,7 +6895,9 @@  static void disas_arm_insn(CPUState * env, DisasContext *s)
                                 tcg_gen_movi_i32(tmp, val);
                             } else if (user) {
                                 tmp = new_tmp();
-                                gen_helper_get_user_reg(tmp, tcg_const_i32(i));
+                                tmp2 = tcg_const_i32(i);
+                                gen_helper_get_user_reg(tmp, tmp2);
+                                tcg_temp_free_i32(tmp2);
                             } else {
                                 tmp = load_reg(s, i);
                             }
@@ -7264,7 +7313,9 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                         addr = load_reg(s, 13);
                     } else {
                         addr = new_tmp();
-                        gen_helper_get_r13_banked(addr, cpu_env, tcg_const_i32(op));
+                        tmp = tcg_const_i32(op);
+                        gen_helper_get_r13_banked(addr, cpu_env, tmp);
+                        tcg_temp_free_i32(tmp);
                     }
                     if ((insn & (1 << 24)) == 0) {
                         tcg_gen_addi_i32(addr, addr, -8);
@@ -7284,8 +7335,9 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                         if (op == (env->uncached_cpsr & CPSR_M)) {
                             store_reg(s, 13, addr);
                         } else {
-                            gen_helper_set_r13_banked(cpu_env,
-                                tcg_const_i32(op), addr);
+                            tmp = tcg_const_i32(op);
+                            gen_helper_set_r13_banked(cpu_env, tmp, addr);
+                            tcg_temp_free_i32(tmp);
                         }
                     } else {
                         dead_tmp(addr);
@@ -7515,6 +7567,7 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                 tcg_gen_shri_i64(tmp64, tmp64, 16);
                 tmp = new_tmp();
                 tcg_gen_trunc_i64_i32(tmp, tmp64);
+                tcg_temp_free_i64(tmp64);
                 if (rs != 15)
                   {
                     tmp2 = load_reg(s, rs);
@@ -7584,6 +7637,7 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                 dead_tmp(tmp);
                 gen_addq(s, tmp64, rs, rd);
                 gen_storeq_reg(s, rs, rd, tmp64);
+                tcg_temp_free_i64(tmp64);
             } else {
                 if (op & 0x20) {
                     /* Unsigned 64-bit multiply  */
@@ -7610,6 +7664,7 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                     gen_addq(s, tmp64, rs, rd);
                 }
                 gen_storeq_reg(s, rs, rd, tmp64);
+                tcg_temp_free_i64(tmp64);
             }
             break;
         }
@@ -7673,6 +7728,8 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                             tmp = load_reg(s, rn);
                             addr = tcg_const_i32(insn & 0xff);
                             gen_helper_v7m_msr(cpu_env, addr, tmp);
+                            tcg_temp_free_i32(addr);
+                            dead_tmp(tmp);
                             gen_lookup_tb(s);
                             break;
                         }
@@ -7742,6 +7799,7 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                         if (IS_M(env)) {
                             addr = tcg_const_i32(insn & 0xff);
                             gen_helper_v7m_mrs(tmp, cpu_env, addr);
+                            tcg_temp_free_i32(addr);
                         } else {
                             gen_helper_cpsr_read(tmp);
                         }
@@ -7842,6 +7900,7 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                             else
                                 gen_helper_ssat(tmp, tmp, tmp2);
                         }
+                        tcg_temp_free_i32(tmp2);
                         break;
                     }
                     store_reg(s, rd, tmp);
@@ -8614,12 +8673,15 @@  static void disas_thumb_insn(CPUState *env, DisasContext *s)
                 if (insn & 1) {
                     addr = tcg_const_i32(16);
                     gen_helper_v7m_msr(cpu_env, addr, tmp);
+                    tcg_temp_free_i32(addr);
                 }
                 /* FAULTMASK */
                 if (insn & 2) {
                     addr = tcg_const_i32(17);
                     gen_helper_v7m_msr(cpu_env, addr, tmp);
+                    tcg_temp_free_i32(addr);
                 }
+                tcg_temp_free_i32(tmp);
                 gen_lookup_tb(s);
             } else {
                 if (insn & (1 << 4))