Re: [Qemu-devel] [PATCH 01/12] target-s390x: Fix wrong argument in call of tcg_gen_shl_i64()

[Stefan Weil <weil@mail.berlios.de>]

Am 26.05.2011 00:15, schrieb Alexander Graf:
> On 25.05.2011, at 22:25, Stefan Weil wrote:
>
>> tcg_gen_shl_i64 needs an argument of type TCGv_i64.
>> Using tmp4 needs some additional changes.
>>
>> Signed-off-by: Stefan Weil<weil@mail.berlios.de>
>> ---
>> target-s390x/translate.c |    8 +++++---
>> 1 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/target-s390x/translate.c b/target-s390x/translate.c
>> index 8e71df3..3614516 100644
>> --- a/target-s390x/translate.c
>> +++ b/target-s390x/translate.c
>> @@ -2056,7 +2056,6 @@ do_mh:
>>             even for very long ones... */
>>          tmp = get_address(s, 0, b2, d2);
>>          tmp3 = tcg_const_i64(stm_len);
>> -        tmp4 = tcg_const_i64(32);
>>          for (i = r1;; i = (i + 1) % 16) {
>>              switch (op) {
>>              case 0x4:
>> @@ -2070,7 +2069,9 @@ do_mh:
>> #else
>>                  tmp2 = tcg_temp_new_i64();
>>                  tcg_gen_qemu_ld32u(tmp2, tmp, get_mem_index(s));
>> -                tcg_gen_shl_i64(tmp2, tmp2, 4);
>> +                tmp4 = tcg_const_i64(4);
>> +                tcg_gen_shl_i64(tmp2, tmp2, tmp4);
>> +                tcg_temp_free_i64(tmp4);
>>                  tcg_gen_ext32u_i64(regs[i], regs[i]);
>>                  tcg_gen_or_i64(regs[i], regs[i], tmp2);
>> #endif
>> @@ -2081,7 +2082,9 @@ do_mh:
>>                  break;
>>              case 0x26:
>>                  tmp2 = tcg_temp_new_i64();
>> +                tmp4 = tcg_const_i64(32);
> This moves the const inside the loop, which is exactly what I was trying to avoid here. The problem is that every new const generated here issues 1 additional tcg op, which really sums up when there's too many of them. I've had the buffer exceed here plenty of times.
>
> Alex

I noticed that, too. But adding a tmp5 and generating two const outside 
the loop
of whom only one or even none is used is also a bad solution.

What about moving the loop inside the switch statement (one loop for 
every case)?
This does not look nice but looks like the best solution here.

If you prefer a different solution, just omit this part of my patch series.

Grüße, Stefan