ARROW-7819: [C++][Gandiva] Add DumpIR to Filter/Projector object

The following patch exposes the generated IR as a method of the objects
for further inspection. This is a breaking change for the internal
method `FinalizeModule` which doesn't take the dump_ir and optimize
flags, it receives `debug` from Configuration now.

- Refactored Engine, notably removed dead code, organized init in a single
  function and simplified LLVMGenerator.
- Dumping IR should not write to stdout, but instead return it as a
  string in the `DumpIR` method.
- Refactored Types, fixing some bad methods type.
- Added the optimize field to `Configuration` class.
- Simplified some unit tests.

Author: fsaintjacques

cpp/src/gandiva/arrow.h

@@ -25,6 +25,7 @@
 #include <arrow/builder.h>
 #include <arrow/pretty_print.h>
 #include <arrow/record_batch.h>
+#include <arrow/result.h>
 #include <arrow/status.h>
 #include <arrow/type.h>
 
@@ -51,6 +52,9 @@ using ArrayDataVector = std::vector<ArrayDataPtr>;
 using Status = arrow::Status;
 using StatusCode = arrow::StatusCode;
 
+template <typename T>
+using Result = arrow::Result<T>;
+
 static inline bool is_decimal_128(DataTypePtr type) {
   if (type->id() == arrow::Type::DECIMAL) {
     auto decimal_type = arrow::internal::checked_cast<arrow::DecimalType*>(type.get());

cpp/src/gandiva/configuration.h

@@ -38,6 +38,12 @@ class GANDIVA_EXPORT Configuration {
   std::size_t Hash() const;
   bool operator==(const Configuration& other) const;
   bool operator!=(const Configuration& other) const;
+
+  bool optimize() const { return optimize_; }
+  void set_optimize(bool optimize) { optimize_ = optimize; }
+
+ private:
+  bool optimize_ = true;
 };
 
 /// \brief configuration builder for gandiva

cpp/src/gandiva/engine.cc

@@ -24,6 +24,8 @@
 #include "gandiva/engine.h"
 
 #include <iostream>
+#include <memory>
+#include <mutex>
 #include <sstream>
 #include <string>
 #include <unordered_set>
@@ -41,8 +43,11 @@
 #include <llvm/Analysis/Passes.h>
 #include <llvm/Analysis/TargetTransformInfo.h>
 #include <llvm/Bitcode/BitcodeReader.h>
+#include <llvm/ExecutionEngine/ExecutionEngine.h>
 #include <llvm/ExecutionEngine/MCJIT.h>
 #include <llvm/IR/DataLayout.h>
+#include <llvm/IR/IRBuilder.h>
+#include <llvm/IR/LLVMContext.h>
 #include <llvm/IR/LegacyPassManager.h>
 #include <llvm/IR/Verifier.h>
 #include <llvm/Linker/Linker.h>
@@ -61,72 +66,87 @@
 #pragma warning(pop)
 #endif
 
+#include "gandiva/configuration.h"
 #include "gandiva/decimal_ir.h"
 #include "gandiva/exported_funcs_registry.h"
 
+#include "arrow/util/make_unique.h"
+
 namespace gandiva {
 
 extern const unsigned char kPrecompiledBitcode[];
 extern const size_t kPrecompiledBitcodeSize;
 
-std::once_flag init_once_flag;
-
-bool Engine::init_once_done_ = false;
-std::set<std::string> Engine::loaded_libs_ = {};
-std::mutex Engine::mtx_;
+std::once_flag llvm_init_once_flag;
+static bool llvm_init = false;
 
-// One-time initializations.
 void Engine::InitOnce() {
-  DCHECK_EQ(init_once_done_, false);
+  DCHECK_EQ(llvm_init, false);
 
   llvm::InitializeNativeTarget();
   llvm::InitializeNativeTargetAsmPrinter();
   llvm::InitializeNativeTargetAsmParser();
   llvm::InitializeNativeTargetDisassembler();
-
   llvm::sys::DynamicLibrary::LoadLibraryPermanently(nullptr);
 
-  init_once_done_ = true;
+  llvm_init = true;
 }
 
-/// factory method to construct the engine.
-Status Engine::Make(std::shared_ptr<Configuration> config,
-                    std::unique_ptr<Engine>* engine) {
-  static auto host_cpu_name = llvm::sys::getHostCPUName();
-  std::unique_ptr<Engine> engine_obj(new Engine());
-
-  std::call_once(init_once_flag, [&engine_obj] { engine_obj->InitOnce(); });
-  engine_obj->context_.reset(new llvm::LLVMContext());
-  engine_obj->ir_builder_.reset(new llvm::IRBuilder<>(*(engine_obj->context())));
-  engine_obj->types_.reset(new LLVMTypes(*(engine_obj->context())));
-
-  // Create the execution engine
-  std::unique_ptr<llvm::Module> cg_module(
-      new llvm::Module("codegen", *(engine_obj->context())));
-  engine_obj->module_ = cg_module.get();
-
-  llvm::EngineBuilder engineBuilder(std::move(cg_module));
-  engineBuilder.setMCPU(host_cpu_name);
-  engineBuilder.setEngineKind(llvm::EngineKind::JIT);
-  engineBuilder.setOptLevel(llvm::CodeGenOpt::Aggressive);
-  engineBuilder.setErrorStr(&(engine_obj->llvm_error_));
-  engine_obj->execution_engine_.reset(engineBuilder.create());
-  if (engine_obj->execution_engine_ == NULL) {
-    engine_obj->module_ = NULL;
-    return Status::CodeGenError(engine_obj->llvm_error_);
-  }
-
+Engine::Engine(const std::shared_ptr<Configuration>& conf,
+               std::unique_ptr<llvm::LLVMContext> ctx,
+               std::unique_ptr<llvm::ExecutionEngine> engine, llvm::Module* module)
+    : context_(std::move(ctx)),
+      execution_engine_(std::move(engine)),
+      ir_builder_(arrow::internal::make_unique<llvm::IRBuilder<>>(*context_)),
+      module_(module),
+      types_(*context_),
+      optimize_(conf->optimize()) {}
+
+Status Engine::Init() {
   // Add mappings for functions that can be accessed from LLVM/IR module.
-  engine_obj->AddGlobalMappings();
+  AddGlobalMappings();
 
-  auto status = engine_obj->LoadPreCompiledIR();
-  ARROW_RETURN_NOT_OK(status);
+  ARROW_RETURN_NOT_OK(LoadPreCompiledIR());
+  ARROW_RETURN_NOT_OK(DecimalIR::AddFunctions(this));
 
-  // Add decimal functions
-  status = DecimalIR::AddFunctions(engine_obj.get());
-  ARROW_RETURN_NOT_OK(status);
+  return Status::OK();
+}
 
-  *engine = std::move(engine_obj);
+/// factory method to construct the engine.
+Status Engine::Make(const std::shared_ptr<Configuration>& conf,
+                    std::unique_ptr<Engine>* out) {
+  std::call_once(llvm_init_once_flag, InitOnce);
+
+  auto ctx = arrow::internal::make_unique<llvm::LLVMContext>();
+  auto module = arrow::internal::make_unique<llvm::Module>("codegen", *ctx);
+
+  // Capture before moving, ExceutionEngine does not allow retrieving the
+  // original Module.
+  auto module_ptr = module.get();
+
+  auto opt_level =
+      conf->optimize() ? llvm::CodeGenOpt::Aggressive : llvm::CodeGenOpt::None;
+  // Note that the lifetime of the error string is not captured by the
+  // ExecutionEngine but only for the lifetime of the builder. Found by
+  // inspecting LLVM sources.
+  std::string builder_error;
+  std::unique_ptr<llvm::ExecutionEngine> exec_engine{
+      llvm::EngineBuilder(std::move(module))
+          .setMCPU(llvm::sys::getHostCPUName())
+          .setEngineKind(llvm::EngineKind::JIT)
+          .setOptLevel(opt_level)
+          .setErrorStr(&builder_error)
+          .create()};
+
+  if (exec_engine == nullptr) {
+    return Status::CodeGenError("Could not instantiate llvm::ExecutionEngine: ",
+                                builder_error);
+  }
+
+  std::unique_ptr<Engine> engine{
+      new Engine(conf, std::move(ctx), std::move(exec_engine), module_ptr)};
+  ARROW_RETURN_NOT_OK(engine->Init());
+  *out = std::move(engine);
   return Status::OK();
 }
 
@@ -191,15 +211,10 @@ Status Engine::RemoveUnusedFunctions() {
 }
 
 // Optimise and compile the module.
-Status Engine::FinalizeModule(bool optimise_ir, bool dump_ir, std::string* final_ir) {
-  auto status = RemoveUnusedFunctions();
-  ARROW_RETURN_NOT_OK(status);
-
-  if (dump_ir) {
-    DumpIR("Before optimise");
-  }
+Status Engine::FinalizeModule() {
+  ARROW_RETURN_NOT_OK(RemoveUnusedFunctions());
 
-  if (optimise_ir) {
+  if (optimize_) {
     // misc passes to allow for inlining, vectorization, ..
     std::unique_ptr<llvm::legacy::PassManager> pass_manager(
         new llvm::legacy::PassManager());
@@ -222,15 +237,8 @@ Status Engine::FinalizeModule(bool optimise_ir, bool dump_ir, std::string* final
     pass_builder.OptLevel = 3;
     pass_builder.populateModulePassManager(*pass_manager);
     pass_manager->run(*module_);
-
-    if (dump_ir) {
-      DumpIR("After optimise");
-    }
-  }
-  if (final_ir != nullptr) {
-    llvm::raw_string_ostream stream(*final_ir);
-    module_->print(stream, nullptr);
   }
+
   ARROW_RETURN_IF(llvm::verifyModule(*module_, &llvm::errs()),
                   Status::CodeGenError("Module verification failed after optimizer"));
 
@@ -249,20 +257,20 @@ void* Engine::CompiledFunction(llvm::Function* irFunction) {
 void Engine::AddGlobalMappingForFunc(const std::string& name, llvm::Type* ret_type,
                                      const std::vector<llvm::Type*>& args,
                                      void* function_ptr) {
-  auto prototype = llvm::FunctionType::get(ret_type, args, false /*isVarArg*/);
-  auto fn = llvm::Function::Create(prototype, llvm::GlobalValue::ExternalLinkage, name,
-                                   module());
+  constexpr bool is_var_arg = false;
+  auto prototype = llvm::FunctionType::get(ret_type, args, is_var_arg);
+  constexpr auto linkage = llvm::GlobalValue::ExternalLinkage;
+  auto fn = llvm::Function::Create(prototype, linkage, name, module());
   execution_engine_->addGlobalMapping(fn, function_ptr);
 }
 
 void Engine::AddGlobalMappings() { ExportedFuncsRegistry::AddMappings(this); }
 
-void Engine::DumpIR(std::string prefix) {
-  std::string str;
-
-  llvm::raw_string_ostream stream(str);
+std::string Engine::DumpIR() {
+  std::string ir;
+  llvm::raw_string_ostream stream(ir);
   module_->print(stream, nullptr);
-  std::cout << "====" << prefix << "===" << str << "\n";
+  return ir;
 }
 
 }  // namespace gandiva

cpp/src/gandiva/engine.h

@@ -23,9 +23,9 @@
 #include <string>
 #include <vector>
 
-#include "arrow/status.h"
 #include "arrow/util/macros.h"
 
+#include "gandiva/arrow.h"
 #include "gandiva/configuration.h"
 #include "gandiva/llvm_includes.h"
 #include "gandiva/llvm_types.h"
@@ -34,21 +34,19 @@
 
 namespace gandiva {
 
-class FunctionIRBuilder;
-
 /// \brief LLVM Execution engine wrapper.
 class GANDIVA_EXPORT Engine {
  public:
   llvm::LLVMContext* context() { return context_.get(); }
   llvm::IRBuilder<>* ir_builder() { return ir_builder_.get(); }
-  LLVMTypes* types() { return types_.get(); }
+  LLVMTypes* types() { return &types_; }
   llvm::Module* module() { return module_; }
 
   /// Factory method to create and initialize the engine object.
   ///
   /// \param[in] config the engine configuration
   /// \param[out] engine the created engine
-  static Status Make(std::shared_ptr<Configuration> config,
+  static Status Make(const std::shared_ptr<Configuration>& config,
                      std::unique_ptr<Engine>* engine);
 
   /// Add the function to the list of IR functions that need to be compiled.
@@ -59,7 +57,7 @@ class GANDIVA_EXPORT Engine {
   }
 
   /// Optimise and compile the module.
-  Status FinalizeModule(bool optimise_ir, bool dump_ir, std::string* final_ir = NULLPTR);
+  Status FinalizeModule();
 
   /// Get the compiled function corresponding to the irfunction.
   void* CompiledFunction(llvm::Function* irFunction);
@@ -68,16 +66,20 @@ class GANDIVA_EXPORT Engine {
   void AddGlobalMappingForFunc(const std::string& name, llvm::Type* ret_type,
                                const std::vector<llvm::Type*>& args, void* func);
 
+  /// Return the generated IR for the module.
+  std::string DumpIR();
+
  private:
-  /// private constructor to ensure engine is created
-  /// only through the factory.
-  Engine() : module_finalized_(false) {}
+  Engine(const std::shared_ptr<Configuration>& conf,
+         std::unique_ptr<llvm::LLVMContext> ctx,
+         std::unique_ptr<llvm::ExecutionEngine> engine, llvm::Module* module);
+
+  // Post construction init. This _must_ be called after the constructor.
+  Status Init();
 
-  /// do one time inits.
   static void InitOnce();
-  static bool init_once_done_;
 
-  llvm::ExecutionEngine& execution_engine() { return *execution_engine_.get(); }
+  llvm::ExecutionEngine& execution_engine() { return *execution_engine_; }
 
   /// load pre-compiled IR modules from precompiled_bitcode.cc and merge them into
   /// the main module.
@@ -89,23 +91,16 @@ class GANDIVA_EXPORT Engine {
   // Remove unused functions to reduce compile time.
   Status RemoveUnusedFunctions();
 
-  /// dump the IR code to stdout with the prefix string.
-  void DumpIR(std::string prefix);
-
   std::unique_ptr<llvm::LLVMContext> context_;
   std::unique_ptr<llvm::ExecutionEngine> execution_engine_;
-  std::unique_ptr<LLVMTypes> types_;
   std::unique_ptr<llvm::IRBuilder<>> ir_builder_;
-  llvm::Module* module_;  // This is owned by the execution_engine_, so doesn't need to be
-                          // explicitly deleted.
+  llvm::Module* module_;
+  LLVMTypes types_;
 
   std::vector<std::string> functions_to_compile_;
 
-  bool module_finalized_;
-  std::string llvm_error_;
-
-  static std::set<std::string> loaded_libs_;
-  static std::mutex mtx_;
+  bool optimize_ = true;
+  bool module_finalized_ = false;
 };
 
 }  // namespace gandiva