[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 18/19] tcg-arm: Convert to CONFIG_QEMU_LDST_O
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH v5 18/19] tcg-arm: Convert to CONFIG_QEMU_LDST_OPTIMIZATION |
Date: |
Tue, 23 Apr 2013 10:18:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.16) Gecko/20121215 Iceowl/1.0b1 Icedove/3.0.11 |
Le 23/04/2013 10:13, Richard Henderson a écrit :
> On 2013-04-23 07:44, Aurelien Jarno wrote:
>> On Mon, Apr 22, 2013 at 03:39:42PM +0100, Richard Henderson wrote:
>>> On 2013-04-22 13:59, Aurelien Jarno wrote:
>>>>>> + /* The code buffer is limited to 16MB, with the prologue located
>>>>>> + at the end of it. Therefore we needn't care for any out of
>>>>>> + range branches. */
>>>>>> + assert(val - 8 < 0x01fffffd && val - 8 > -0x01fffffd);
>>>>>> +
>>>>>> + tcg_out_b(s, cond, val);
>>>> While this is currently true, I am not sure we want to get rid of that
>>>> code, and I hope we'll be able to eventually get rid of the 16MB limit.
>>>>
>>>> For me this dramatically reduce the boot time of guests. That said it is
>>>> not a real benchmark, and it should theoretically reduce the
>>>> performances in some cases as doing so interleaves code and data.
>>>> Someone has to spend time doing benchmarks before we can progress on that.
>>>>
>>>
>>> Ok, sure, but how do we progress in the short term?
>>> Certainly rejecting this patch and its changes to tcg_out_goto
>>> is not compatible with approving 19/19, which relies on it.
>>
>> Oh, I haven't seen realized that the comment about goto not using
>> multi-insns ops was referring to patch 18.
>>
>>> IMO, tcg_out_goto should be the simple case of goto within a TB,
>>> with only the code for INDEX_goto_tb needing to handle the >16MB case.
>>>
>>
>> >From what I have seen in the code, we need to do jumps in the following
>> conditions:
>> - jumps within TB
>> - jumps between slow/fast path in the ld/st code
>> - jump to the epilogue
>> - jump between TB
>>
>> Currently the first three use tcg_goto_out, while only the third one
>> need to (eventually) jump above the 16MB limit.
>
> The epilogue is exactly two insns. When we eliminate the 16MB limit
> we should just fold the add sp + pop into the exit_tb. We can't do
> better than that.
>
>> The last one should also jump above the 16MB limit, but do not use
>> tcg_goto_out.
>
> Agreed.
>
>> For jumps within TB tcg_out_goto_label can use tcg_out_b directly,
>> as tcg_out_b_noaddr is already used in the other pass, and the only
>> benefit of tcg_goto_out over tcg_out_b is the asserts, which can not
>> really been triggered here.
>>
>> For jumps from the slow path to the fast path, I think tcg_out_b
>> can also be used, as anyway tcg_out_b_noaddr is already used from the
>> fast path to the slow path.
>>
>> This leaves out tcg_goto_out used only for the jump to the epilogue, and
>> it can be modified later when someone works on the 16MB limit issue. Of
>> course that means that patch 19 has to be dropped, but I don't think
>> it's a big issue.
>
> Ok, I'll drop patch 19 for now so that we can make progress.
>
If you plan to respin the series, do you want I start applying the first
patches, at least the "Fix local stack frame" one that we want to see in
1.5 (and stable)?
--
Aurelien Jarno GPG: 1024D/F1BCDB73
address@hidden http://www.aurel32.net