[JIT/llvm-rtdyld] Don't waste cycles invalidating the Instruction cache

[davide]

Summary

We call InvalidateInstructionCache() twice here and I think the second call is just wasted work. Hopefully my guess is correct.

[lhames]

lib/Support/Unix/Memory.inc

266–282

Whatever we do here should be consistent for all platforms. I haven't taken a good look at the icache invalidation code - I'll have a think about this tomorrow.

[davide]

Lang, what are your thoughts about this?
Should we invalidate the cache unconditionally instead?

Thanks!

[davide]

Updated after our discussion on IRC. Thanks!

[lhames]

Hi Davide,

I spoke with Jim Grosbach about this and we agreed that 'setExecutable' should do everything required to make the memory executable on the host processor. That means unconditionally setting the permissions and invalidating the icache (on processors that need that). The problem is with clients who call InvalidateInstructionCache after calling setExecutable: that should be redundant. Clients should either call setExecutable alone, or call both protectMappedMemory and InvalidateInstructionCache manually.

So - we should remove the redundant calls, and modify setExecutable where necessary to properly set the permissions.

[lhames]

This comment was removed by lhames.

[lhames]

LGTM, pending the removal of the icache invalidation for the data sections.

tools/llvm-rtdyld/llvm-rtdyld.cpp

393

The instruction cache doesn't need to be invalidated for data sections, so this loop should be removed.

include/llvm/Support/Memory.h

public:
     /// InvalidateInstructionCache - Before the JIT can run a block of code
     /// that has been emitted it must invalidate the instruction cache on some
     /// platforms.
     static void InvalidateInstructionCache(const void *Addr, size_t Len);
 
     /// setExecutable - Before the JIT can run a block of code, it has to be
     /// given read and executable privilege. Return true if it is already r-x
     /// or the system is able to change its previlege.
-    static bool setExecutable(MemoryBlock &M, std::string *ErrMsg = nullptr);
+    /// If InvalidateCache is true, the instruction cache is also invalidated
+    //  as result of this call.
+    static bool setExecutable(MemoryBlock &M, std::string *ErrMsg = nullptr,
+                              bool InvalidateCache = false);
 
     /// setWritable - When adding to a block of code, the JIT may need
     /// to mark a block of code as RW since the protections are on page
     /// boundaries, and the JIT internal allocations are not page aligned.
     static bool setWritable(MemoryBlock &M, std::string *ErrMsg = nullptr);
 
     /// setRangeExecutable - Mark the page containing a range of addresses
     /// as executable.

lib/Support/Unix/Memory.inc

#if defined(__APPLE__) && (defined(__arm__) || defined(__arm64__))
   kern_return_t kr = vm_protect(mach_task_self(), (vm_address_t)M.Address,
     (vm_size_t)M.Size, 0, VM_PROT_READ | VM_PROT_WRITE);
   return KERN_SUCCESS == kr;
 #else
   return true;
 #endif
 }
 
-bool Memory::setExecutable (MemoryBlock &M, std::string *ErrMsg) {
+bool Memory::setExecutable(MemoryBlock &M, std::string *ErrMsg,
+                           bool InvalidateCache) {
 #if defined(__APPLE__) && (defined(__arm__) || defined(__arm64__))
   if (M.Address == 0 || M.Size == 0) return false;
   Memory::InvalidateInstructionCache(M.Address, M.Size);
   kern_return_t kr = vm_protect(mach_task_self(), (vm_address_t)M.Address,
     (vm_size_t)M.Size, 0, VM_PROT_READ | VM_PROT_EXECUTE | VM_PROT_COPY);
   return KERN_SUCCESS == kr;
 #elif defined(__arm__) || defined(__aarch64__)
   Memory::InvalidateInstructionCache(M.Address, M.Size);
   return true;
 #else
+  if (InvalidateCache)
+    Memory::InvalidateInstructionCache(M.Address, M.Size);
   return true;
 #endif
 }
 
 bool Memory::setRangeWritable(const void *Addr, size_t Size) {
 #if defined(__APPLE__) && (defined(__arm__) || defined(__arm64__))
   kern_return_t kr = vm_protect(mach_task_self(), (vm_address_t)Addr,
                                 (vm_size_t)Size, 0,
                                 VM_PROT_READ | VM_PROT_WRITE);
   return KERN_SUCCESS == kr;
 #else

lib/Support/Windows/Memory.inc

 
 bool Memory::setWritable(MemoryBlock &M, std::string *ErrMsg) {
   if (!setRangeWritable(M.Address, M.Size)) {
     return MakeErrMsg(ErrMsg, "Cannot set memory to writeable: ");
   }
   return true;
 }
 
-bool Memory::setExecutable(MemoryBlock &M, std::string *ErrMsg) {
+bool Memory::setExecutable(MemoryBlock &M, std::string *ErrMsg,
+                           bool InvalidateCache) {
   if (!setRangeExecutable(M.Address, M.Size)) {
     return MakeErrMsg(ErrMsg, "Cannot set memory to executable: ");
   }
   return true;
 }
 
 bool Memory::setRangeWritable(const void *Addr, size_t Size) {
   DWORD prot = getProtection(Addr);

tools/llvm-rtdyld/llvm-rtdyld.cpp

static int executeInput() {
   if (!MainAddress)
     return Error("no definition for '" + EntryPoint + "'");
 
   // Invalidate the instruction cache for each loaded function.
   for (unsigned i = 0, e = MemMgr.FunctionMemory.size(); i != e; ++i) {
     sys::MemoryBlock &Data = MemMgr.FunctionMemory[i];
     // Make sure the memory is executable.
     std::string ErrorStr;
-    sys::Memory::InvalidateInstructionCache(Data.base(), Data.size());
-    if (!sys::Memory::setExecutable(Data, &ErrorStr))
+    if (!sys::Memory::setExecutable(Data, &ErrorStr,
+                                    true /* invalidateCache */))
       return Error("unable to mark function executable: '" + ErrorStr + "'");
   }
 
   // Dispatch to _main().
   errs() << "loaded '" << EntryPoint << "' at: " << (void*)MainAddress << "\n";
 
   int (*Main)(int, const char**) =
     (int(*)(int,const char**)) uintptr_t(MainAddress);
   const char **Argv = new const char*[2];
   // Use the name of the first input object module as argv[0] for the target.
   Argv[0] = InputFileList[0].c_str();
   Argv[1] = nullptr;