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 <mschif...@universe-factory.net>
>>> Cc: Paul Burton <paul.bur...@imgtec.com>
>>> Cc: Daniel Schwierzeck <daniel.schwierz...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
