Patchwork target-arm: cleanup internal resource leaks

login
register
mail settings
Submitter Juha.Riihimaki@nokia.com
Date Oct. 22, 2009, 7:13 a.m.
Message ID <E76B4615-7F7C-44E6-A549-CC4C64317FF4@nokia.com>
Download mbox | patch
Permalink /patch/36652/
State New
Headers show

Comments

Juha.Riihimaki@nokia.com - Oct. 22, 2009, 7:13 a.m.
On Oct 22, 2009, at 08:40, Riihimaki Juha (Nokia-D/Helsinki) wrote:

>>> @@ -4676,12 +4694,16 @@ 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) {
>>> +                        dead_tmp(tmp2);
>>
>> This looks wrong if size == 3 since we have TCGV_UNUSED(tmp2).
>
> You're right. However, looking at the surrounding code a bit closer I
> began to wonder if it works correctly at all since tmp2 is used as a
> shift value if size < 2 but it is also used to store the result of the
> first pass. Therefore the result of the first pass is used as a shift
> value during second pass... perhaps not correct? Wouldn't it make more
> sense to store the result of the first pass in the lower half of the
> destination neon register directly during the first pass? I see no
> point in keeping it in a temporary variable and only store both halves
> of the destination neon register during the second pass. Especially
> since there is no memory access involved, everything is done in
> registers. What do you think?

To make it more clear what I'm after, look at the patch below that  
changes the code into what I think is correct functionality. The patch  
applies on top of the v2 resource leak patch which I just sent a short  
while ago.


Cheers,
Juha

---
              } else if (op == 10) {
                  /* VSHLL */

Patch

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 813f661..7598246 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4696,18 +4696,12 @@  static int disas_neon_data_insn(CPUState *  
env, DisasContext *s, uint32_t insn)
                          else
                              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);
-                    }
+                    neon_store_reg(rd, pass, tmp);
                  } /* for pass */
                  if (size == 3) {
                      tcg_temp_free_i64(tmp64);
+                } else {
+                    dead_tmp(tmp2);
                  }