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 vesion 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 reenable 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

kou · 2025-01-25T02:55:56Z

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

+ARG LLVM=18
+


It seems that we don't need this. (This isn't used.)

kou · 2025-01-25T02:56:07Z

.env

@@ -89,7 +89,7 @@ TZ=UTC
 # Used through docker-compose.yml and serves as the default version for the
 # ci/scripts/install_vcpkg.sh script. Prefer to use short SHAs to keep the
 # docker tags more readable.
-VCPKG="943c5ef1c8f6b5e6ced092b242c8299caae2ff01"    # 2024.04.26 Release
+VCPKG="f7423ee180c4b7f40d43402c2feb3859161ef625"    #  2024.06.15 Release


Suggested change

VCPKG="f7423ee180c4b7f40d43402c2feb3859161ef625" # 2024.06.15 Release

VCPKG="f7423ee180c4b7f40d43402c2feb3859161ef625" # 2024.06.15 Release

kou · 2025-01-25T02:57:01Z

docker-compose.yml

@@ -1338,6 +1338,7 @@ services:
    build:
      args:
        base: ${REPO}:python-wheel-windows-test-vs2019-base-${PYTHON_WHEEL_WINDOWS_TEST_IMAGE_REVISION}
+        llvm: 18


Do we need this?

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

@github-actions crossbow submit -g wheel

github-actions · 2025-01-25T03:02:27Z

Revision: e77fae5

Submitted crossbow builds: ursacomputing/crossbow @ actions-f3a96f0968

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

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

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

github-actions bot added the awaiting changes Awaiting changes label Jan 25, 2025

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

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 Jan 25, 2025

kou Jan 25, 2025

kou Jan 25, 2025

kou commented Jan 25, 2025

github-actions bot commented Jan 25, 2025

	VCPKG="f7423ee180c4b7f40d43402c2feb3859161ef625" # 2024.06.15 Release
	VCPKG="f7423ee180c4b7f40d43402c2feb3859161ef625" # 2024.06.15 Release

		ARG LLVM=18

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

Are you sure you want to change the base?

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

Conversation

lriggs commented Dec 27, 2024 • edited 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 Jan 25, 2025

Choose a reason for hiding this comment

kou Jan 25, 2025

Choose a reason for hiding this comment

kou Jan 25, 2025

Choose a reason for hiding this comment

kou commented Jan 25, 2025

github-actions bot commented Jan 25, 2025

lriggs commented Dec 27, 2024 •

edited

Loading