ARROW-3469: [Gandiva] Add gandiva travis OSX entry

Author: Pindikura Ravindra <ravindra@dremio.com> Closes #2813 from pravindra/osx-travis1 and squashes the following commits: aac8248 <Pindikura Ravindra> ARROW-3469: remove submodule init in gdv cfc60be <Pindikura Ravindra> ARROW-3469: remove mutable refs in engine 1929e95 <Pindikura Ravindra> ARROW-3469: fix order in conda toolchain ddea118 <Pindikura Ravindra> ARROW-3469: Add gandiva travis OSX entry
apache · Oct 24, 2018 · fc2b3b5 · fc2b3b5
1 parent d904106
commit fc2b3b5
Show file tree

Hide file tree

Showing 22 changed files with 332 additions and 293 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -111,8 +111,6 @@ matrix:
     - $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
     - $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
     - $TRAVIS_BUILD_DIR/ci/travis_lint.sh
-    # If either C++ or Python changed, we must install the C++ libraries
-    - git submodule update --init
     - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh --only-library
     script:
     - $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_cpp.sh
@@ -139,6 +137,25 @@ matrix:
     - if [ $ARROW_CI_CPP_AFFECTED == "1" ]; then $TRAVIS_BUILD_DIR/ci/travis_script_cpp.sh; fi
     - $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 2.7
     - $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 3.6
