GH-45132: [C++][Gandiva] Update LLVM to 18.1 #45114

lriggs · 2024-12-27T23:14:03Z

Rationale for this change

#37848 upgraded the JIT compiler for LLVM/Gandiva code which presented linking errors with newer version of LLVM. Some Gandiva tests were disabled, and here at Dremio I am running into the same linking problem when trying to build with an updated Arrow library. After reading some threads on the LLVM discord server it appears that updating to LLVM 18.1 will fix the symbol issue. I tested locally and was able to re-enable the disabled java tests which were showing the unexported ORC symbol issue.

More discussion in apache/arrow-java#63.

What changes are included in this PR?

Updating vcpkg and pinning LLVM to 18.1 Notably I found encountered some build problems using the newest vcpkg update, which appeared to be related to the updated gRPC libraries. My Arrow jar CI build was timing out in this case with no clear error in the logs. The vcpkg version included here has the LLVM 18 update but not the gRPC update (which isn't needed for this issue).

Are these changes tested?

Covered by existing tests. Will also re-enable the disabled Java tests in a future change.

Are there any user-facing changes?

No.

GitHub Issue: [C++][Gandiva] LLVM JIT upgrade causing test and build failures #45132

github-actions · 2024-12-27T23:14:29Z

⚠️ GitHub issue #55 has been automatically assigned in GitHub to PR creator.

github-actions · 2024-12-27T23:14:30Z

⚠️ GitHub issue #43981 has no components, please add labels for components.

kou · 2024-12-28T21:36:51Z

Could you open a new issue for this?
GH-43981 doesn't exist in apache/arrow now.

github-actions · 2024-12-30T22:21:00Z

⚠️ GitHub issue #55 has been automatically assigned in GitHub to PR creator.

github-actions · 2024-12-30T22:21:01Z

⚠️ GitHub issue #43981 has no components, please add labels for components.

github-actions · 2024-12-31T01:18:39Z

⚠️ GitHub issue #45132 has been automatically assigned in GitHub to PR creator.

kou · 2024-12-31T02:29:15Z

We have AddAbsoluteSymbol() helper:

arrow/cpp/src/gandiva/engine.cc

Lines 159 to 177 in f5c340b

    
           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)); 
        
           }

Can we register llvm_orc_registerEHFrameSectionWrapper() and needed symbols manually instead?

lriggs · 2025-01-02T18:20:21Z

We have AddAbsoluteSymbol() helper:

arrow/cpp/src/gandiva/engine.cc

Lines 159 to 177 in f5c340b

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));

}

Can we register llvm_orc_registerEHFrameSectionWrapper() and needed symbols manually instead?

Good idea, I'll try it.

…mbol issu.

lriggs · 2025-01-24T23:48:12Z

I didn't have any luck trying to register the symbols since this error was happening when creating the llvm object, before registration could be done. My latest change updates to llvm 18.1 which seems to fix the symbol issue.

kou

Wow! LLVM 18.1 has a fix of this!?

BTW, can we update our default LLVM?

diff --git a/.env b/.env
index 0651d189c2..73644c0d1f 100644
--- a/.env
+++ b/.env
@@ -62,8 +62,7 @@ GCC=
 HDFS=3.2.1
 JDK=11
 KARTOTHEK=latest
-# LLVM 12 and GCC 11 reports -Wmismatched-new-delete.
-LLVM=14
+LLVM=18
 MAVEN=3.8.7
 NODE=18
 NUMBA=latest

ci/docker/java-jni-manylinux-201x.dockerfile

.env

docker-compose.yml

kou · 2025-01-25T02:59:49Z

@github-actions crossbow submit -g wheel

lriggs · 2025-02-01T01:12:05Z

@kou I think this is ready now. The CI failures don't seem related to my changes (and some of the passed earlier). Can you rerun those tests, or is there a way to know that they aren't related to my change?

kou · 2025-02-01T06:54:10Z

@github-actions crossbow submit -g wheel

github-actions · 2025-02-01T06:57:06Z

Revision: 55e0c81

Submitted crossbow builds: ursacomputing/crossbow @ actions-9d3e8e6b6f

