[PATCH] [MIPS64] Make __clear_cache more optimal

[Petar Jovanovic]

================
Comment at: lib/builtins/clear_cache.c:27
@@ +26,3 @@
+     */
+    void clear_mips_cache(const void* Addr, size_t Size);
+      asm(
----------------
joerg wrote:
> petarj wrote:
> > joerg wrote:
> > > Missing static to not pollute global namespace?
> > Adding static will trigger a warning:
> > 
> > warning: function 'clear_mips_cache' has internal linkage but is not defined [-Wundefined-internal]
> __attribute__((__used__)) should disable the warning again?
No, it will not.

================
Comment at: lib/builtins/clear_cache.c:31
@@ +30,3 @@
+        ".align 2\n"
+        ".globl clear_mips_cache\n"
+        "clear_mips_cache:\n"
----------------
joerg wrote:
> petarj wrote:
> > joerg wrote:
> > > This should go away too. Why do you not use inline asm and let the compiler create the call and/or inline it? I'd prefer that...
> > This code needs to clear instruction hazards at its end, and using "jr.hb" is the most appropriate way to do so.
> > Making a version that could be inlined would, I believe, require a non-optimal trick to get pc value.
> > So, having this in a separate function is more optimal.
> > Last, the call to clear_mips_cache() will be accompanied with CALL16 relocations, and removing a .globl directive will trigger an error:
> > 
> > "CALL16 reloc at 0xAB not against global symbol"
> OK, so inline is not an option. That doesn't explain why it has to be global -- the warning above can be silenced, so what is the problem with declaring a file-local version?
> OK, so inline is not an option.
Inline is an option, but this seems to be more clear solution. I can create an inlineable version if necessary.

> That doesn't explain why it has to be global -- the warning above can be silenced,

Any other idea how?

> so what is the problem with declaring a file-local version?

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.

http://reviews.llvm.org/D6661

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/