[ExecutionEngine] Use std::function with InstallLazyFunctionCreator

[pasaulais]

Summary

ExecutionEngine::InstallLazyFunctionCreator takes a function pointer, which prevents passing lambdas that capture state or member functions. With this commit, InstallLazyFunctionCreator takes a std::function parameter which allows lambdas and member functions to be used.

The original patch failed on ARM due to an unexpected additional unresolved function ('__aeabi_unwind_cpp_pr0'):

• http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/4137/steps/ninja%20check%202/logs/FAIL%3A%20LLVM-Unit%3A%3AMCJITTest.lazy_function_creator_pointer
• http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-selfhost/builds/4137/steps/ninja%20check%202/logs/FAIL%3A%20LLVM-Unit%3A%3AMCJITTest.lazy_function_creator_lambda

This patch has been updated to resolve this issue. It also includes Lang's follow-up commits (r241962, r241974).

[lhames]

LGTM. Committed in r242213. Thanks Pierre-Andre.

include/llvm/ExecutionEngine/ExecutionEngine.h

 #include "llvm/Object/Binary.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 #include <map>
 #include <string>
 #include <vector>
+#include <functional>
 
 namespace llvm {
 
 struct GenericValue;
 class Constant;
 class DataLayout;
 class ExecutionEngine;
 class Function;
public:
   }
 
   /// \brief Erase an entry from the mapping table.
   ///
   /// \returns The address that \p ToUnmap was happed to.
   uint64_t RemoveMapping(StringRef Name);
 };
 
+using FunctionCreator = std::function<void *(const std::string &)>;
+
 /// \brief Abstract interface for implementation execution of LLVM modules,
 /// designed to support both interpreter and just-in-time (JIT) compiler
 /// implementations.
 class ExecutionEngine {
   /// The state object holding the global address mapping, which must be
   /// accessed synchronously.
   //
   // FIXME: There is no particular need the entire map needs to be
static ExecutionEngine *(*OrcMCJITReplacementCtor)(
                                 std::unique_ptr<TargetMachine> TM);
 
   static ExecutionEngine *(*InterpCtor)(std::unique_ptr<Module> M,
                                         std::string *ErrorStr);
 
   /// LazyFunctionCreator - If an unknown function is needed, this function
   /// pointer is invoked to create it.  If this returns null, the JIT will
   /// abort.
-  void *(*LazyFunctionCreator)(const std::string &);
+  FunctionCreator LazyFunctionCreator;
 
   /// getMangledName - Get mangled name.
   std::string getMangledName(const GlobalValue *GV);
 
 public:
   /// lock - This lock protects the ExecutionEngine and MCJIT classes. It must
   /// be held while changing the internal state of any of those classes.
   sys::Mutex lock;
public:
   }
   bool getVerifyModules() const {
     return VerifyModules;
   }
 
   /// InstallLazyFunctionCreator - If an unknown function is needed, the
   /// specified function pointer is invoked to create it.  If it returns null,
   /// the JIT will abort.
