kou commented on code in PR #39098:
URL: https://github.com/apache/arrow/pull/39098#discussion_r1433607844


##########
cpp/src/gandiva/engine.cc:
##########
@@ -103,9 +112,136 @@ extern const size_t kPrecompiledBitcodeSize;
 std::once_flag llvm_init_once_flag;
 static bool llvm_init = false;
 static llvm::StringRef cpu_name;
-static llvm::SmallVector<std::string, 10> cpu_attrs;
+static std::vector<std::string> cpu_attrs;
 std::once_flag register_exported_funcs_flag;
 
+template <typename T>
+arrow::Result<T> AsArrowResult(llvm::Expected<T>& expected,
+                               const std::string& error_context = "") {

Review Comment:
   Can we require `error_context`?
   
   ```suggestion
                                  const std::string& error_context ) {
   ```



##########
cpp/src/gandiva/engine.cc:
##########
@@ -103,9 +112,135 @@ extern const size_t kPrecompiledBitcodeSize;
 std::once_flag llvm_init_once_flag;
 static bool llvm_init = false;
 static llvm::StringRef cpu_name;
-static llvm::SmallVector<std::string, 10> cpu_attrs;
+static std::vector<std::string> cpu_attrs;
 std::once_flag register_exported_funcs_flag;
 
+template <typename T>
+arrow::Result<T> AsArrowResult(llvm::Expected<T>& expected,
+                               const std::string& error_context = "") {
+  if (!expected) {
+    return Status::CodeGenError(error_context, 
llvm::toString(expected.takeError()));
+  }
+  return std::move(expected.get());
+}
+
+Result<llvm::orc::JITTargetMachineBuilder> GetTargetMachineBuilder(
+    const Configuration& conf) {
+  llvm::orc::JITTargetMachineBuilder jtmb(
+      (llvm::Triple(llvm::sys::getDefaultTargetTriple())));
+  if (conf.target_host_cpu()) {
+    jtmb.setCPU(cpu_name.str());
+    jtmb.addFeatures(cpu_attrs);
+  }
+  auto const opt_level =
+      conf.optimize() ? llvm::CodeGenOpt::Aggressive : llvm::CodeGenOpt::None;
+  jtmb.setCodeGenOptLevel(opt_level);
+  return jtmb;
+}
+
+std::string GetModuleIR(const llvm::Module& module) {
+  std::string ir;
+  llvm::raw_string_ostream stream(ir);
+  module.print(stream, nullptr);
+  return ir;
+}
+
+void AddAbsoluteSymbol(llvm::orc::LLJIT& lljit, const std::string& name,
+                       void* function_ptr) {
+  llvm::orc::MangleAndInterner mangle(lljit.getExecutionSession(), 
lljit.getDataLayout());
+
+  // 
https://github.com/llvm/llvm-project/commit/8b1771bd9f304be39d4dcbdcccedb6d3bcd18200#diff-77984a824d9182e5c67a481740f3bc5da78d5bd4cf6e1716a083ddb30a4a4931
+  // LLVM 17 introduced ExecutorSymbolDef and move most of ORC APIs to 
ExecutorAddr
+#if LLVM_VERSION_MAJOR >= 17
+  llvm::orc::ExecutorSymbolDef symbol(
+      llvm::orc::ExecutorAddr(reinterpret_cast<uint64_t>(function_ptr)),
+      llvm::JITSymbolFlags::Exported);
+#else
+  llvm::JITEvaluatedSymbol 
symbol(reinterpret_cast<llvm::JITTargetAddress>(function_ptr),
+                                  llvm::JITSymbolFlags::Exported);
+#endif
+
+  auto error = lljit.getMainJITDylib().define(
+      llvm::orc::absoluteSymbols({{mangle(name), symbol}}));
+  llvm::cantFail(std::move(error));

Review Comment:
   @niyue It seems that this is not changed yet. You might forget to push your 
change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to