From 7a63d79bd6bfca0dfd4d186365540b62cb6391f0 Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Wed, 9 Sep 2026 22:05:00 +0800 Subject: [PATCH 1/6] add support for llvm 23.1 --- ci/conda_env_gandiva.txt | 2 +- cpp/CMakeLists.txt | 1 + cpp/src/arrow/compute/exec.cc | 10 ----- cpp/src/arrow/compute/function_internal.h | 2 +- .../arrow/compute/kernels/codegen_internal.h | 8 ++-- cpp/src/arrow/compute/kernels/vector_hash.cc | 2 +- cpp/src/arrow/scalar.cc | 10 ++--- cpp/src/arrow/scalar_test.cc | 36 ++++++++++++++- cpp/src/arrow/util/async_generator.h | 6 +-- cpp/src/arrow/util/bit_block_counter.h | 8 ++-- cpp/src/arrow/util/future.h | 2 +- cpp/src/gandiva/engine.cc | 44 +++++++++++++------ 12 files changed, 87 insertions(+), 44 deletions(-) diff --git a/ci/conda_env_gandiva.txt b/ci/conda_env_gandiva.txt index 7127dc905586..edc49b692b84 100644 --- a/ci/conda_env_gandiva.txt +++ b/ci/conda_env_gandiva.txt @@ -16,4 +16,4 @@ # under the License. clang -llvmdev<23 +llvmdev diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index fd5eaf77f23b..9c81360a9571 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -179,6 +179,7 @@ set(ARROW_DOC_DIR "${CMAKE_INSTALL_DOCDIR}") set(BUILD_SUPPORT_DIR "${CMAKE_SOURCE_DIR}/build-support") set(ARROW_LLVM_VERSIONS + "23.1" "22.1" "21.1" "20.1" diff --git a/cpp/src/arrow/compute/exec.cc b/cpp/src/arrow/compute/exec.cc index 411ff0bb0263..db86a75c53ff 100644 --- a/cpp/src/arrow/compute/exec.cc +++ b/cpp/src/arrow/compute/exec.cc @@ -1185,16 +1185,6 @@ class ScalarAggExecutor : public KernelExecutorImpl { const FunctionOptions* options_; }; -template -Result> MakeExecutor(ExecContext* ctx, - const Function* func, - const FunctionOptions* options) { - DCHECK_EQ(ExecutorType::function_kind, func->kind()); - auto typed_func = checked_cast(func); - return std::make_unique(ctx, typed_func, options); -} - } // namespace Status PropagateNulls(KernelContext* ctx, const ExecSpan& batch, ArrayData* output) { diff --git a/cpp/src/arrow/compute/function_internal.h b/cpp/src/arrow/compute/function_internal.h index d1f8d06de84d..53e4388d953a 100644 --- a/cpp/src/arrow/compute/function_internal.h +++ b/cpp/src/arrow/compute/function_internal.h @@ -535,7 +535,7 @@ static inline std::enable_if_t, Result> GenericFromScalar( } template -static enable_if_same::ArrowType, ListType, Result> +enable_if_same::ArrowType, ListType, Result> GenericFromScalar(const std::shared_ptr& value) { using ValueType = typename T::value_type; if (value->type->id() != Type::LIST) { diff --git a/cpp/src/arrow/compute/kernels/codegen_internal.h b/cpp/src/arrow/compute/kernels/codegen_internal.h index bfc529dc0a46..672583c2be4d 100644 --- a/cpp/src/arrow/compute/kernels/codegen_internal.h +++ b/cpp/src/arrow/compute/kernels/codegen_internal.h @@ -481,7 +481,7 @@ struct UnboxScalar { template requires std::is_void_v::T>> -static void VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_func, +void VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_func, NullFunc&& null_func) { VisitArraySpanInline( arr, @@ -494,7 +494,7 @@ static void VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_func, template requires std::is_same_v::T>, Status> -static Status VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_func, +Status VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_func, NullFunc&& null_func) { return VisitArraySpanInline( arr, @@ -507,7 +507,7 @@ static Status VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_fun // Like VisitArrayValuesInline, but for binary functions. template -static void VisitTwoArrayValuesInline(const ArraySpan& arr0, const ArraySpan& arr1, +void VisitTwoArrayValuesInline(const ArraySpan& arr0, const ArraySpan& arr1, VisitFunc&& valid_func, NullFunc&& null_func) { ArrayIterator arr0_it(arr0); ArrayIterator arr1_it(arr1); @@ -584,7 +584,7 @@ namespace applicator { // static Status Call(KernelContext*, const Scalar& arg0, const ArraySpan& arg1, // ExecResult* out) template -static Status SimpleBinary(KernelContext* ctx, const ExecSpan& batch, ExecResult* out) { +Status SimpleBinary(KernelContext* ctx, const ExecSpan& batch, ExecResult* out) { if (batch.length == 0) return Status::OK(); if (batch[0].is_array()) { diff --git a/cpp/src/arrow/compute/kernels/vector_hash.cc b/cpp/src/arrow/compute/kernels/vector_hash.cc index 90ec9e365c35..25c0a457f2ed 100644 --- a/cpp/src/arrow/compute/kernels/vector_hash.cc +++ b/cpp/src/arrow/compute/kernels/vector_hash.cc @@ -136,7 +136,7 @@ class ValueCountsAction final : ActionBase { } template - void ObserveNullNotFound(Index index) { + [[maybe_unused]] void ObserveNullNotFound(Index index) { ARROW_LOG(FATAL) << "ObserveNullNotFound without err_status should not be called"; } diff --git a/cpp/src/arrow/scalar.cc b/cpp/src/arrow/scalar.cc index 22cffae05cb1..49398c15d37d 100644 --- a/cpp/src/arrow/scalar.cc +++ b/cpp/src/arrow/scalar.cc @@ -1191,13 +1191,13 @@ constexpr int64_t kMillisecondsInDay = 86400000; // date to date template -enable_if_t::value, Result>> +enable_if_t::value, Result>> CastImpl(const Date32Scalar& from, std::shared_ptr to_type) { return std::make_shared(from.value * kMillisecondsInDay, std::move(to_type)); } template -enable_if_t::value, Result>> +enable_if_t::value, Result>> CastImpl(const Date64Scalar& from, std::shared_ptr to_type) { return std::make_shared( static_cast(from.value / kMillisecondsInDay), std::move(to_type)); @@ -1205,7 +1205,7 @@ CastImpl(const Date64Scalar& from, std::shared_ptr to_type) { // timestamp to date template -enable_if_t::value, Result>> +enable_if_t::value, Result>> CastImpl(const TimestampScalar& from, std::shared_ptr to_type) { ARROW_ASSIGN_OR_RAISE( auto millis, @@ -1214,7 +1214,7 @@ CastImpl(const TimestampScalar& from, std::shared_ptr to_type) { std::move(to_type)); } template -enable_if_t::value, Result>> +enable_if_t::value, Result>> CastImpl(const TimestampScalar& from, std::shared_ptr to_type) { ARROW_ASSIGN_OR_RAISE( auto millis, @@ -1225,7 +1225,7 @@ CastImpl(const TimestampScalar& from, std::shared_ptr to_type) { // date to timestamp template -enable_if_timestamp>> CastImpl( +enable_if_timestamp>> CastImpl( const DateScalar& from, std::shared_ptr to_type) { using ToScalar = typename TypeTraits::ScalarType; int64_t millis = from.value; diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index a08ea6aa96d2..1b57a7f01817 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -183,7 +183,8 @@ TEST(TestScalar, IdentityCast) { test_identity_cast_for_type(type); } for (auto& type : { - arrow::fixed_size_list(arrow::int32(), 20), arrow::list(arrow::int32()), + arrow::fixed_size_list(arrow::int32(), 20), + arrow::list(arrow::int32()), arrow::large_list(arrow::int32()), // TODO(GH-45430): CastTo for ListView is not implemented yet. // arrow::list_view(arrow::int32()), arrow::large_list_view(arrow::int32()) @@ -973,6 +974,39 @@ TEST(TestDateScalars, MakeScalar) { Date64Scalar(-188171LL * 24 * 60 * 60 * 1000)); } +TEST(TestDateScalars, CastTo) { + constexpr int64_t kMillisecondsInDay = 86400000; + + ASSERT_OK_AND_ASSIGN(auto casted_date64, Date32Scalar(2).CastTo(date64())); + EXPECT_EQ(*casted_date64, Date64Scalar(2 * kMillisecondsInDay)); + + ASSERT_OK_AND_ASSIGN(auto casted_date32, + Date64Scalar(2 * kMillisecondsInDay).CastTo(date32())); + EXPECT_EQ(*casted_date32, Date32Scalar(2)); + + const auto timestamp_type = timestamp(TimeUnit::SECOND); + + ASSERT_OK_AND_ASSIGN(auto timestamp_from_date32, + Date32Scalar(2).CastTo(timestamp_type)); + EXPECT_EQ(*timestamp_from_date32, TimestampScalar(2 * 24 * 60 * 60, timestamp_type)); + + ASSERT_OK_AND_ASSIGN(auto timestamp_from_date64, + Date64Scalar(2 * kMillisecondsInDay).CastTo(timestamp_type)); + EXPECT_EQ(*timestamp_from_date64, TimestampScalar(2 * 24 * 60 * 60, timestamp_type)); + + ASSERT_OK_AND_ASSIGN( + auto date64_from_timestamp, + TimestampScalar(2 * kMillisecondsInDay + 3, timestamp(TimeUnit::MILLI)) + .CastTo(date64())); + EXPECT_EQ(*date64_from_timestamp, Date64Scalar(2 * kMillisecondsInDay)); + + ASSERT_OK_AND_ASSIGN( + auto date32_from_timestamp, + TimestampScalar(2 * kMillisecondsInDay + 3, timestamp(TimeUnit::MILLI)) + .CastTo(date32())); + EXPECT_EQ(*date32_from_timestamp, Date32Scalar(2)); +} + TEST(TestTimeScalars, Basics) { auto type1 = time32(TimeUnit::MILLI); auto type2 = time32(TimeUnit::SECOND); diff --git a/cpp/src/arrow/util/async_generator.h b/cpp/src/arrow/util/async_generator.h index 63cb01a9fe49..701a96321740 100644 --- a/cpp/src/arrow/util/async_generator.h +++ b/cpp/src/arrow/util/async_generator.h @@ -1863,7 +1863,7 @@ constexpr int kDefaultBackgroundQRestart = 16; /// /// This generator will queue up to max_q blocks template -static Result> MakeBackgroundGenerator( +Result> MakeBackgroundGenerator( Iterator iterator, internal::Executor* io_executor, int max_q = kDefaultBackgroundMaxQ, int q_restart = kDefaultBackgroundQRestart) { if (max_q < q_restart) { @@ -1887,7 +1887,7 @@ static Result> MakeBackgroundGenerator( /// /// This generator does not queue template -static Result> MakeBlockingGenerator( +Result> MakeBlockingGenerator( std::shared_ptr> iterator) { return [it = std::move(iterator)]() mutable -> Future { return Future::MakeFinished(it->Next()); @@ -1895,7 +1895,7 @@ static Result> MakeBlockingGenerator( } template -static Result> MakeBlockingGenerator(Iterator iterator) { +Result> MakeBlockingGenerator(Iterator iterator) { return MakeBlockingGenerator(std::make_shared>(std::move(iterator))); } diff --git a/cpp/src/arrow/util/bit_block_counter.h b/cpp/src/arrow/util/bit_block_counter.h index 82651a9d3877..d52754c96722 100644 --- a/cpp/src/arrow/util/bit_block_counter.h +++ b/cpp/src/arrow/util/bit_block_counter.h @@ -425,7 +425,7 @@ class ARROW_EXPORT OptionalBinaryBitBlockCounter { // Functional-style bit block visitors. template -static Status VisitBitBlocks(const uint8_t* bitmap, int64_t offset, int64_t length, +Status VisitBitBlocks(const uint8_t* bitmap, int64_t offset, int64_t length, VisitNotNull&& visit_not_null, VisitNull&& visit_null) { internal::OptionalBitBlockCounter bit_counter(bitmap, offset, length); int64_t position = 0; @@ -453,7 +453,7 @@ static Status VisitBitBlocks(const uint8_t* bitmap, int64_t offset, int64_t leng } template -static void VisitBitBlocksVoid(const uint8_t* bitmap, int64_t offset, int64_t length, +void VisitBitBlocksVoid(const uint8_t* bitmap, int64_t offset, int64_t length, VisitNotNull&& visit_not_null, VisitNull&& visit_null) { internal::OptionalBitBlockCounter bit_counter(bitmap, offset, length); int64_t position = 0; @@ -480,7 +480,7 @@ static void VisitBitBlocksVoid(const uint8_t* bitmap, int64_t offset, int64_t le } template -static Status VisitTwoBitBlocks(const uint8_t* left_bitmap, int64_t left_offset, +Status VisitTwoBitBlocks(const uint8_t* left_bitmap, int64_t left_offset, const uint8_t* right_bitmap, int64_t right_offset, int64_t length, VisitNotNull&& visit_not_null, VisitNull&& visit_null) { @@ -524,7 +524,7 @@ static Status VisitTwoBitBlocks(const uint8_t* left_bitmap, int64_t left_offset, } template -static void VisitTwoBitBlocksVoid(const uint8_t* left_bitmap, int64_t left_offset, +void VisitTwoBitBlocksVoid(const uint8_t* left_bitmap, int64_t left_offset, const uint8_t* right_bitmap, int64_t right_offset, int64_t length, VisitNotNull&& visit_not_null, VisitNull&& visit_null) { diff --git a/cpp/src/arrow/util/future.h b/cpp/src/arrow/util/future.h index fa45427a9d3d..f02775df2a7f 100644 --- a/cpp/src/arrow/util/future.h +++ b/cpp/src/arrow/util/future.h @@ -702,7 +702,7 @@ class WeakFuture { /// If a Result holds an error instead of a Future, construct a finished Future /// holding that error. template -static Future DeferNotOk(Result> maybe_future) { +Future DeferNotOk(Result> maybe_future) { if (ARROW_PREDICT_FALSE(!maybe_future.ok())) { return Future::MakeFinished(std::move(maybe_future).status()); } diff --git a/cpp/src/gandiva/engine.cc b/cpp/src/gandiva/engine.cc index e6edb0042ce5..057254b5ef4c 100644 --- a/cpp/src/gandiva/engine.cc +++ b/cpp/src/gandiva/engine.cc @@ -131,6 +131,8 @@ template arrow::Result AsArrowResult(llvm::Expected& expected, const std::string& error_context) { if (!expected) { + // NOTE: llvm::handleAllErrors() fails linking with RTTI-disabled LLVM builds + // (ARROW-5148) return Status::CodeGenError(error_context, llvm::toString(expected.takeError())); } return std::move(expected.get()); @@ -207,7 +209,12 @@ Status UseJITLinkIfEnabled(llvm::orc::LLJITBuilder& jit_builder) { static auto maybe_use_jit_link = ::arrow::internal::GetEnvVar("GANDIVA_USE_JIT_LINK"); if (maybe_use_jit_link.ok()) { ARROW_ASSIGN_OR_RAISE(static auto memory_manager, CreateMemmoryManager()); -# if LLVM_VERSION_MAJOR >= 21 +# if LLVM_VERSION_MAJOR >= 23 + jit_builder.setObjectLinkingLayerCreator( + [](llvm::orc::ExecutionSession& ES, llvm::jitlink::JITLinkMemoryManager&) { + return std::make_unique(ES, *memory_manager); + }); +# elif LLVM_VERSION_MAJOR >= 21 jit_builder.setObjectLinkingLayerCreator([&](llvm::orc::ExecutionSession& ES) { return std::make_unique(ES, *memory_manager); }); @@ -261,13 +268,8 @@ Result> BuildJIT( return jit; } -arrow::Status VerifyAndLinkModule( - llvm::Module& dest_module, - llvm::Expected> src_module_or_error) { - ARROW_ASSIGN_OR_RAISE( - auto src_ir_module, - AsArrowResult(src_module_or_error, "Failed to verify and link module: ")); - +arrow::Status VerifyAndLinkModule(llvm::Module& dest_module, + std::unique_ptr src_ir_module) { src_ir_module->setDataLayout(dest_module.getDataLayout()); std::string error_info; @@ -282,6 +284,14 @@ arrow::Status VerifyAndLinkModule( return Status::OK(); } +void RemoveBuildTargetAttributes(llvm::Module& module) { + for (auto& function : module.functions()) { + function.removeFnAttr("target-cpu"); + function.removeFnAttr("target-features"); + function.removeFnAttr("tune-cpu"); + } +} + } // namespace Status Engine::SetLLVMObjectCache(GandivaObjectCache& object_cache) { @@ -422,10 +432,15 @@ Status Engine::LoadPreCompiledIR() { /// Parse the IR module. llvm::Expected> module_or_error = llvm::getOwningLazyBitcodeModule(std::move(buffer), *context()); - // NOTE: llvm::handleAllErrors() fails linking with RTTI-disabled LLVM builds - // (ARROW-5148) - ARROW_RETURN_NOT_OK(VerifyAndLinkModule(*module_, std::move(module_or_error))); - return Status::OK(); + ARROW_ASSIGN_OR_RAISE( + auto src_ir_module, + AsArrowResult(module_or_error, "Failed to verify and link module: ")); + + // Built-in bitcode is JIT-compiled on the runtime host. Do not retain the target + // selected by Clang when the bitcode was built. + RemoveBuildTargetAttributes(*src_ir_module); + + return VerifyAndLinkModule(*module_, std::move(src_ir_module)); } static llvm::MemoryBufferRef AsLLVMMemoryBuffer(const arrow::Buffer& arrow_buffer) { @@ -439,7 +454,10 @@ Status Engine::LoadExternalPreCompiledIR() { for (const auto& buffer : buffers) { auto llvm_memory_buffer_ref = AsLLVMMemoryBuffer(*buffer); auto module_or_error = llvm::parseBitcodeFile(llvm_memory_buffer_ref, *context()); - ARROW_RETURN_NOT_OK(VerifyAndLinkModule(*module_, std::move(module_or_error))); + ARROW_ASSIGN_OR_RAISE( + auto src_ir_module, + AsArrowResult(module_or_error, "Failed to verify and link module: ")); + ARROW_RETURN_NOT_OK(VerifyAndLinkModule(*module_, std::move(src_ir_module))); } return Status::OK(); From 658e96e9b97b681fd6350feda99d8cde565500ae Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Wed, 9 Sep 2026 22:46:49 +0800 Subject: [PATCH 2/6] fix cpplint ci and add a minor refactor --- cpp/src/arrow/compute/function_internal.h | 4 ++-- cpp/src/arrow/compute/kernels/codegen_internal.h | 6 +++--- cpp/src/arrow/scalar_test.cc | 3 +-- cpp/src/arrow/util/async_generator.h | 3 +-- cpp/src/arrow/util/bit_block_counter.h | 16 ++++++++-------- cpp/src/gandiva/engine.cc | 15 ++++++++++----- 6 files changed, 25 insertions(+), 22 deletions(-) diff --git a/cpp/src/arrow/compute/function_internal.h b/cpp/src/arrow/compute/function_internal.h index 53e4388d953a..92f8529ab787 100644 --- a/cpp/src/arrow/compute/function_internal.h +++ b/cpp/src/arrow/compute/function_internal.h @@ -535,8 +535,8 @@ static inline std::enable_if_t, Result> GenericFromScalar( } template -enable_if_same::ArrowType, ListType, Result> -GenericFromScalar(const std::shared_ptr& value) { +enable_if_same::ArrowType, ListType, Result> GenericFromScalar( + const std::shared_ptr& value) { using ValueType = typename T::value_type; if (value->type->id() != Type::LIST) { return Status::Invalid("Expected type LIST but got ", value->type->ToString()); diff --git a/cpp/src/arrow/compute/kernels/codegen_internal.h b/cpp/src/arrow/compute/kernels/codegen_internal.h index 672583c2be4d..3a2bcefab0a1 100644 --- a/cpp/src/arrow/compute/kernels/codegen_internal.h +++ b/cpp/src/arrow/compute/kernels/codegen_internal.h @@ -482,7 +482,7 @@ struct UnboxScalar { template requires std::is_void_v::T>> void VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_func, - NullFunc&& null_func) { + NullFunc&& null_func) { VisitArraySpanInline( arr, [&](typename GetViewType::PhysicalType v) { @@ -495,7 +495,7 @@ template requires std::is_same_v::T>, Status> Status VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_func, - NullFunc&& null_func) { + NullFunc&& null_func) { return VisitArraySpanInline( arr, [&](typename GetViewType::PhysicalType v) { @@ -508,7 +508,7 @@ Status VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_func, template void VisitTwoArrayValuesInline(const ArraySpan& arr0, const ArraySpan& arr1, - VisitFunc&& valid_func, NullFunc&& null_func) { + VisitFunc&& valid_func, NullFunc&& null_func) { ArrayIterator arr0_it(arr0); ArrayIterator arr1_it(arr1); diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 1b57a7f01817..ad49b7fb84ea 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -183,8 +183,7 @@ TEST(TestScalar, IdentityCast) { test_identity_cast_for_type(type); } for (auto& type : { - arrow::fixed_size_list(arrow::int32(), 20), - arrow::list(arrow::int32()), + arrow::fixed_size_list(arrow::int32(), 20), arrow::list(arrow::int32()), arrow::large_list(arrow::int32()), // TODO(GH-45430): CastTo for ListView is not implemented yet. // arrow::list_view(arrow::int32()), arrow::large_list_view(arrow::int32()) diff --git a/cpp/src/arrow/util/async_generator.h b/cpp/src/arrow/util/async_generator.h index 701a96321740..8b2c4dc54d9c 100644 --- a/cpp/src/arrow/util/async_generator.h +++ b/cpp/src/arrow/util/async_generator.h @@ -1887,8 +1887,7 @@ Result> MakeBackgroundGenerator( /// /// This generator does not queue template -Result> MakeBlockingGenerator( - std::shared_ptr> iterator) { +Result> MakeBlockingGenerator(std::shared_ptr> iterator) { return [it = std::move(iterator)]() mutable -> Future { return Future::MakeFinished(it->Next()); }; diff --git a/cpp/src/arrow/util/bit_block_counter.h b/cpp/src/arrow/util/bit_block_counter.h index d52754c96722..ec01ac84aca9 100644 --- a/cpp/src/arrow/util/bit_block_counter.h +++ b/cpp/src/arrow/util/bit_block_counter.h @@ -426,7 +426,7 @@ class ARROW_EXPORT OptionalBinaryBitBlockCounter { template Status VisitBitBlocks(const uint8_t* bitmap, int64_t offset, int64_t length, - VisitNotNull&& visit_not_null, VisitNull&& visit_null) { + VisitNotNull&& visit_not_null, VisitNull&& visit_null) { internal::OptionalBitBlockCounter bit_counter(bitmap, offset, length); int64_t position = 0; while (position < length) { @@ -454,7 +454,7 @@ Status VisitBitBlocks(const uint8_t* bitmap, int64_t offset, int64_t length, template void VisitBitBlocksVoid(const uint8_t* bitmap, int64_t offset, int64_t length, - VisitNotNull&& visit_not_null, VisitNull&& visit_null) { + VisitNotNull&& visit_not_null, VisitNull&& visit_null) { internal::OptionalBitBlockCounter bit_counter(bitmap, offset, length); int64_t position = 0; while (position < length) { @@ -481,9 +481,9 @@ void VisitBitBlocksVoid(const uint8_t* bitmap, int64_t offset, int64_t length, template Status VisitTwoBitBlocks(const uint8_t* left_bitmap, int64_t left_offset, - const uint8_t* right_bitmap, int64_t right_offset, - int64_t length, VisitNotNull&& visit_not_null, - VisitNull&& visit_null) { + const uint8_t* right_bitmap, int64_t right_offset, + int64_t length, VisitNotNull&& visit_not_null, + VisitNull&& visit_null) { if (left_bitmap == NULLPTR || right_bitmap == NULLPTR) { // At most one bitmap is present if (left_bitmap == NULLPTR) { @@ -525,9 +525,9 @@ Status VisitTwoBitBlocks(const uint8_t* left_bitmap, int64_t left_offset, template void VisitTwoBitBlocksVoid(const uint8_t* left_bitmap, int64_t left_offset, - const uint8_t* right_bitmap, int64_t right_offset, - int64_t length, VisitNotNull&& visit_not_null, - VisitNull&& visit_null) { + const uint8_t* right_bitmap, int64_t right_offset, + int64_t length, VisitNotNull&& visit_not_null, + VisitNull&& visit_null) { if (left_bitmap == NULLPTR || right_bitmap == NULLPTR) { // At most one bitmap is present if (left_bitmap == NULLPTR) { diff --git a/cpp/src/gandiva/engine.cc b/cpp/src/gandiva/engine.cc index 057254b5ef4c..42ceb4fe6ab2 100644 --- a/cpp/src/gandiva/engine.cc +++ b/cpp/src/gandiva/engine.cc @@ -200,29 +200,34 @@ void AddProcessSymbol(llvm::orc::LLJIT& lljit) { } #ifdef JIT_LINK_SUPPORTED +# if LLVM_VERSION_MAJOR < 23 Result> CreateMemmoryManager() { auto maybe_mem_manager = llvm::jitlink::InProcessMemoryManager::Create(); return AsArrowResult(maybe_mem_manager, "Could not create memory manager: "); } +# endif Status UseJITLinkIfEnabled(llvm::orc::LLJITBuilder& jit_builder) { static auto maybe_use_jit_link = ::arrow::internal::GetEnvVar("GANDIVA_USE_JIT_LINK"); if (maybe_use_jit_link.ok()) { - ARROW_ASSIGN_OR_RAISE(static auto memory_manager, CreateMemmoryManager()); # if LLVM_VERSION_MAJOR >= 23 jit_builder.setObjectLinkingLayerCreator( - [](llvm::orc::ExecutionSession& ES, llvm::jitlink::JITLinkMemoryManager&) { - return std::make_unique(ES, *memory_manager); + [](llvm::orc::ExecutionSession& ES, + llvm::jitlink::JITLinkMemoryManager& memory_manager) { + return std::make_unique(ES, memory_manager); }); -# elif LLVM_VERSION_MAJOR >= 21 +# else + ARROW_ASSIGN_OR_RAISE(static auto memory_manager, CreateMemmoryManager()); +# if LLVM_VERSION_MAJOR >= 21 jit_builder.setObjectLinkingLayerCreator([&](llvm::orc::ExecutionSession& ES) { return std::make_unique(ES, *memory_manager); }); -# else +# else jit_builder.setObjectLinkingLayerCreator( [&](llvm::orc::ExecutionSession& ES, const llvm::Triple& TT) { return std::make_unique(ES, *memory_manager); }); +# endif # endif } return Status::OK(); From eee5f513f7b0809249dac744354f18bf34ac56be Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Wed, 9 Sep 2026 23:09:26 +0800 Subject: [PATCH 3/6] Update llvmdev version constraint in conda environment Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- ci/conda_env_gandiva.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/conda_env_gandiva.txt b/ci/conda_env_gandiva.txt index edc49b692b84..2eeedbc94efb 100644 --- a/ci/conda_env_gandiva.txt +++ b/ci/conda_env_gandiva.txt @@ -16,4 +16,4 @@ # under the License. clang -llvmdev +llvmdev<24 From 6c10961b4eed8a591ef551178e48b7540bca1784 Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Wed, 9 Sep 2026 23:32:21 +0800 Subject: [PATCH 4/6] add more comments --- cpp/src/gandiva/engine.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cpp/src/gandiva/engine.cc b/cpp/src/gandiva/engine.cc index 42ceb4fe6ab2..718a9a9061de 100644 --- a/cpp/src/gandiva/engine.cc +++ b/cpp/src/gandiva/engine.cc @@ -443,6 +443,9 @@ Status Engine::LoadPreCompiledIR() { // Built-in bitcode is JIT-compiled on the runtime host. Do not retain the target // selected by Clang when the bitcode was built. + // LLVM 23 checks target-feature compatibility even for alwaysinline functions and + // prevents inlining on a mismatch. See the LLVM 23 release notes: + // https://releases.llvm.org/23.1.0/docs/ReleaseNotes.html#changes-to-the-llvm-ir RemoveBuildTargetAttributes(*src_ir_module); return VerifyAndLinkModule(*module_, std::move(src_ir_module)); From 1fedaa64aa6d025e221465c03c61f98b55f9defe Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Thu, 10 Sep 2026 12:47:43 +0800 Subject: [PATCH 5/6] fix ci --- cpp/src/gandiva/engine.cc | 24 +++++++++++++++++- cpp/src/gandiva/llvm_generator.cc | 27 ++++++++++++++++---- cpp/src/gandiva/llvm_generator.h | 1 + cpp/src/gandiva/llvm_generator_test.cc | 35 ++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 6 deletions(-) diff --git a/cpp/src/gandiva/engine.cc b/cpp/src/gandiva/engine.cc index 718a9a9061de..98a94628ba01 100644 --- a/cpp/src/gandiva/engine.cc +++ b/cpp/src/gandiva/engine.cc @@ -185,6 +185,26 @@ void AddAbsoluteSymbol(llvm::orc::LLJIT& lljit, const std::string& name, llvm::cantFail(std::move(error)); } +void AddNativeBoolZExtAttrs(llvm::Function& function) { + // Gandiva uses i1 parameters and results in native C++ mappings only for bool. + const auto* function_type = function.getFunctionType(); + if (function_type->getReturnType()->isIntegerTy(1)) { + // A native bool result must be zero-extended by the callee before it crosses + // the ABI boundary. This matches Clang's lowering of C++ bool. + function.addRetAttr(llvm::Attribute::ZExt); + } + + for (unsigned i = 0; i < function_type->getNumParams(); ++i) { + if (function_type->getParamType(i)->isIntegerTy(1)) { + // The caller must pass a native bool as 0 or 1. + // LLVM 23 can replace `icmp ne (and X, 1), 0` with `trunc X to i1`; i1 only defines + // bit 0, so this ABI attribute is required to normalize the value at the call. + // https://github.com/llvm/llvm-project/pull/178977 + function.addParamAttr(i, llvm::Attribute::ZExt); + } + } +} + // add current process symbol to dylib // LLVM >= 18 does this automatically void AddProcessSymbol(llvm::orc::LLJIT& lljit) { @@ -627,7 +647,9 @@ Result Engine::CompiledFunction(const std::string& function) { void Engine::AddGlobalMappingForFunc(const std::string& name, llvm::Type* ret_type, const std::vector& args, void* func) { const auto prototype = llvm::FunctionType::get(ret_type, args, /*is_var_arg*/ false); - llvm::Function::Create(prototype, llvm::GlobalValue::ExternalLinkage, name, module()); + auto* function = llvm::Function::Create(prototype, llvm::GlobalValue::ExternalLinkage, + name, module()); + AddNativeBoolZExtAttrs(*function); AddAbsoluteSymbol(*lljit_, name, func); } diff --git a/cpp/src/gandiva/llvm_generator.cc b/cpp/src/gandiva/llvm_generator.cc index a42f71a1f7f1..0201e79588e5 100644 --- a/cpp/src/gandiva/llvm_generator.cc +++ b/cpp/src/gandiva/llvm_generator.cc @@ -32,6 +32,22 @@ namespace gandiva { +namespace { + +void CopyZExtAttrs(const llvm::Function& function, llvm::CallBase& call) { + if (function.hasRetAttribute(llvm::Attribute::ZExt)) { + call.addRetAttr(llvm::Attribute::ZExt); + } + + for (unsigned i = 0; i < function.arg_size(); ++i) { + if (function.hasParamAttribute(i, llvm::Attribute::ZExt)) { + call.addParamAttr(i, llvm::Attribute::ZExt); + } + } +} + +} // namespace + #define ADD_TRACE(...) \ if (enable_ir_traces_) { \ AddTrace(__VA_ARGS__); \ @@ -543,16 +559,17 @@ llvm::Value* LLVMGenerator::AddFunctionCall(const std::string& full_name, } // build a call to the llvm function. - llvm::Value* value; + llvm::CallInst* call; if (ret_type->isVoidTy()) { // void functions can't have a name for the call. - value = ir_builder()->CreateCall(fn, args); + call = ir_builder()->CreateCall(fn, args); } else { - value = ir_builder()->CreateCall(fn, args, full_name); - DCHECK(value->getType() == ret_type); + call = ir_builder()->CreateCall(fn, args, full_name); + DCHECK(call->getType() == ret_type); } + CopyZExtAttrs(*fn, *call); - return value; + return call; } std::shared_ptr LLVMGenerator::BuildDecimalLValue(llvm::Value* value, diff --git a/cpp/src/gandiva/llvm_generator.h b/cpp/src/gandiva/llvm_generator.h index a60e2bf6b29e..c6e4e821dfe5 100644 --- a/cpp/src/gandiva/llvm_generator.h +++ b/cpp/src/gandiva/llvm_generator.h @@ -91,6 +91,7 @@ class GANDIVA_EXPORT LLVMGenerator { FRIEND_TEST(TestLLVMGenerator, VerifyPCFunctions); FRIEND_TEST(TestLLVMGenerator, TestAdd); + FRIEND_TEST(TestLLVMGenerator, TestBoolCallAttrs); FRIEND_TEST(TestLLVMGenerator, TestNullInternal); friend class TestLLVMGenerator; diff --git a/cpp/src/gandiva/llvm_generator_test.cc b/cpp/src/gandiva/llvm_generator_test.cc index b3aa0465e6c9..cadf25fa96e7 100644 --- a/cpp/src/gandiva/llvm_generator_test.cc +++ b/cpp/src/gandiva/llvm_generator_test.cc @@ -66,6 +66,41 @@ TEST_F(TestLLVMGenerator, VerifyPCFunctions) { } } +TEST_F(TestLLVMGenerator, TestBoolCallAttrs) { + ASSERT_OK_AND_ASSIGN(auto generator, LLVMGenerator::Make(TestConfiguration(), false)); + + // Check that native bool declarations carry the ABI attributes. + auto* function = generator->module()->getFunction("gdv_fn_in_expr_lookup_int32"); + ASSERT_NE(function, nullptr); + EXPECT_TRUE(function->hasRetAttribute(llvm::Attribute::ZExt)); + EXPECT_TRUE(function->hasParamAttribute(2, llvm::Attribute::ZExt)); + + auto* types = generator->types(); + auto* prototype = llvm::FunctionType::get(types->void_type(), /*is_var_arg=*/false); + auto* caller = llvm::Function::Create(prototype, llvm::GlobalValue::ExternalLinkage, + "bool_call_attrs", generator->module()); + auto* entry = llvm::BasicBlock::Create(*generator->context(), "entry", caller); + generator->ir_builder()->SetInsertPoint(entry); + + // Check that AddFunctionCall copies the attributes to the call site. + auto* call = llvm::cast(generator->AddFunctionCall( + "gdv_fn_in_expr_lookup_int32", types->i1_type(), + {types->i64_constant(0), types->i32_constant(0), types->true_constant()})); + EXPECT_TRUE(call->getAttributes().hasRetAttr(llvm::Attribute::ZExt)); + EXPECT_TRUE(call->getAttributes().hasParamAttr(2, llvm::Attribute::ZExt)); + + // Check that ordinary LLVM i1 functions do not receive the attributes. + auto* i1_prototype = + llvm::FunctionType::get(types->i1_type(), {types->i1_type()}, false); + llvm::Function::Create(i1_prototype, llvm::GlobalValue::ExternalLinkage, "plain_i1", + generator->module()); + auto* i1_call = llvm::cast( + generator->AddFunctionCall("plain_i1", types->i1_type(), {types->true_constant()})); + generator->ir_builder()->CreateRetVoid(); + EXPECT_FALSE(i1_call->getAttributes().hasRetAttr(llvm::Attribute::ZExt)); + EXPECT_FALSE(i1_call->getAttributes().hasParamAttr(0, llvm::Attribute::ZExt)); +} + TEST_F(TestLLVMGenerator, TestAdd) { // Setup LLVM generator to do an arithmetic add of two vectors ASSERT_OK_AND_ASSIGN(auto generator, From 6e940058274576c356a5213e95ab5f54add69adc Mon Sep 17 00:00:00 2001 From: Zehua Zou Date: Thu, 10 Sep 2026 14:05:46 +0800 Subject: [PATCH 6/6] add two TODO comments --- cpp/src/gandiva/engine.cc | 2 ++ cpp/src/gandiva/llvm_generator.cc | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/cpp/src/gandiva/engine.cc b/cpp/src/gandiva/engine.cc index 98a94628ba01..3746363c457b 100644 --- a/cpp/src/gandiva/engine.cc +++ b/cpp/src/gandiva/engine.cc @@ -649,6 +649,8 @@ void Engine::AddGlobalMappingForFunc(const std::string& name, llvm::Type* ret_ty const auto prototype = llvm::FunctionType::get(ret_type, args, /*is_var_arg*/ false); auto* function = llvm::Function::Create(prototype, llvm::GlobalValue::ExternalLinkage, name, module()); + // TODO: Other native function mappings may require target-specific ABI attributes + // that cannot be inferred from their LLVM types alone. AddNativeBoolZExtAttrs(*function); AddAbsoluteSymbol(*lljit_, name, func); } diff --git a/cpp/src/gandiva/llvm_generator.cc b/cpp/src/gandiva/llvm_generator.cc index 0201e79588e5..23a7d5bc16a7 100644 --- a/cpp/src/gandiva/llvm_generator.cc +++ b/cpp/src/gandiva/llvm_generator.cc @@ -35,6 +35,13 @@ namespace gandiva { namespace { void CopyZExtAttrs(const llvm::Function& function, llvm::CallBase& call) { + // https://llvm.org/docs/LangRef.html#parameter-attributes + // "ABI attributes must be specified both at the function declaration/definition and + // call-site, otherwise the behavior may be undefined. ABI attributes cannot be safely + // dropped." + // + // TODO: Copy other ABI attributes as well. This currently copies only `zeroext`, + // which is required for Gandiva's native bool parameters and results. if (function.hasRetAttribute(llvm::Attribute::ZExt)) { call.addRetAttr(llvm::Attribute::ZExt); }