-  void InstallLazyFunctionCreator(void* (*P)(const std::string &)) {
-    LazyFunctionCreator = P;
+  void InstallLazyFunctionCreator(FunctionCreator C) {
+    LazyFunctionCreator = C;
   }
 
 protected:
   ExecutionEngine() {}
   explicit ExecutionEngine(std::unique_ptr<Module> M);
 
   void emitGlobals();
 

unittests/ExecutionEngine/MCJIT/MCJITTest.cpp

 //===- MCJITTest.cpp - Unit tests for the MCJIT ---------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This test suite verifies basic MCJIT functionality such as making function
 // calls, using global variables, and compiling multpile modules.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ExecutionEngine/MCJIT.h"
+#include "llvm/Support/DynamicLibrary.h"
 #include "MCJITTestBase.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 
 namespace {
 
 class MCJITTest : public testing::Test, public MCJITTestBase {
TEST_F(MCJITTest, multiple_decl_lookups) {
   createJIT(std::move(M));
   void *A = TheJIT->getPointerToFunction(Foo);
   void *B = TheJIT->getPointerToFunction(Foo);
 
   EXPECT_TRUE(A != 0) << "Failed lookup - test not correctly configured.";
   EXPECT_EQ(A, B) << "Repeat calls to getPointerToFunction fail.";
 }
 
+typedef void * (*FunctionHandlerPtr)(const std::string &str);
+
+TEST_F(MCJITTest, lazy_function_creator_pointer) {
+  SKIP_UNSUPPORTED_PLATFORM;
+  
+  Function *Foo = insertExternalReferenceToFunction<int32_t(void)>(M.get(),
+                                                                   "Foo");
+  startFunction<int32_t(void)>(M.get(), "Parent");
+  CallInst *Call = Builder.CreateCall(Foo, {});
+  Builder.CreateRet(Call);
+  
+  createJIT(std::move(M));
+  
+  // Set up the lazy function creator that records the name of the last
+  // unresolved external function found in the module. Using a function pointer
+  // prevents us from capturing local variables, which is why this is static.
+  static std::string UnresolvedExternal;
+  FunctionHandlerPtr UnresolvedHandler = [] (const std::string &str) {
+    // Try to resolve the function in the current process before marking it as
+    // unresolved. This solves an issue on ARM where '__aeabi_*' function names
+    // are passed to this handler.
+    void *symbol =
+        llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(str.c_str());
+    if (symbol) {
+      return symbol;
+    }
+    
+    UnresolvedExternal = str;
+    return (void *)(uintptr_t)-1;
+  };
+  TheJIT->InstallLazyFunctionCreator(UnresolvedHandler);
+  
+  // JIT the module.
+  TheJIT->finalizeObject();
+  
+  // Verify that our handler was called.
+  EXPECT_EQ(UnresolvedExternal, "Foo");
+}
+
+TEST_F(MCJITTest, lazy_function_creator_lambda) {
+  SKIP_UNSUPPORTED_PLATFORM;
+  
+  Function *Foo1 = insertExternalReferenceToFunction<int32_t(void)>(M.get(),
+                                                                   "Foo1");
+  Function *Foo2 = insertExternalReferenceToFunction<int32_t(void)>(M.get(),
+                                                                   "Foo2");
+  startFunction<int32_t(void)>(M.get(), "Parent");
+  CallInst *Call1 = Builder.CreateCall(Foo1, {});
+  CallInst *Call2 = Builder.CreateCall(Foo2, {});
+  Value *Result = Builder.CreateAdd(Call1, Call2);
+  Builder.CreateRet(Result);
+  
+  createJIT(std::move(M));
+  
+  // Set up the lazy function creator that records the name of unresolved
+  // external functions in the module.
+  std::vector<std::string> UnresolvedExternals;
+  auto UnresolvedHandler = [&UnresolvedExternals] (const std::string &str) {
+    // Try to resolve the function in the current process before marking it as
+    // unresolved. This solves an issue on ARM where '__aeabi_*' function names
+    // are passed to this handler.
+    void *symbol =
+        llvm::sys::DynamicLibrary::SearchForAddressOfSymbol(str.c_str());
+    if (symbol) {
+      return symbol;
+    }
+    UnresolvedExternals.push_back(str);
+    return (void *)(uintptr_t)-1;
+  };
+  TheJIT->InstallLazyFunctionCreator(UnresolvedHandler);
+  
+  // JIT the module.
+  TheJIT->finalizeObject();
+  
+  // Verify that our handler was called for each unresolved function.
+  auto I = UnresolvedExternals.begin(), E = UnresolvedExternals.end();
+  EXPECT_EQ(UnresolvedExternals.size(), static_cast<size_t>(2));
+  EXPECT_FALSE(std::find(I, E, "Foo1") == E);
+  EXPECT_FALSE(std::find(I, E, "Foo2") == E);
+}
+
 }