Use synci implementation of clear_cache for short address ranges.
For long address ranges, make a kernel call.
Details
Diff Detail
Event Timeline
lib/builtins/clear_cache.c | ||
---|---|---|
32 | Adding static will trigger a warning: warning: function 'clear_mips_cache' has internal linkage but is not defined [-Wundefined-internal] | |
36 | This code needs to clear instruction hazards at its end, and using "jr.hb" is the most appropriate way to do so. "CALL16 reloc at 0xAB not against global symbol" |
lib/builtins/clear_cache.c | ||
---|---|---|
32 | No, it will not. | |
36 |
Inline is an option, but this seems to be more clear solution. I can create an inlineable version if necessary.
Any other idea how?
If we are going deeper in this discussion, the main issue will be that the bitcode does not hold the information that the function linkage type is internal, it leaves default linkage type and that is global. MIPS relocations are different for local and global symbols, so when the backend translates 'call clear_cache()', it checks its linkage type and emits a relocation for the global symbol (R_MIPS_CALL16) instead of relocations for local symbols (R_MIPS_GOT16 + R_MIPS_LO16). This error is later reported by the linker. |
Leaking clear_mips_cache is still absolutely not appropriate. I have been busy before Christmas. I'll try to come up with a better solution over the next week.
What about using something like the following:
static void clear_mips_cache(const void* Addr, size_t Size) { __asm__("...\n" "jr.hb %0" :: r(&&after); after: ; }
and not creating a separate function in asm at all?
Can't comment on the correctness of the MIPS assembly, but this looks much better.
lib/builtins/clear_cache.c | ||
---|---|---|
59 | I take it the 12 is the offset of the instruction just after the nop? If yes, please comment. |
lib/builtins/clear_cache.c | ||
---|---|---|
59 | Done. |
I can't speak to the MIPS asm either, but I do have one question: isn't this code useful for non-Android MIPS as well?
@danalbert, it is useful, and it will be enabled in later changes.
It shall also be followed with additional conditionals, like - check whether '-msynci' is set before we use it, etc.
The need for this change comes from Android build, thus we are making sure we fix Android build here first.
Missing static to not pollute global namespace?