kou commented on PR #37867:
URL: https://github.com/apache/arrow/pull/37867#issuecomment-1736261229
> I've compared the old/new LLVM IR generated by the particular test code
today, and they are exactly the same
Great!
> For such cases, do we need to use macro to call different APIs? Currently,
the lowest version of LLVM listed in the support list is `7.0`, we probably
won't be able to verify many different versions of LLVMs, one option is we use
two implementations for `PassManager`:
>
> 1. the legacy PassManager one for LLVM version < 14 (the old
implementation)
> 2. the new PassManager for LLVM version >= 14 (the implementation in
this PR)
Is it something like the following?
```diff
diff --git a/cpp/src/gandiva/engine.cc b/cpp/src/gandiva/engine.cc
index 6abe81405..b5104c5b7 100644
--- a/cpp/src/gandiva/engine.cc
+++ b/cpp/src/gandiva/engine.cc
@@ -308,6 +308,7 @@ Status Engine::FinalizeModule() {
ARROW_RETURN_NOT_OK(RemoveUnusedFunctions());
if (optimize_) {
+#if LLVM_VERSION_MAJOR >= 14
// Setup an optimiser pipeline
llvm::PassBuilder pass_builder;
llvm::LoopAnalysisManager loop_am;
@@ -345,6 +346,30 @@ Status Engine::FinalizeModule() {
pass_builder.buildPerModuleDefaultPipeline(llvm::OptimizationLevel::O3)
.run(*module_, module_am);
+#else
+ // misc passes to allow for inlining, vectorization, ..
+ std::unique_ptr<llvm::legacy::PassManager> pass_manager(
+ new llvm::legacy::PassManager());
+
+ llvm::TargetIRAnalysis target_analysis =
+ execution_engine_->getTargetMachine()->getTargetIRAnalysis();
+
pass_manager->add(llvm::createTargetTransformInfoWrapperPass(target_analysis));
+ pass_manager->add(llvm::createFunctionInliningPass());
+ pass_manager->add(llvm::createInstructionCombiningPass());
+ pass_manager->add(llvm::createPromoteMemoryToRegisterPass());
+ pass_manager->add(llvm::createGVNPass());
+ pass_manager->add(llvm::createNewGVNPass());
+ pass_manager->add(llvm::createCFGSimplificationPass());
+ pass_manager->add(llvm::createLoopVectorizePass());
+ pass_manager->add(llvm::createSLPVectorizerPass());
+ pass_manager->add(llvm::createGlobalOptimizerPass());
+
+ // run the optimiser
+ llvm::PassManagerBuilder pass_builder;
+ pass_builder.OptLevel = 3;
+ pass_builder.populateModulePassManager(*pass_manager);
+ pass_manager->run(*module_);
+#endif
}
ARROW_RETURN_IF(llvm::verifyModule(*module_, &llvm::errs()),
```
I like the approach.
> # linker error
>
> This CI check
(https://github.com/apache/arrow/actions/runs/6307346377/job/17123834680?pr=37867)
failed with some linker error `undefined reference`, but it is not a compiler
error, do we have multiple version of LLVMs installed in the environment?
It uses static linking. I think that we need to add more components for the
new PassManager API at:
https://github.com/apache/arrow/blob/23c1bf9ff74b749c11385b38911d6aa8d85e180c/cpp/cmake_modules/FindLLVMAlt.cmake#L89-L98
If you're not familiar with CMake, I can do it. But I can't work on it in
this week...
--
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]