Task	Status
python-sdist
wheel-macos-monterey-cp310-cp310-amd64
wheel-macos-monterey-cp310-cp310-arm64
wheel-macos-monterey-cp311-cp311-amd64
wheel-macos-monterey-cp311-cp311-arm64
wheel-macos-monterey-cp312-cp312-amd64
wheel-macos-monterey-cp312-cp312-arm64
wheel-macos-monterey-cp313-cp313-amd64
wheel-macos-monterey-cp313-cp313-arm64
wheel-macos-monterey-cp313-cp313t-amd64
wheel-macos-monterey-cp313-cp313t-arm64
wheel-macos-monterey-cp39-cp39-amd64
wheel-macos-monterey-cp39-cp39-arm64
wheel-manylinux-2-28-cp310-cp310-amd64
wheel-manylinux-2-28-cp310-cp310-arm64
wheel-manylinux-2-28-cp311-cp311-amd64
wheel-manylinux-2-28-cp311-cp311-arm64
wheel-manylinux-2-28-cp312-cp312-amd64
wheel-manylinux-2-28-cp312-cp312-arm64
wheel-manylinux-2-28-cp313-cp313-amd64
wheel-manylinux-2-28-cp313-cp313-arm64
wheel-manylinux-2-28-cp313-cp313t-amd64
wheel-manylinux-2-28-cp313-cp313t-arm64
wheel-manylinux-2-28-cp39-cp39-amd64
wheel-manylinux-2-28-cp39-cp39-arm64
wheel-manylinux-2014-cp310-cp310-amd64
wheel-manylinux-2014-cp310-cp310-arm64
wheel-manylinux-2014-cp311-cp311-amd64
wheel-manylinux-2014-cp311-cp311-arm64
wheel-manylinux-2014-cp312-cp312-amd64
wheel-manylinux-2014-cp312-cp312-arm64
wheel-manylinux-2014-cp313-cp313-amd64
wheel-manylinux-2014-cp313-cp313-arm64
wheel-manylinux-2014-cp313-cp313t-amd64
wheel-manylinux-2014-cp313-cp313t-arm64
wheel-manylinux-2014-cp39-cp39-amd64
wheel-manylinux-2014-cp39-cp39-arm64
wheel-windows-cp310-cp310-amd64
wheel-windows-cp311-cp311-amd64
wheel-windows-cp312-cp312-amd64
wheel-windows-cp313-cp313-amd64
wheel-windows-cp313-cp313t-amd64
wheel-windows-cp39-cp39-amd64

kou

+1

Thanks!

.env

conbench-apache-arrow · 2025-02-04T06:36:12Z

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 31747f0.

There were 8 benchmark results with an error:

