[JIT] 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.

[lhames]

Looks good to me. Thanks Pierre.

Do you have commit access? Otherwise I can commit this for you.

[pasaulais]

Thanks for the review Lang. I don't have commit access yet, it would be great if you could commit the changes for me.

[lhames]

Hi Pierre,

My apologies - I just realized this doesn't have a unit-test. Would you be able to write one up?

If not, I'll try to get to it tomorrow, but I'm a bit swamped at the moment.

Cheers,
Lang.

[pasaulais]

Hi Lang,

My bad, here is a couple unit tests (for both the old function pointer use case and the new lambda / std::function one).

Cheers,
Pierre-Andre

[lhames]

Thanks Pierre-Andre! Committed in r241962.

[pasaulais]

Thanks Lang! I have noticed that this commit was reverted due to tests failing on ARM. I'll create another review with an updated patch.

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

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");
+  Function *Parent = 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) {
+    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");
+  Function *Parent = 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) {
+    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(), 2);
+  EXPECT_FALSE(std::find(I, E, "Foo1") == E);
+  EXPECT_FALSE(std::find(I, E, "Foo2") == E);
+}
+
 }