On Tue, Oct 9, 2012 at 11:13 PM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 9 October 2012 21:30, Aurelien Jarno <aurel...@aurel32.net> wrote:
>> The TCG arm backend considers likely that the offset to the TLB
>> entries does not exceed 12 bits for mem_index = 0. In practice this is
>> not true for at list the MIPS target.
>>
>> The current patch fixes that by loading the bits 23-12 with a separate
>> instruction, and using loads with address writeback, independently of
>> the value of mem_idx. In total this allow a 24-bit offset, which is a
>> lot more than needed.
>
> Would probably be good to assert() that the bits above 24 are zero,
> though.
>
>> Cc: Andrzej Zaborowski <balr...@gmail.com>
>> Cc: Peter Maydell <peter.mayd...@linaro.org>
>> Cc: qemu-sta...@nongnu.org
>> Signed-off-by: Aurelien Jarno <aurel...@aurel32.net>
>> ---
>> tcg/arm/tcg-target.c | 73
>> +++++++++++++++++++++++++-------------------------
>> 1 file changed, 37 insertions(+), 36 deletions(-)
>>
>> diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
>> index 737200e..6cde512 100644
>> --- a/tcg/arm/tcg-target.c
>> +++ b/tcg/arm/tcg-target.c
>> @@ -624,6 +624,19 @@ static inline void tcg_out_ld32_12(TCGContext *s, int
>> cond,
>> (rn << 16) | (rd << 12) | ((-im) & 0xfff));
>> }
>>
>> +/* Offset pre-increment with base writeback. */
>> +static inline void tcg_out_ld32_12wb(TCGContext *s, int cond,
>> + int rd, int rn, tcg_target_long im)
>> +{
>
> Worth asserting that rd != rn ? (that's an UNPREDICTABLE case
> for ldr with writeback)
>
>> + if (im >= 0) {
>> + tcg_out32(s, (cond << 28) | 0x05b00000 |
>> + (rn << 16) | (rd << 12) | (im & 0xfff));
>> + } else {
>> + tcg_out32(s, (cond << 28) | 0x05300000 |
>> + (rn << 16) | (rd << 12) | ((-im) & 0xfff));
>> + }
>
> you could avoid the if() by just setting the U bit using "(im >= 0) << 23"
> Clearer? Dunno.
You also have to negate the im value so it's not enough to just
change the opcode.
Laurent
> Looks OK otherwise (though I haven't tested it.)
>
> Random aside: we emit a pretty long string of COND_EQ predicated
> code in these functions, especially in the 64 bit address and
> byteswapped cases. It might be interesting to see if performance
> on an A9, say, was any better if we just did a conditional branch
> over it instead... Probably borderline to no-effect, though.
>
> -- PMM
>
