[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2][RESEND] target-arm: cleanup internal resour
From: |
Laurent Desnogues |
Subject: |
Re: [Qemu-devel] [PATCH v2][RESEND] target-arm: cleanup internal resource leaks |
Date: |
Mon, 26 Oct 2009 11:46:24 +0100 |
On Thu, Oct 22, 2009 at 1:17 PM, <address@hidden> wrote:
> From: Juha Riihimäki <address@hidden>
>
> 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 <address@hidden>
> ---
> 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
>
>
>
>