Commit Run on amd64-m5-4xlarge-linux at 2025-02-04 00:58:54Z
- tpch (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-03, scale_factor=1
- tpch (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-03, scale_factor=10
and 6 more (see the report linked below)

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

Fixes GH-55. ### Rationale for this change I fixed the linking error here: apache/arrow#45114 ### What changes are included in this PR? Reenabling some disabled tests. ### Are these changes tested? Yes, they are tests. ### Are there any user-facing changes? No.

lriggs requested a review from wjones127 as a code owner December 27, 2024 23:14

github-actions bot added Component: C++ Component: Python Component: Gandiva awaiting review Awaiting review labels Dec 27, 2024

kou changed the title ~~GH-43981 [Gandiva] Revert LVVM JIT upgrade.~~ GH-45132: [C++][Gandiva] Revert LVVM JIT upgrade Dec 31, 2024

pitrou changed the title ~~GH-45132: [C++][Gandiva] Revert LVVM JIT upgrade~~ GH-45132: [C++][Gandiva] Revert LLVM JIT upgrade Jan 6, 2025

lriggs force-pushed the apache_main_lriggs branch from 150d723 to 3bb3cc0 Compare January 24, 2025 23:32

lriggs requested review from assignUser, jonkeane, kou and raulcd as code owners January 24, 2025 23:32

apacheGH-45132 Upgrade LLVM to 18.1 which fixes the unexported orc sy…

e77fae5

…mbol issu.

lriggs force-pushed the apache_main_lriggs branch from 3bb3cc0 to e77fae5 Compare January 24, 2025 23:43

github-actions bot removed Component: C++ Component: Python Component: Gandiva labels Jan 24, 2025

lriggs changed the title ~~GH-45132: [C++][Gandiva] Revert LLVM JIT upgrade~~ GH-45132: [C++][Gandiva] Update LLVM to 18.1 Jan 24, 2025

kou reviewed Jan 25, 2025

View reviewed changes

ci/docker/java-jni-manylinux-201x.dockerfile Outdated Show resolved Hide resolved

.env Outdated Show resolved Hide resolved

docker-compose.yml Outdated Show resolved Hide resolved

github-actions bot removed the awaiting review Awaiting review label Jan 25, 2025

github-actions bot added the Component: Documentation label Jan 30, 2025

lriggs force-pushed the apache_main_lriggs branch from 6b904e4 to 521a7be Compare January 30, 2025 18:10

github-actions bot removed Component: Parquet Component: C++ Component: Python Component: FlightRPC Component: GLib Component: Documentation labels Jan 30, 2025

Don't need llvm 18 here

55e0c81

kou approved these changes Feb 1, 2025

View reviewed changes

.env Show resolved Hide resolved

github-actions bot added awaiting review Awaiting review awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Feb 1, 2025

Update PYTHON_WHEEL_WINDOWS_IMAGE_REVISION

7f4efe5

github-actions bot added awaiting review Awaiting review and removed awaiting review Awaiting review awaiting merge Awaiting merge labels Feb 3, 2025

Merge branch 'main' into apache_main_lriggs

5044ccb

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Feb 3, 2025

kou merged commit 31747f0 into apache:main Feb 3, 2025
15 checks passed

kou removed the awaiting committer review Awaiting committer review label Feb 3, 2025

kou mentioned this pull request Feb 3, 2025

[C++][Gandiva] LLVM JIT upgrade causing test and build failures #45132

Closed

This was referenced Feb 8, 2025

GH-55: [Gandiva] Re-enable tests apache/arrow-java#595

Merged

[Java][Gandiva] Synchronize some methods on Projector. apache/arrow-java#601

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-45132: [C++][Gandiva] Update LLVM to 18.1 #45114

GH-45132: [C++][Gandiva] Update LLVM to 18.1 #45114

lriggs commented Dec 27, 2024 •

edited by kou

Loading

github-actions bot commented Dec 27, 2024

github-actions bot commented Dec 27, 2024

kou commented Dec 28, 2024

github-actions bot commented Dec 30, 2024

github-actions bot commented Dec 30, 2024

github-actions bot commented Dec 31, 2024

kou commented Dec 31, 2024

lriggs commented Jan 2, 2025

lriggs commented Jan 24, 2025

kou left a comment

kou commented Jan 25, 2025

lriggs commented Feb 1, 2025

kou commented Feb 1, 2025

github-actions bot commented Feb 1, 2025

kou left a comment

conbench-apache-arrow bot commented Feb 4, 2025

GH-45132: [C++][Gandiva] Update LLVM to 18.1 #45114

GH-45132: [C++][Gandiva] Update LLVM to 18.1 #45114

Conversation

lriggs commented Dec 27, 2024 • edited by kou Loading

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

github-actions bot commented Dec 27, 2024

github-actions bot commented Dec 27, 2024

kou commented Dec 28, 2024

github-actions bot commented Dec 30, 2024

github-actions bot commented Dec 30, 2024

github-actions bot commented Dec 31, 2024

kou commented Dec 31, 2024

lriggs commented Jan 2, 2025

lriggs commented Jan 24, 2025

kou left a comment

Choose a reason for hiding this comment

kou commented Jan 25, 2025

lriggs commented Feb 1, 2025

kou commented Feb 1, 2025

github-actions bot commented Feb 1, 2025

kou left a comment

Choose a reason for hiding this comment

conbench-apache-arrow bot commented Feb 4, 2025

lriggs commented Dec 27, 2024 •

edited by kou

Loading