[PATCH] [RFC] __builtin___clear_cache support in LLVM

[Renato Golin]

  Tim, that's a good point.

  I really didn't have a reason to move it down other than "I thought it was ugly for the front-end to define target-specific library invocations", and because other library call names are dealt with by the backends.

  When I got to implement, I realised that it wasn't much different than what a front-end would do, so now I'm not certain this is the best of the ideas. Since the call to __builtin___clear_cache will always translate to __clear_cache or nothing (AFAIK), we should be safe by doing it in the front-end, if people think it's the best way.

  Also, I'm not sure the user wants the compiler to second-guess their decisions to hammer the cache, as I believe these calls are not inserted lightly, but on execution errors after hours of debugging and internet searching. ;)

  cheers,
  --renato


================
Comment at: include/llvm/IR/Intrinsics.td:515
@@ +514,3 @@
+def int_clear_cache : Intrinsic<[], [llvm_ptr_ty, llvm_ptr_ty],
+                                [IntrReadWriteArgMem], "llvm.clear_cache">;
+
----------------
JF Bastien wrote:
> Maybe I'm being overly paranoid, but __clear_cache is sometimes used when something is mapped at multiple virtual addresses. IIUC having ReadWriteArgMem wouldn't be correct in these cases, even if the amount of memory accessed from the arguments is unspecified. I think specifying nothing would be better (equivalent to the non-existent ReadWriteMem?).
__clear_cache can use or not the arguments provided, it can invalidate different ranges of cache, read from memory and, who knows, even write to memory. Since the behaviour is completely unspecified, I tried to use the most conservative model, ie. "we break stuff". If specifying nothing would achieve that, I'm fine with it.

Most of the cases, though, nothing will be there to move around loads and stores, so that will only be relevant on platforms that do implement the cleaning.

================
Comment at: include/llvm/Target/TargetLowering.h:2114
@@ +2113,3 @@
+    return 0;
+  }
+
----------------
JF Bastien wrote:
> This seems like a pretty bad silent failure waiting to occur for targets other than x86/ARM/MIPS: implementor won't know that they have to add this. Making the function pure virtual would force everyone to return 0 when, like x86, there's nothing to do, and in all other cases they'd need to do the right thing for their platform. It's either break everyone's build loudly, or everyone's runtime silently.
> 
> It would also help understanding if the comment said that return 0 means "call nothing" (unless I misunderstood and that's not what it means).
good point, I'll change that to llvm_unreachable or something of the sort.

================
Comment at: test/CodeGen/X86/cache-intrinsic.ll:20
@@ +19,3 @@
+
+; CHECK-NOT: __clear_cache
+
----------------
JF Bastien wrote:
> Comment that x86 doesn't need to clear the cache? clear_cache.c says "Intel processors have a unified instruction and data cache so there is nothing to do."
will do, thanks!


http://llvm-reviews.chandlerc.com/D3084