[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target/m68k: add a mechanism to automatically f
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH] target/m68k: add a mechanism to automatically free TCGv |
Date: |
Mon, 19 Mar 2018 10:07:12 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
Le 19/03/2018 à 09:39, Richard Henderson a écrit :
> On 03/19/2018 12:12 AM, Laurent Vivier wrote:
>> SRC_EA() and gen_extend() can return either a temporary
>> TCGv or a memory allocated one. Mark them when they are
>> allocated, and free them automatically at end of the
>> instruction translation.
>>
>> We want to free locally allocated TCGv to avoid
>> overflow in sequence like:
>>
>> 0xc00ae406: movel %fp@(-132),%fp@(-268)
>> 0xc00ae40c: movel %fp@(-128),%fp@(-264)
>> 0xc00ae412: movel %fp@(-20),%fp@(-212)
>> 0xc00ae418: movel %fp@(-16),%fp@(-208)
>> 0xc00ae41e: movel %fp@(-60),%fp@(-220)
>> 0xc00ae424: movel %fp@(-56),%fp@(-216)
>> 0xc00ae42a: movel %fp@(-124),%fp@(-252)
>> 0xc00ae430: movel %fp@(-120),%fp@(-248)
>> 0xc00ae436: movel %fp@(-12),%fp@(-260)
>> 0xc00ae43c: movel %fp@(-8),%fp@(-256)
>> 0xc00ae442: movel %fp@(-52),%fp@(-276)
>> 0xc00ae448: movel %fp@(-48),%fp@(-272)
>> ...
>>
>> That can fill a lot of TCGv entries in a sequence,
>> especially since 15fa08f845 ("tcg: Dynamically allocate TCGOps")
>> we have no limit to fill the TCGOps cache and we can fill
>> the entire TCG variables array and overflow it.
>>
>> Suggested-by: Richard Henderson <address@hidden>
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
>
> This is a good start. It's hard to see all of where else might have been
> missed; at least the call to gen_load in gen_lea_indexed.
>
> For next development cycle it would be good to convert the translator loop and
> enable TCGv leak detection.
I agree.
> That said, this looks good so far.
> Reviewed-by: Richard Henderson <address@hidden>
I'm going to update the patch by splitting it in two as Philippe asked
and mark the missing gen_load() in gen_lea_indexed().
Thanks,
Laurent