+  # [OS X] Gandiva C++ w/ XCode 8.3 & Java
+  - compiler: clang
+    language: cpp
+    # xcode 7.3 has a bug in strptime.
+    osx_image: xcode8.3
+    os: osx
+    cache:
+    addons:
+    env:
+    - ARROW_TRAVIS_GANDIVA=1
+    - ARROW_TRAVIS_USE_TOOLCHAIN=1
+    - ARROW_BUILD_WARNING_LEVEL=CHECKIN
+    before_script:
+    # Run if something changed in CPP or Java.
+    - if [ $ARROW_CI_CPP_AFFECTED != "1" ] && [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
+    - $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh --only-library
+    script:
+    - $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_cpp.sh
+    - $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_java.sh
   # [manylinux1] Python
   - language: cpp
     before_script:

diff --git a/ci/travis_install_toolchain.sh b/ci/travis_install_toolchain.sh
@@ -22,25 +22,32 @@ source $TRAVIS_BUILD_DIR/ci/travis_env_common.sh
 source $TRAVIS_BUILD_DIR/ci/travis_install_conda.sh
 
 if [ ! -e $CPP_TOOLCHAIN ]; then
+    if [ $ARROW_TRAVIS_GANDIVA == "1" ] && [ $TRAVIS_OS_NAME == "osx" ]; then
+        CONDA_LLVM="llvmdev=6.0.1"
+    fi
+
     # Set up C++ toolchain from conda-forge packages for faster builds
-    conda create -y -q -p $CPP_TOOLCHAIN python=3.6 nomkl \
+    conda create -y -q -p $CPP_TOOLCHAIN \
         boost-cpp \
         brotli \
         bzip2 \
+        ${CONDA_LLVM} \
         ccache \
         cmake \
         curl \
         flatbuffers \
-        lz4-c \
         gflags \
+        glog \
         gtest \
         libprotobuf \
+        lz4-c \
         ninja \
+        nomkl \
+        python=3.6 \
         rapidjson \
+        re2 \
         snappy \
         thrift-cpp=0.11.0 \
         zlib \
-        glog \
-        zstd \
-        re2
+        zstd
 fi
diff --git a/cpp/cmake_modules/FindLLVM.cmake b/cpp/cmake_modules/FindLLVM.cmake
@@ -30,9 +30,21 @@ message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")
 # Find the libraries that correspond to the LLVM components
 llvm_map_components_to_libnames(LLVM_LIBS core mcjit native ipo bitreader target linker analysis debuginfodwarf)
 
-set(CLANG_EXECUTABLE ${LLVM_TOOLS_BINARY_DIR}/clang CACHE STRING "clang")
-set(LINK_EXECUTABLE ${LLVM_TOOLS_BINARY_DIR}/llvm-link CACHE STRING "link")
-set(CLANG_FORMAT_EXECUTABLE ${LLVM_TOOLS_BINARY_DIR}/clang-format CACHE STRING "clang-format")
+find_program(CLANG_EXECUTABLE clang
+  HINTS ${LLVM_TOOLS_BINARY_DIR})
+if (CLANG_EXECUTABLE)
+  message(STATUS "Found clang ${CLANG_EXECUTABLE}")
+else ()
+  message(FATAL_ERROR "Couldn't find clang")
+endif ()
+
+find_program(LLVM_LINK_EXECUTABLE llvm-link
+  HINTS ${LLVM_TOOLS_BINARY_DIR})
+if (LLVM_LINK_EXECUTABLE)
+  message(STATUS "Found llvm-link ${LLVM_LINK_EXECUTABLE}")
+else ()
+  message(FATAL_ERROR "Couldn't find llvm-link")
+endif ()
 
 add_library(LLVM::LLVM_INTERFACE INTERFACE IMPORTED)
 
@@ -41,3 +53,8 @@ set_target_properties(LLVM::LLVM_INTERFACE PROPERTIES
   INTERFACE_COMPILE_FLAGS "${LLVM_DEFINITIONS}"
   INTERFACE_LINK_LIBRARIES "${LLVM_LIBS}"
 )
+
+mark_as_advanced(
+  CLANG_EXECUTABLE
+  LLVM_LINK_EXECUTABLE
+)
diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake
@@ -113,15 +113,13 @@ if ("${UPPERCASE_BUILD_WARNING_LEVEL}" STREQUAL "CHECKIN")
     if ("${COMPILER_VERSION}" VERSION_GREATER "3.6")
       set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-reserved-id-macro")
       set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-range-loop-analysis")
-    endif()
-    if ("${COMPILER_VERSION}" VERSION_GREATER "3.7")
       set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-double-promotion")
     endif()
     if ("${COMPILER_VERSION}" VERSION_GREATER "3.8")
       set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-undefined-func-template")
     endif()
 
-    if ("${COMPILER_VERSION}" VERSION_GREATER "4.0")
+    if ("${COMPILER_VERSION}" VERSION_GREATER "3.9")
       set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-zero-as-null-pointer-constant")
     endif()
 

diff --git a/cpp/src/arrow/util/compression_snappy.h b/cpp/src/arrow/util/compression_snappy.h
@@ -38,7 +38,7 @@ class ARROW_EXPORT SnappyCodec : public Codec {
 
   int64_t MaxCompressedLen(int64_t input_len, const uint8_t* input) override;
 
-  Status MakeCompressor(std::shared_ptr<Compressor>* out);
+  Status MakeCompressor(std::shared_ptr<Compressor>* out) override;
 
   Status MakeDecompressor(std::shared_ptr<Decompressor>* out) override;
 

diff --git a/cpp/src/gandiva/context_helper.cc b/cpp/src/gandiva/context_helper.cc
@@ -25,16 +25,17 @@
 
 namespace gandiva {
 
-void ExportedContextFunctions::AddMappings(Engine& engine) const {
+void ExportedContextFunctions::AddMappings(Engine* engine) const {
   std::vector<llvm::Type*> args;
-  auto types = engine.types();
+  auto types = engine->types();
 
   // gdv_fn_context_set_error_msg
-  args = {types.i64_type(),      // int64_t context_ptr
-          types.i8_ptr_type()};  // char const* err_msg
+  args = {types->i64_type(),      // int64_t context_ptr
+          types->i8_ptr_type()};  // char const* err_msg
 
-  engine.AddGlobalMappingForFunc("gdv_fn_context_set_error_msg", types.void_type(), args,
-                                 reinterpret_cast<void*>(gdv_fn_context_set_error_msg));
+  engine->AddGlobalMappingForFunc("gdv_fn_context_set_error_msg", types->void_type(),
+                                  args,
+                                  reinterpret_cast<void*>(gdv_fn_context_set_error_msg));
 }
 
 }  // namespace gandiva

diff --git a/cpp/src/gandiva/engine.cc b/cpp/src/gandiva/engine.cc
@@ -221,7 +221,7 @@ void Engine::AddGlobalMappingForFunc(const std::string& name, llvm::Type* ret_ty
   execution_engine_->addGlobalMapping(fn, function_ptr);
 }
 
-void Engine::AddGlobalMappings() { ExportedFuncsRegistry::AddMappings(*this); }
+void Engine::AddGlobalMappings() { ExportedFuncsRegistry::AddMappings(this); }
 
 void Engine::DumpIR(std::string prefix) {
   std::string str;

diff --git a/cpp/src/gandiva/engine.h b/cpp/src/gandiva/engine.h
@@ -41,8 +41,8 @@ namespace gandiva {
 class Engine {
  public:
   llvm::LLVMContext* context() { return context_.get(); }
-  llvm::IRBuilder<>& ir_builder() { return *ir_builder_.get(); }
-
+  llvm::IRBuilder<>* ir_builder() { return ir_builder_.get(); }
+  LLVMTypes* types() { return types_.get(); }
   llvm::Module* module() { return module_; }
 
   /// factory method to create and initialize the engine object.
@@ -64,8 +64,6 @@ class Engine {
   /// Get the compiled function corresponding to the irfunction.
   void* CompiledFunction(llvm::Function* irFunction);
 
-  LLVMTypes& types() { return *types_; }
-
   // Create and add a mapping for the cpp function to make it accessible from LLVM.
   void AddGlobalMappingForFunc(const std::string& name, llvm::Type* ret_type,
                                const std::vector<llvm::Type*>& args, void* func);

diff --git a/cpp/src/gandiva/engine_llvm_test.cc b/cpp/src/gandiva/engine_llvm_test.cc
@@ -30,7 +30,7 @@ class TestEngine : public ::testing::Test {
 };
 
 llvm::Function* TestEngine::BuildVecAdd(Engine* engine, LLVMTypes* types) {
-  llvm::IRBuilder<>& builder = engine->ir_builder();
+  llvm::IRBuilder<>* builder = engine->ir_builder();
   llvm::LLVMContext* context = engine->context();
 
   // Create fn prototype :
@@ -62,39 +62,39 @@ llvm::Function* TestEngine::BuildVecAdd(Engine* engine, LLVMTypes* types) {
   llvm::BasicBlock* loop_exit = llvm::BasicBlock::Create(*context, "exit", fn);
 
   // Loop entry
-  builder.SetInsertPoint(loop_entry);
-  builder.CreateBr(loop_body);
+  builder->SetInsertPoint(loop_entry);
+  builder->CreateBr(loop_body);
 
   // Loop body
-  builder.SetInsertPoint(loop_body);
+  builder->SetInsertPoint(loop_body);
 
-  llvm::PHINode* loop_var = builder.CreatePHI(types->i32_type(), 2, "loop_var");
-  llvm::PHINode* sum = builder.CreatePHI(types->i64_type(), 2, "sum");
+  llvm::PHINode* loop_var = builder->CreatePHI(types->i32_type(), 2, "loop_var");
+  llvm::PHINode* sum = builder->CreatePHI(types->i64_type(), 2, "sum");
 
   loop_var->addIncoming(types->i32_constant(0), loop_entry);
   sum->addIncoming(types->i64_constant(0), loop_entry);
 
   // setup loop PHI
   llvm::Value* loop_update =
-      builder.CreateAdd(loop_var, types->i32_constant(1), "loop_var+1");
+      builder->CreateAdd(loop_var, types->i32_constant(1), "loop_var+1");
   loop_var->addIncoming(loop_update, loop_body);
 
   // get the current value
-  llvm::Value* offset = builder.CreateGEP(arg_elements, loop_var, "offset");
-  llvm::Value* current_value = builder.CreateLoad(offset, "value");
+  llvm::Value* offset = builder->CreateGEP(arg_elements, loop_var, "offset");
+  llvm::Value* current_value = builder->CreateLoad(offset, "value");
 
   // setup sum PHI
-  llvm::Value* sum_update = builder.CreateAdd(sum, current_value, "sum+ith");
+  llvm::Value* sum_update = builder->CreateAdd(sum, current_value, "sum+ith");
   sum->addIncoming(sum_update, loop_body);
 
   // check loop_var
   llvm::Value* loop_var_check =
-      builder.CreateICmpSLT(loop_update, arg_nelements, "loop_var < nrec");
-  builder.CreateCondBr(loop_var_check, loop_body, loop_exit);
+      builder->CreateICmpSLT(loop_update, arg_nelements, "loop_var < nrec");
+  builder->CreateCondBr(loop_var_check, loop_body, loop_exit);
 
   // Loop exit
-  builder.SetInsertPoint(loop_exit);
-  builder.CreateRet(sum_update);
+  builder->SetInsertPoint(loop_exit);
+  builder->CreateRet(sum_update);
   return fn;
 }
 

diff --git a/cpp/src/gandiva/exported_funcs.h b/cpp/src/gandiva/exported_funcs.h
@@ -28,18 +28,20 @@ class Engine;
 // Base-class type for exporting functions that can be accessed from LLVM/IR.
 class ExportedFuncsBase {
  public:
-  virtual void AddMappings(Engine& engine) const = 0;
+  virtual ~ExportedFuncsBase() = default;
+
+  virtual void AddMappings(Engine* engine) const = 0;
 };
 
 // Class for exporting Stub functions
 class ExportedStubFunctions : public ExportedFuncsBase {
-  void AddMappings(Engine& engine) const override;
+  void AddMappings(Engine* engine) const override;
 };
 REGISTER_EXPORTED_FUNCS(ExportedStubFunctions);
 
 // Class for exporting Context functions
 class ExportedContextFunctions : public ExportedFuncsBase {
-  void AddMappings(Engine& engine) const override;
+  void AddMappings(Engine* engine) const override;
 };
 REGISTER_EXPORTED_FUNCS(ExportedContextFunctions);
 

diff --git a/cpp/src/gandiva/exported_funcs_registry.cc b/cpp/src/gandiva/exported_funcs_registry.cc
@@ -21,7 +21,7 @@
 
 namespace gandiva {
 
-void ExportedFuncsRegistry::AddMappings(Engine& engine) {
+void ExportedFuncsRegistry::AddMappings(Engine* engine) {
   for (auto entry : registered()) {
     entry->AddMappings(engine);
   }

diff --git a/cpp/src/gandiva/exported_funcs_registry.h b/cpp/src/gandiva/exported_funcs_registry.h
@@ -33,7 +33,7 @@ class ExportedFuncsRegistry {
   using list_type = std::vector<ExportedFuncsBase*>;
 
   // Add functions from all the registered classes to the engine.
-  static void AddMappings(Engine& engine);
+  static void AddMappings(Engine* engine);
 
   static bool Register(ExportedFuncsBase* entry) {
     registered().push_back(entry);

diff --git a/cpp/src/gandiva/expr_validator.cc b/cpp/src/gandiva/expr_validator.cc
@@ -45,7 +45,7 @@ Status ExprValidator::Validate(const ExpressionPtr& expr) {
 }
 
 Status ExprValidator::Visit(const FieldNode& node) {
-  auto llvm_type = types_.IRType(node.return_type()->id());
+  auto llvm_type = types_->IRType(node.return_type()->id());
   if (llvm_type == nullptr) {
     std::stringstream ss;
     ss << "Field " << node.field()->name() << " has unsupported data type "
@@ -120,7 +120,7 @@ Status ExprValidator::Visit(const IfNode& node) {
 }
 
 Status ExprValidator::Visit(const LiteralNode& node) {
-  auto llvm_type = types_.IRType(node.return_type()->id());
+  auto llvm_type = types_->IRType(node.return_type()->id());
   if (llvm_type == nullptr) {
     std::stringstream ss;
     ss << "Value " << node.holder() << " has unsupported data type "

diff --git a/cpp/src/gandiva/expr_validator.h b/cpp/src/gandiva/expr_validator.h
@@ -38,7 +38,7 @@ class FunctionRegistry;
 /// data types, signatures and return types
 class ExprValidator : public NodeVisitor {
  public:
-  explicit ExprValidator(LLVMTypes& types, SchemaPtr schema)
+  explicit ExprValidator(LLVMTypes* types, SchemaPtr schema)
       : types_(types), schema_(schema) {
     for (auto& field : schema_->fields()) {
       field_map_[field->name()] = field;
@@ -62,7 +62,7 @@ class ExprValidator : public NodeVisitor {
 
   FunctionRegistry registry_;
 
-  LLVMTypes& types_;
+  LLVMTypes* types_;
 
   SchemaPtr schema_;
 

diff --git a/cpp/src/gandiva/filter.h b/cpp/src/gandiva/filter.h
@@ -46,9 +46,9 @@ class Filter {
 
   /// Build a filter for the given schema and condition, with the default configuration.
   ///
-  /// \param[in] : schema schema for the record batches, and the condition.
-  /// \param[in] : condition filter condition.
-  /// \param[out]: filter the returned filter object
+  /// \param[in] schema schema for the record batches, and the condition.
+  /// \param[in] condition filter condition.
+  /// \param[out] filter the returned filter object
   static Status Make(SchemaPtr schema, ConditionPtr condition,
                      std::shared_ptr<Filter>* filter) {
     return Make(schema, condition, ConfigurationBuilder::DefaultConfiguration(), filter);
@@ -57,19 +57,19 @@ class Filter {
   /// \brief Build a filter for the given schema and condition.
   /// Customize the filter with runtime configuration.
   ///
-  /// \param[in] : schema schema for the record batches, and the condition.
-  /// \param[in] : condition filter conditions.
-  /// \param[in] : config run time configuration.
-  /// \param[out]: filter the returned filter object
+  /// \param[in] schema schema for the record batches, and the condition.
+  /// \param[in] condition filter conditions.
+  /// \param[in] config run time configuration.
+  /// \param[out] filter the returned filter object
   static Status Make(SchemaPtr schema, ConditionPtr condition,
                      std::shared_ptr<Configuration> config,
                      std::shared_ptr<Filter>* filter);
 
   /// Evaluate the specified record batch, and populate output selection vector.
   ///
-  /// \param[in] : batch the record batch. schema should be the same as the one in 'Make'
-  /// \param[in/out]: out_selection the selection array with indices of rows that match
-  ///                 the condition.
+  /// \param[in] batch the record batch. schema should be the same as the one in 'Make'
+  /// \param[in,out] out_selection the selection array with indices of rows that match
+  ///                the condition.
   Status Evaluate(const arrow::RecordBatch& batch,
                   std::shared_ptr<SelectionVector> out_selection);