change code to work with LLVM 3.9.1

7 files changed, 96 insertions, 73 deletions

diff --git a/eval/src/tests/eval/compiled_function/compiled_function_test.cpp b/eval/src/tests/eval/compiled_function/compiled_function_test.cpp
index 6a1b1f587e3..0eb5b409a3e 100644
--- a/eval/src/tests/eval/compiled_function/compiled_function_test.cpp
+++ b/eval/src/tests/eval/compiled_function/compiled_function_test.cpp
@@ -179,12 +179,15 @@ TEST("require that expressions with constant sub-expressions evaluate correctly"
 }
 
 TEST("dump ir code to verify lazy casting") {
-    CompiledFunction cf(Function::parse({"a", "b"}, "12==2+if(a==3&&a<10||b,10,5)"), PassParams::SEPARATE);
-    auto fun = cf.get_function<2>();
+    Function function = Function::parse({"a", "b"}, "12==2+if(a==3&&a<10||b,10,5)");
+    LLVMWrapper wrapper;
+    size_t id = wrapper.make_function(function.num_params(), PassParams::SEPARATE, function.root(), {});
+    wrapper.compile(true); // dump module before compiling it
+    using fun_type = double (*)(double, double);
+    fun_type fun = (fun_type) wrapper.get_function_address(id);
     EXPECT_EQUAL(0.0, fun(0.0, 0.0));
     EXPECT_EQUAL(1.0, fun(0.0, 1.0));
     EXPECT_EQUAL(1.0, fun(3.0, 0.0));
-    cf.dump();
 }
 
 TEST_MT("require that multithreaded compilation works", 64) {
diff --git a/eval/src/vespa/eval/CMakeLists.txt b/eval/src/vespa/eval/CMakeLists.txt
index c4edc1f380f..0dd5ad9e7c1 100644
--- a/eval/src/vespa/eval/CMakeLists.txt
+++ b/eval/src/vespa/eval/CMakeLists.txt
@@ -11,5 +11,5 @@ vespa_add_library(vespaeval
     $<TARGET_OBJECTS:eval_tensor_serialization>
     INSTALL lib64
     DEPENDS
-    LLVM-3.4
+    LLVM-3.9
 )
diff --git a/eval/src/vespa/eval/eval/llvm/compiled_function.cpp b/eval/src/vespa/eval/eval/llvm/compiled_function.cpp
index 472f794d136..9a5f81c9282 100644
--- a/eval/src/vespa/eval/eval/llvm/compiled_function.cpp
+++ b/eval/src/vespa/eval/eval/llvm/compiled_function.cpp
@@ -33,10 +33,12 @@ CompiledFunction::CompiledFunction(const Function &function_in, PassParams pass_
       _num_params(function_in.num_params()),
       _pass_params(pass_params_in)
 {
-    _address = _llvm_wrapper.compile_function(function_in.num_params(),
-                                              _pass_params,
-                                              function_in.root(),
-                                              forest_optimizers);
+    size_t id = _llvm_wrapper.make_function(function_in.num_params(),
+                                            _pass_params,
+                                            function_in.root(),
+                                            forest_optimizers);
+    _llvm_wrapper.compile();
+    _address = _llvm_wrapper.get_function_address(id);
 }
 
 CompiledFunction::CompiledFunction(CompiledFunction &&rhs)
diff --git a/eval/src/vespa/eval/eval/llvm/compiled_function.h b/eval/src/vespa/eval/eval/llvm/compiled_function.h
index c79629ca8a0..912453f290a 100644
--- a/eval/src/vespa/eval/eval/llvm/compiled_function.h
+++ b/eval/src/vespa/eval/eval/llvm/compiled_function.h
@@ -61,7 +61,6 @@ public:
     const std::vector<gbdt::Forest::UP> &get_forests() const {
         return _llvm_wrapper.get_forests();
     }
-    void dump() const { _llvm_wrapper.dump(); }
     double estimate_cost_us(const std::vector<double> &params, double budget = 5.0) const;
     static Function::Issues detect_issues(const Function &function);
 };
diff --git a/eval/src/vespa/eval/eval/llvm/deinline_forest.cpp b/eval/src/vespa/eval/eval/llvm/deinline_forest.cpp
index c976fb811c3..bcfb94634bc 100644
--- a/eval/src/vespa/eval/eval/llvm/deinline_forest.cpp
+++ b/eval/src/vespa/eval/eval/llvm/deinline_forest.cpp
@@ -10,6 +10,7 @@ namespace gbdt {
 DeinlineForest::DeinlineForest(const std::vector<const nodes::Node *> &trees)
 {
     size_t idx = 0;
+    size_t num_fragments = 0;
     while (idx < trees.size()) {
         size_t fragment_size = 0;
         std::vector<const nodes::Node *> fragment;
@@ -18,8 +19,13 @@ DeinlineForest::DeinlineForest(const std::vector<const nodes::Node *> &trees)
             fragment.push_back(trees[idx++]);
         }
         ForestStats stats(fragment);
-        void *address = _llvm_wrapper.compile_forest_fragment(stats.num_params, fragment);
-        _fragments.push_back((array_function)address);
+        size_t id = _llvm_wrapper.make_forest_fragment(stats.num_params, fragment);
+        assert(id == num_fragments);
+        ++num_fragments;
+    }
+    _llvm_wrapper.compile();
+    for (size_t id = 0; id < num_fragments; ++id) {
+        _fragments.push_back((array_function)_llvm_wrapper.get_function_address(id));
     }
 }
 
diff --git a/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp b/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp
index 021e850188d..5b607280403 100644
--- a/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp
+++ b/eval/src/vespa/eval/eval/llvm/llvm_wrapper.cpp
@@ -4,7 +4,8 @@
 #include "llvm_wrapper.h"
 #include <vespa/eval/eval/node_visitor.h>
 #include <vespa/eval/eval/node_traverser.h>
-#include <llvm/Analysis/Verifier.h>
+#include <llvm/IR/Verifier.h>
+#include <llvm/Support/TargetSelect.h>
 #include <llvm/IR/IRBuilder.h>
 #include <llvm/IR/Intrinsics.h>
 #include <llvm/ExecutionEngine/ExecutionEngine.h>
@@ -68,7 +69,6 @@ struct SetMemberHash : PluginState {
 
 struct FunctionBuilder : public NodeVisitor, public NodeTraverser {
 
-    llvm::ExecutionEngine    &engine;
     llvm::LLVMContext        &context;
     llvm::Module             &module;
     llvm::IRBuilder<>         builder;
@@ -119,8 +119,7 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser {
         return llvm::PointerType::get(function_type, 0);
     }
 
-    FunctionBuilder(llvm::ExecutionEngine &engine_in,
-                    llvm::LLVMContext &context_in,
+    FunctionBuilder(llvm::LLVMContext &context_in,
                     llvm::Module &module_in,
                     const vespalib::string &name_in,
                     size_t num_params_in,
@@ -128,8 +127,7 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser {
                     const gbdt::Optimize::Chain &forest_optimizers_in,
                     std::vector<gbdt::Forest::UP> &forests_out,
                     std::vector<PluginState::UP> &plugin_state_out)
-        : engine(engine_in),
-          context(context_in),
+        : context(context_in),
           module(module_in),
           builder(context),
           params(),
@@ -160,7 +158,7 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser {
         llvm::BasicBlock *block = llvm::BasicBlock::Create(context, "entry", function);
         builder.SetInsertPoint(block);
         for (llvm::Function::arg_iterator itr = function->arg_begin(); itr != function->arg_end(); ++itr) {
-            params.push_back(itr);
+            params.push_back(&(*itr));
         }
     }
     ~FunctionBuilder();
@@ -180,7 +178,7 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser {
         }
         assert(pass_params == PassParams::LAZY);
         assert(params.size() == 2);
-        return builder.CreateCall2(params[0], params[1], builder.getInt64(idx), "resolve_param");
+        return builder.CreateCall(params[0], {params[1], builder.getInt64(idx)}, "resolve_param");
     }
 
     //-------------------------------------------------------------------------
@@ -232,12 +230,12 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser {
         llvm::Value *eval_fun = builder.CreateIntToPtr(builder.getInt64((uint64_t)eval_ptr), eval_funptr_t, "inject_eval");
         llvm::Value *ctx = builder.CreateIntToPtr(builder.getInt64((uint64_t)forest), builder.getVoidTy()->getPointerTo(), "inject_ctx");
         if (pass_params == PassParams::ARRAY) {
-            push(builder.CreateCall2(eval_fun, ctx, params[0], "call_eval"));
+	    push(builder.CreateCall(eval_fun, {ctx, params[0]}, "call_eval"));
         } else {
             assert(pass_params == PassParams::LAZY);
             llvm::PointerType *proxy_funptr_t = make_eval_forest_proxy_funptr_t();
             llvm::Value *proxy_fun = builder.CreateIntToPtr(builder.getInt64((uint64_t)vespalib_eval_forest_proxy), proxy_funptr_t, "inject_eval_proxy");
-            push(builder.CreateCall5(proxy_fun, eval_fun, ctx, params[0], params[1], builder.getInt64(stats.num_params)));
+            push(builder.CreateCall(proxy_fun, {eval_fun, ctx, params[0], params[1], builder.getInt64(stats.num_params)}));
         }
         return true;
     }
@@ -292,11 +290,11 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser {
         inside_forest = false;
     }
 
-    void *compile() {
+    llvm::Function *build() {
         builder.CreateRet(pop_double());
         assert(values.empty());
         llvm::verifyFunction(*function);
-        return engine.getPointerToFunction(function);
+        return function;
     }
 
     //-------------------------------------------------------------------------
@@ -334,7 +332,7 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser {
         }
         llvm::Value *b = pop_double();
         llvm::Value *a = pop_double();
-        push(builder.CreateCall2(fun, a, b));
+        push(builder.CreateCall(fun, {a, b}));
     }
     void make_call_2(const llvm::Intrinsic::ID &id) {
         make_call_2(llvm::Intrinsic::getDeclaration(&module, id, builder.getDoubleTy()));
@@ -511,7 +509,7 @@ struct FunctionBuilder : public NodeVisitor, public NodeTraverser {
                 llvm::PointerType *funptr_t = make_check_membership_funptr_t();
                 llvm::Value *call_fun = builder.CreateIntToPtr(builder.getInt64((uint64_t)call_ptr), funptr_t, "inject_call_addr");
                 llvm::Value *ctx = builder.CreateIntToPtr(builder.getInt64((uint64_t)state), builder.getVoidTy()->getPointerTo(), "inject_ctx");
-                push(builder.CreateCall2(call_fun, ctx, lhs, "call_check_membership"));
+                push(builder.CreateCall(call_fun, {ctx, lhs}, "call_check_membership"));
             } else {
                 // build explicit code to check all set members
                 llvm::Value *found = builder.getFalse();
@@ -625,70 +623,84 @@ FunctionBuilder::~FunctionBuilder() { }
 
 struct InitializeNativeTarget {
     InitializeNativeTarget() {
-        LLVMInitializeNativeTarget();
+        llvm::InitializeNativeTarget();
+        llvm::InitializeNativeTargetAsmPrinter();
+        llvm::InitializeNativeTargetAsmParser();
     }
 } initialize_native_target;
 
 std::recursive_mutex LLVMWrapper::_global_llvm_lock;
 
 LLVMWrapper::LLVMWrapper()
-    : _context(nullptr),
-      _module(nullptr),
-      _engine(nullptr),
-      _num_functions(0),
+    : _context(),
+      _module(),
+      _engine(),
+      _functions(),
       _forests(),
       _plugin_state()
 {
     std::lock_guard<std::recursive_mutex> guard(_global_llvm_lock);
-    _context = new llvm::LLVMContext();
-    _module = new llvm::Module("LLVMWrapper", *_context);
-    _engine = llvm::EngineBuilder(_module).setOptLevel(llvm::CodeGenOpt::Aggressive).create();
-    assert(_engine != nullptr && "llvm jit not available for your platform");
+    _context = std::make_unique<llvm::LLVMContext>();
+    _module = std::make_unique< llvm::Module>("LLVMWrapper", *_context);
 }
 
-LLVMWrapper::LLVMWrapper(LLVMWrapper &&rhs)
-    : _context(rhs._context),
-      _module(rhs._module),
-      _engine(rhs._engine),
-      _num_functions(rhs._num_functions),
-      _forests(std::move(rhs._forests)),
-      _plugin_state(std::move(rhs._plugin_state))
-{
-    rhs._context = nullptr;
-    rhs._module = nullptr;
-    rhs._engine = nullptr;
-}
 
-void *
-LLVMWrapper::compile_function(size_t num_params, PassParams pass_params, const Node &root,
-                              const gbdt::Optimize::Chain &forest_optimizers)
-{
+size_t
+LLVMWrapper::make_function(size_t num_params, PassParams pass_params, const Node &root,
+                           const gbdt::Optimize::Chain &forest_optimizers)
+{ 
     std::lock_guard<std::recursive_mutex> guard(_global_llvm_lock);
-    FunctionBuilder builder(*_engine, *_context, *_module,
-                            vespalib::make_string("f%zu", ++_num_functions),
+    size_t function_id = _functions.size();
+    FunctionBuilder builder(*_context, *_module,
+                            vespalib::make_string("f%zu", function_id),
                             num_params, pass_params,
                             forest_optimizers, _forests, _plugin_state);
     builder.build_root(root);
-    return builder.compile();
+    _functions.push_back(builder.build());
+    return function_id;
 }
 
-void *
-LLVMWrapper::compile_forest_fragment(size_t num_params, const std::vector<const Node *> &fragment)
+size_t
+LLVMWrapper::make_forest_fragment(size_t num_params, const std::vector<const Node *> &fragment)
 {
     std::lock_guard<std::recursive_mutex> guard(_global_llvm_lock);
-    FunctionBuilder builder(*_engine, *_context, *_module,
-                            vespalib::make_string("f%zu", ++_num_functions),
+    size_t function_id = _functions.size();
+    FunctionBuilder builder(*_context, *_module,
+                            vespalib::make_string("f%zu", function_id),
                             num_params, PassParams::ARRAY,
                             gbdt::Optimize::none, _forests, _plugin_state);
     builder.build_forest_fragment(fragment);
-    return builder.compile();
+    _functions.push_back(builder.build());
+    return function_id;
+}
+
+void
+LLVMWrapper::compile(bool dump_module)
+{
+    std::lock_guard<std::recursive_mutex> guard(_global_llvm_lock);
+    if (dump_module) {
+        _module->dump();
+    }
+    _engine.reset(llvm::EngineBuilder(std::move(_module)).setOptLevel(llvm::CodeGenOpt::Aggressive).create());
+    assert(_engine && "llvm jit not available for your platform");
+    _engine->finalizeObject();
+}
+
+void *
+LLVMWrapper::get_function_address(size_t function_id)
+{
+    std::lock_guard<std::recursive_mutex> guard(_global_llvm_lock);
+    return _engine->getPointerToFunction(_functions[function_id]);
 }
 
 LLVMWrapper::~LLVMWrapper() {
     std::lock_guard<std::recursive_mutex> guard(_global_llvm_lock);
-    delete _engine;
-    // _module is owned by _engine
-    delete _context;
+    _plugin_state.clear();
+    _forests.clear();
+    _functions.clear();
+    _engine.reset();
+    _module.reset();
+    _context.reset();
 }
 
 } // namespace vespalib::eval
diff --git a/eval/src/vespa/eval/eval/llvm/llvm_wrapper.h b/eval/src/vespa/eval/eval/llvm/llvm_wrapper.h
index 95092692675..4f2a322dff1 100644
--- a/eval/src/vespa/eval/eval/llvm/llvm_wrapper.h
+++ b/eval/src/vespa/eval/eval/llvm/llvm_wrapper.h
@@ -8,7 +8,6 @@
 #include <llvm/IR/LLVMContext.h>
 #include <llvm/IR/Module.h>
 #include <llvm/ExecutionEngine/ExecutionEngine.h>
-#include <llvm/PassManager.h>
 #include <mutex>
 
 extern "C" {
@@ -43,23 +42,25 @@ struct PluginState {
 class LLVMWrapper
 {
 private:
-    llvm::LLVMContext         *_context;
-    llvm::Module              *_module; // owned by engine
-    llvm::ExecutionEngine     *_engine;
-    size_t                     _num_functions;
-    std::vector<gbdt::Forest::UP> _forests;
-    std::vector<PluginState::UP> _plugin_state;
+    std::unique_ptr<llvm::LLVMContext>     _context;
+    std::unique_ptr<llvm::Module>          _module;
+    std::unique_ptr<llvm::ExecutionEngine> _engine;
+    std::vector<llvm::Function*>           _functions;
+    std::vector<gbdt::Forest::UP>          _forests;
+    std::vector<PluginState::UP>           _plugin_state;
 
     static std::recursive_mutex _global_llvm_lock;
 
 public:
     LLVMWrapper();
-    LLVMWrapper(LLVMWrapper &&rhs);
-    void *compile_function(size_t num_params, PassParams pass_params, const nodes::Node &root,
-                           const gbdt::Optimize::Chain &forest_optimizers);
-    void *compile_forest_fragment(size_t num_params, const std::vector<const nodes::Node *> &fragment);
+    LLVMWrapper(LLVMWrapper &&rhs) = default;
+
+    size_t make_function(size_t num_params, PassParams pass_params, const nodes::Node &root,
+                         const gbdt::Optimize::Chain &forest_optimizers);
+    size_t make_forest_fragment(size_t num_params, const std::vector<const nodes::Node *> &fragment);
     const std::vector<gbdt::Forest::UP> &get_forests() const { return _forests; }
-    void dump() const { _module->dump(); }
+    void compile(bool dump_module = false);
+    void *get_function_address(size_t function_id);
     ~LLVMWrapper();
 };