Re: [Qemu-devel] [PATCH v5 18/19] tcg-arm: Convert to CONFIG_QEMU_LDST_OPTIMIZATION

[Richard Henderson <rth@twiddle.net>]

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.


r~