[U-Boot] [PATCH] MIPS: fix mips_cache fallback without __builtin_mips_cache

[Daniel Schwierzeck]

Am 06.03.2016 um 20:53 schrieb Matthias Schiffer:
> On 03/06/2016 08:38 PM, Daniel Schwierzeck wrote:
>>
>>
>> Am 05.03.2016 um 04:15 schrieb Matthias Schiffer:
>>> The "R" constraint supplies the address of an variable in a register. Use
>>> "r" instead and adjust asm to supply the content of addr in a register
>>> instead.
>>>
>>> Fixes: 2b8bcc5a ("MIPS: avoid .set ISA for cache operations")
>>> Signed-off-by: Matthias Schiffer <mschiffer at universe-factory.net>
>>> Cc: Paul Burton <paul.burton at imgtec.com>
>>> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>>> ---
>>>
>>> Hi,
>>> I've noticed this when reading the code to understand how the cache
>>> instruction is used. I'm not sure if this bug had any practical
>>> consequences, or if nowadays all relevant compilers have
>>> __builtin_mips_cache anyways.
>>>
>>> Please keep me in Cc in follow-up mails, I'm not subscribed to the u-boot
>>> ML.
>>>
>>> Matthias
>>
>> I've disabled the builtin code and compared dissaemblies with and without your patch. Without your patch, gcc adds an additional store instruction before each cache instruction. 
>>
>> E.g. for flush_dcache_range():
>>
>>   18:	afa20008 	sw	v0,8(sp)
>>   1c:	bfb50008 	cache	0x15,8(sp)
>>
>> vs.
>>
>>   14:	bc550000 	cache	0x15,0(v0)
>>
>> The cache operation works anyway, but with your patch better code is generated.
> 
> If I understand this correctly, the code without my patch would rather
> invalidate the cache for the address 8(sp), which is part of the stack,
> and not the memory pointed at by v0.

"sw v0,8(sp)" stores the address in v0 on the stack (location is current stack pointer + 8 * 4 bytes). The cache op then loads this address from stack. Eventually the cache op always uses the address stored in v0 so there is/was no functional bug. But the extra copy to the stack causes a performance degradation.

-- 
- Daniel