This is an automated email from the ASF dual-hosted git repository.
HuaHuaY pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new bcaac3ba3f4 GH-51245: [C++][Compute][Gandiva] Add support for LLVM
23.1 (#51266)
bcaac3ba3f4 is described below
commit bcaac3ba3f45a9ff597f6f3eb14adf8fe7d34bfc
Author: Zehua Zou <[email protected]>
AuthorDate: Mon Sep 21 18:16:03 2026 +0800
GH-51245: [C++][Compute][Gandiva] Add support for LLVM 23.1 (#51266)
### Rationale for this change
Add support for LLVM 23.1.
### What changes are included in this PR?
1. Remove `static` from some template functions in header files.
2. Fixed a bug in `CastImpl` template instantiation about `Date` and
`Timestamp` data types, and add a unit test.
3. Remove the build target attributes in bitcode generated by Gandiva.
4. Add `zeroext` attr when LLVM JIT passes `i1` to GCC-compiled functions.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* GitHub Issue: #51245
Lead-authored-by: Zehua Zou <[email protected]>
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
Signed-off-by: Zehua Zou <[email protected]>
---
ci/conda_env_gandiva.txt | 2 +-
cpp/CMakeLists.txt | 1 +
cpp/src/arrow/compute/exec.cc | 10 ---
cpp/src/arrow/compute/function_internal.h | 4 +-
cpp/src/arrow/compute/kernels/codegen_internal.h | 14 ++--
cpp/src/arrow/compute/kernels/vector_hash.cc | 2 +-
cpp/src/arrow/scalar.cc | 20 ++++--
cpp/src/arrow/scalar_test.cc | 45 +++++++++++++
cpp/src/arrow/util/async_generator.h | 7 +-
cpp/src/arrow/util/bit_block_counter.h | 24 +++----
cpp/src/arrow/util/future.h | 2 +-
cpp/src/gandiva/decimal_ir.cc | 14 ++--
cpp/src/gandiva/engine.cc | 63 +++++++++++++-----
cpp/src/gandiva/llvm_generator.cc | 12 ++--
cpp/src/gandiva/llvm_generator.h | 1 +
cpp/src/gandiva/llvm_generator_test.cc | 57 ++++++++++++++++
cpp/src/gandiva/llvm_util_internal.h | 84 ++++++++++++++++++++++++
17 files changed, 291 insertions(+), 71 deletions(-)
diff --git a/ci/conda_env_gandiva.txt b/ci/conda_env_gandiva.txt
index 7127dc90558..2eeedbc94ef 100644
--- a/ci/conda_env_gandiva.txt
+++ b/ci/conda_env_gandiva.txt
@@ -16,4 +16,4 @@
# under the License.
clang
-llvmdev<23
+llvmdev<24
diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
index 8fd38974ec6..d39120dec2e 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 411ff0bb026..db86a75c53f 100644
--- a/cpp/src/arrow/compute/exec.cc
+++ b/cpp/src/arrow/compute/exec.cc
@@ -1185,16 +1185,6 @@ class ScalarAggExecutor : public
KernelExecutorImpl<ScalarAggregateKernel> {
const FunctionOptions* options_;
};
-template <typename ExecutorType,
- typename FunctionType = typename ExecutorType::FunctionType>
-Result<std::unique_ptr<KernelExecutor>> MakeExecutor(ExecContext* ctx,
- const Function* func,
- const FunctionOptions*
options) {
- DCHECK_EQ(ExecutorType::function_kind, func->kind());
- auto typed_func = checked_cast<const FunctionType*>(func);
- return std::make_unique<ExecutorType>(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 d1f8d06de84..92f8529ab78 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<is_optional_v<T>, Result<T>>
GenericFromScalar(
}
template <typename T>
-static enable_if_same<typename CTypeTraits<T>::ArrowType, ListType, Result<T>>
-GenericFromScalar(const std::shared_ptr<Scalar>& value) {
+enable_if_same<typename CTypeTraits<T>::ArrowType, ListType, Result<T>>
GenericFromScalar(
+ const std::shared_ptr<Scalar>& 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 bfc529dc0a4..3a2bcefab0a 100644
--- a/cpp/src/arrow/compute/kernels/codegen_internal.h
+++ b/cpp/src/arrow/compute/kernels/codegen_internal.h
@@ -481,8 +481,8 @@ struct UnboxScalar<Decimal256Type> {
template <typename T, typename VisitFunc, typename NullFunc>
requires std::is_void_v<std::invoke_result_t<VisitFunc, typename
GetViewType<T>::T>>
-static void VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&&
valid_func,
- NullFunc&& null_func) {
+void VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_func,
+ NullFunc&& null_func) {
VisitArraySpanInline<T>(
arr,
[&](typename GetViewType<T>::PhysicalType v) {
@@ -494,8 +494,8 @@ static void VisitArrayValuesInline(const ArraySpan& arr,
VisitFunc&& valid_func,
template <typename T, typename VisitFunc, typename NullFunc>
requires std::is_same_v<std::invoke_result_t<VisitFunc, typename
GetViewType<T>::T>,
Status>
-static Status VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&&
valid_func,
- NullFunc&& null_func) {
+Status VisitArrayValuesInline(const ArraySpan& arr, VisitFunc&& valid_func,
+ NullFunc&& null_func) {
return VisitArraySpanInline<T>(
arr,
[&](typename GetViewType<T>::PhysicalType v) {
@@ -507,8 +507,8 @@ static Status VisitArrayValuesInline(const ArraySpan& arr,
VisitFunc&& valid_fun
// Like VisitArrayValuesInline, but for binary functions.
template <typename Arg0Type, typename Arg1Type, typename VisitFunc, typename
NullFunc>
-static void VisitTwoArrayValuesInline(const ArraySpan& arr0, const ArraySpan&
arr1,
- VisitFunc&& valid_func, NullFunc&&
null_func) {
+void VisitTwoArrayValuesInline(const ArraySpan& arr0, const ArraySpan& arr1,
+ VisitFunc&& valid_func, NullFunc&& null_func) {
ArrayIterator<Arg0Type> arr0_it(arr0);
ArrayIterator<Arg1Type> arr1_it(arr1);
@@ -584,7 +584,7 @@ namespace applicator {
// static Status Call(KernelContext*, const Scalar& arg0, const ArraySpan&
arg1,
// ExecResult* out)
template <typename Operator>
-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 f42972e18af..9a29dcf333f 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 <class Index>
- 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 22cffae05cb..e619c5344d3 100644
--- a/cpp/src/arrow/scalar.cc
+++ b/cpp/src/arrow/scalar.cc
@@ -17,6 +17,7 @@
#include "arrow/scalar.h"
+#include <chrono>
#include <memory>
#include <ostream>
#include <sstream>
@@ -31,6 +32,7 @@
#include "arrow/type.h"
#include "arrow/util/bitmap_ops.h"
#include "arrow/util/checked_cast.h"
+#include "arrow/util/chrono_internal.h"
#include "arrow/util/decimal.h"
#include "arrow/util/formatting.h"
#include "arrow/util/hashing.h"
@@ -1191,13 +1193,13 @@ constexpr int64_t kMillisecondsInDay = 86400000;
// date to date
template <typename To>
-enable_if_t<std::is_same<To, Date64Scalar>::value,
Result<std::shared_ptr<Scalar>>>
+enable_if_t<std::is_same<To, Date64Type>::value,
Result<std::shared_ptr<Scalar>>>
CastImpl(const Date32Scalar& from, std::shared_ptr<DataType> to_type) {
return std::make_shared<Date64Scalar>(from.value * kMillisecondsInDay,
std::move(to_type));
}
template <typename To>
-enable_if_t<std::is_same<To, Date32Scalar>::value,
Result<std::shared_ptr<Scalar>>>
+enable_if_t<std::is_same<To, Date32Type>::value,
Result<std::shared_ptr<Scalar>>>
CastImpl(const Date64Scalar& from, std::shared_ptr<DataType> to_type) {
return std::make_shared<Date32Scalar>(
static_cast<int32_t>(from.value / kMillisecondsInDay),
std::move(to_type));
@@ -1205,27 +1207,31 @@ CastImpl(const Date64Scalar& from,
std::shared_ptr<DataType> to_type) {
// timestamp to date
template <typename To>
-enable_if_t<std::is_same<To, Date64Scalar>::value,
Result<std::shared_ptr<Scalar>>>
+enable_if_t<std::is_same<To, Date64Type>::value,
Result<std::shared_ptr<Scalar>>>
CastImpl(const TimestampScalar& from, std::shared_ptr<DataType> to_type) {
ARROW_ASSIGN_OR_RAISE(
auto millis,
util::ConvertTimestampValue(from.type, timestamp(TimeUnit::MILLI),
from.value));
- return std::make_shared<Date64Scalar>(millis - millis % kMillisecondsInDay,
+ const auto days_since_epoch =
+
internal::chrono::floor<internal::chrono::days>(std::chrono::milliseconds{millis});
+ return std::make_shared<Date64Scalar>(days_since_epoch.count() *
kMillisecondsInDay,
std::move(to_type));
}
template <typename To>
-enable_if_t<std::is_same<To, Date32Scalar>::value,
Result<std::shared_ptr<Scalar>>>
+enable_if_t<std::is_same<To, Date32Type>::value,
Result<std::shared_ptr<Scalar>>>
CastImpl(const TimestampScalar& from, std::shared_ptr<DataType> to_type) {
ARROW_ASSIGN_OR_RAISE(
auto millis,
util::ConvertTimestampValue(from.type, timestamp(TimeUnit::MILLI),
from.value));
- return std::make_shared<Date32Scalar>(static_cast<int32_t>(millis /
kMillisecondsInDay),
+ const auto days_since_epoch =
+
internal::chrono::floor<internal::chrono::days>(std::chrono::milliseconds{millis});
+ return
std::make_shared<Date32Scalar>(static_cast<int32_t>(days_since_epoch.count()),
std::move(to_type));
}
// date to timestamp
template <typename To, typename From>
-enable_if_timestamp<Result<std::shared_ptr<To>>> CastImpl(
+enable_if_timestamp<To, Result<std::shared_ptr<Scalar>>> CastImpl(
const DateScalar<From>& from, std::shared_ptr<DataType> to_type) {
using ToScalar = typename TypeTraits<To>::ScalarType;
int64_t millis = from.value;
diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc
index a08ea6aa96d..9594e8883b3 100644
--- a/cpp/src/arrow/scalar_test.cc
+++ b/cpp/src/arrow/scalar_test.cc
@@ -973,6 +973,51 @@ 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));
+
+ // 1969-12-31T12:00:00Z -- floor to the previous day instead of truncating
to zero.
+ constexpr int64_t kHalfDay = kMillisecondsInDay / 2;
+ ASSERT_OK_AND_ASSIGN(
+ auto negative_date64_from_timestamp,
+ TimestampScalar(-kHalfDay, timestamp(TimeUnit::MILLI)).CastTo(date64()));
+ EXPECT_EQ(*negative_date64_from_timestamp,
Date64Scalar(-kMillisecondsInDay));
+
+ ASSERT_OK_AND_ASSIGN(
+ auto negative_date32_from_timestamp,
+ TimestampScalar(-kHalfDay, timestamp(TimeUnit::MILLI)).CastTo(date32()));
+ EXPECT_EQ(*negative_date32_from_timestamp, Date32Scalar(-1));
+}
+
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 63cb01a9fe4..8b2c4dc54d9 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 <typename T>
-static Result<AsyncGenerator<T>> MakeBackgroundGenerator(
+Result<AsyncGenerator<T>> MakeBackgroundGenerator(
Iterator<T> iterator, internal::Executor* io_executor,
int max_q = kDefaultBackgroundMaxQ, int q_restart =
kDefaultBackgroundQRestart) {
if (max_q < q_restart) {
@@ -1887,15 +1887,14 @@ static Result<AsyncGenerator<T>>
MakeBackgroundGenerator(
///
/// This generator does not queue
template <typename T>
-static Result<AsyncGenerator<T>> MakeBlockingGenerator(
- std::shared_ptr<Iterator<T>> iterator) {
+Result<AsyncGenerator<T>> MakeBlockingGenerator(std::shared_ptr<Iterator<T>>
iterator) {
return [it = std::move(iterator)]() mutable -> Future<T> {
return Future<T>::MakeFinished(it->Next());
};
}
template <typename T>
-static Result<AsyncGenerator<T>> MakeBlockingGenerator(Iterator<T> iterator) {
+Result<AsyncGenerator<T>> MakeBlockingGenerator(Iterator<T> iterator) {
return
MakeBlockingGenerator(std::make_shared<Iterator<T>>(std::move(iterator)));
}
diff --git a/cpp/src/arrow/util/bit_block_counter.h
b/cpp/src/arrow/util/bit_block_counter.h
index 82651a9d387..ec01ac84aca 100644
--- a/cpp/src/arrow/util/bit_block_counter.h
+++ b/cpp/src/arrow/util/bit_block_counter.h
@@ -425,8 +425,8 @@ class ARROW_EXPORT OptionalBinaryBitBlockCounter {
// Functional-style bit block visitors.
template <typename VisitNotNull, typename VisitNull>
-static Status VisitBitBlocks(const uint8_t* bitmap, int64_t offset, int64_t
length,
- VisitNotNull&& visit_not_null, VisitNull&&
visit_null) {
+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;
while (position < length) {
@@ -453,8 +453,8 @@ static Status VisitBitBlocks(const uint8_t* bitmap, int64_t
offset, int64_t leng
}
template <typename VisitNotNull, typename VisitNull>
-static void VisitBitBlocksVoid(const uint8_t* bitmap, int64_t offset, int64_t
length,
- VisitNotNull&& visit_not_null, VisitNull&&
visit_null) {
+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;
while (position < length) {
@@ -480,10 +480,10 @@ static void VisitBitBlocksVoid(const uint8_t* bitmap,
int64_t offset, int64_t le
}
template <typename VisitNotNull, typename VisitNull>
-static 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) {
+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) {
if (left_bitmap == NULLPTR || right_bitmap == NULLPTR) {
// At most one bitmap is present
if (left_bitmap == NULLPTR) {
@@ -524,10 +524,10 @@ static Status VisitTwoBitBlocks(const uint8_t*
left_bitmap, int64_t left_offset,
}
template <typename VisitNotNull, typename VisitNull>
-static 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) {
+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) {
if (left_bitmap == NULLPTR || right_bitmap == NULLPTR) {
// At most one bitmap is present
if (left_bitmap == NULLPTR) {
diff --git a/cpp/src/arrow/util/future.h b/cpp/src/arrow/util/future.h
index 1884f596979..0210350d538 100644
--- a/cpp/src/arrow/util/future.h
+++ b/cpp/src/arrow/util/future.h
@@ -693,7 +693,7 @@ class WeakFuture {
/// If a Result<Future> holds an error instead of a Future, construct a
finished Future
/// holding that error.
template <typename T>
-static Future<T> DeferNotOk(Result<Future<T>> maybe_future) {
+Future<T> DeferNotOk(Result<Future<T>> maybe_future) {
if (ARROW_PREDICT_FALSE(!maybe_future.ok())) {
return Future<T>::MakeFinished(std::move(maybe_future).status());
}
diff --git a/cpp/src/gandiva/decimal_ir.cc b/cpp/src/gandiva/decimal_ir.cc
index 30096e4d285..36e423e0647 100644
--- a/cpp/src/gandiva/decimal_ir.cc
+++ b/cpp/src/gandiva/decimal_ir.cc
@@ -23,6 +23,7 @@
#include "arrow/util/logging_internal.h"
#include "gandiva/decimal_ir.h"
#include "gandiva/decimal_type_util.h"
+#include "gandiva/llvm_util_internal.h"
// Algorithms adapted from Apache Impala
@@ -424,9 +425,15 @@ Status DecimalIR::BuildCompare(const std::string&
function_name,
llvm::Value* DecimalIR::CallDecimalFunction(const std::string& function_name,
llvm::Type* return_type,
const std::vector<llvm::Value*>&
params) {
+ auto create_call = [&](const std::vector<llvm::Value*>& args) {
+ auto* call =
ir_builder()->CreateCall(module()->getFunction(function_name), args);
+ internal::CopyZExtAttrs(*call->getCalledFunction(), *call);
+ return call;
+ };
+
if (kDecimalIRBuilderFunctions.count(function_name) != 0) {
// this is fn built with the irbuilder.
- return ir_builder()->CreateCall(module()->getFunction(function_name),
params);
+ return create_call(params);
}
// ppre-compiler fn : disassemble i128 to two i64s and re-assemble.
@@ -454,7 +461,7 @@ llvm::Value* DecimalIR::CallDecimalFunction(const
std::string& function_name,
dis_assembled_args.push_back(out_low_ptr);
// Make call to pre-compiled IR function.
- ir_builder()->CreateCall(module()->getFunction(function_name),
dis_assembled_args);
+ create_call(dis_assembled_args);
auto out_high = ir_builder()->CreateLoad(i64, out_high_ptr);
auto out_low = ir_builder()->CreateLoad(i64, out_low_ptr);
@@ -463,8 +470,7 @@ llvm::Value* DecimalIR::CallDecimalFunction(const
std::string& function_name,
DCHECK_NE(return_type, types()->void_type());
// Make call to pre-compiled IR function.
- result = ir_builder()->CreateCall(module()->getFunction(function_name),
- dis_assembled_args);
+ result = create_call(dis_assembled_args);
}
return result;
}
diff --git a/cpp/src/gandiva/engine.cc b/cpp/src/gandiva/engine.cc
index e6edb0042ce..205d2f866b2 100644
--- a/cpp/src/gandiva/engine.cc
+++ b/cpp/src/gandiva/engine.cc
@@ -113,6 +113,7 @@
#include "gandiva/decimal_ir.h"
#include "gandiva/exported_funcs.h"
#include "gandiva/exported_funcs_registry.h"
+#include "gandiva/llvm_util_internal.h"
namespace gandiva {
@@ -131,6 +132,8 @@ template <typename T>
arrow::Result<T> AsArrowResult(llvm::Expected<T>& 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());
@@ -198,24 +201,34 @@ void AddProcessSymbol(llvm::orc::LLJIT& lljit) {
}
#ifdef JIT_LINK_SUPPORTED
-Result<std::unique_ptr<llvm::jitlink::InProcessMemoryManager>>
CreateMemmoryManager() {
+# if LLVM_VERSION_MAJOR < 23
+Result<std::unique_ptr<llvm::jitlink::InProcessMemoryManager>>
CreateMemoryManager() {
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 >= 21
+# if LLVM_VERSION_MAJOR >= 23
+ jit_builder.setObjectLinkingLayerCreator(
+ [](llvm::orc::ExecutionSession& ES,
+ llvm::jitlink::JITLinkMemoryManager& memory_manager) {
+ return std::make_unique<llvm::orc::ObjectLinkingLayer>(ES,
memory_manager);
+ });
+# else
+ ARROW_ASSIGN_OR_RAISE(static auto memory_manager, CreateMemoryManager());
+# if LLVM_VERSION_MAJOR >= 21
jit_builder.setObjectLinkingLayerCreator([&](llvm::orc::ExecutionSession&
ES) {
return std::make_unique<llvm::orc::ObjectLinkingLayer>(ES,
*memory_manager);
});
-# else
+# else
jit_builder.setObjectLinkingLayerCreator(
[&](llvm::orc::ExecutionSession& ES, const llvm::Triple& TT) {
return std::make_unique<llvm::orc::ObjectLinkingLayer>(ES,
*memory_manager);
});
+# endif
# endif
}
return Status::OK();
@@ -261,13 +274,8 @@ Result<std::unique_ptr<llvm::orc::LLJIT>> BuildJIT(
return jit;
}
-arrow::Status VerifyAndLinkModule(
- llvm::Module& dest_module,
- llvm::Expected<std::unique_ptr<llvm::Module>> 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<llvm::Module> src_ir_module)
{
src_ir_module->setDataLayout(dest_module.getDataLayout());
std::string error_info;
@@ -282,6 +290,18 @@ arrow::Status VerifyAndLinkModule(
return Status::OK();
}
+void RemoveBuildTargetAttributes(llvm::Module& module) {
+ // Precompiled bitcode is JIT-compiled on the runtime host, so do not retain
the
+ // target selected when the bitcode was built. LLVM 23 checks target-feature
+ // compatibility even for alwaysinline functions and prevents inlining on a
mismatch.
+ // See
https://releases.llvm.org/23.1.0/docs/ReleaseNotes.html#changes-to-the-llvm-ir.
+ 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 +442,11 @@ Status Engine::LoadPreCompiledIR() {
/// Parse the IR module.
llvm::Expected<std::unique_ptr<llvm::Module>> 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: "));
+ RemoveBuildTargetAttributes(*src_ir_module);
+ return VerifyAndLinkModule(*module_, std::move(src_ir_module));
}
static llvm::MemoryBufferRef AsLLVMMemoryBuffer(const arrow::Buffer&
arrow_buffer) {
@@ -439,7 +460,11 @@ 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: "));
+ RemoveBuildTargetAttributes(*src_ir_module);
+ ARROW_RETURN_NOT_OK(VerifyAndLinkModule(*module_,
std::move(src_ir_module)));
}
return Status::OK();
@@ -601,7 +626,11 @@ Result<void*> Engine::CompiledFunction(const std::string&
function) {
void Engine::AddGlobalMappingForFunc(const std::string& name, llvm::Type*
ret_type,
const std::vector<llvm::Type*>& 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());
+ // TODO: Other native function mappings may require target-specific ABI
attributes
+ // that cannot be inferred from their LLVM types alone.
+ internal::AddNativeBoolZExtAttrs(*function);
AddAbsoluteSymbol(*lljit_, name, func);
}
diff --git a/cpp/src/gandiva/llvm_generator.cc
b/cpp/src/gandiva/llvm_generator.cc
index a42f71a1f7f..7caafa58369 100644
--- a/cpp/src/gandiva/llvm_generator.cc
+++ b/cpp/src/gandiva/llvm_generator.cc
@@ -28,6 +28,7 @@
#include "gandiva/expr_decomposer.h"
#include "gandiva/expression.h"
#include "gandiva/llvm_types.h"
+#include "gandiva/llvm_util_internal.h"
#include "gandiva/lvalue.h"
namespace gandiva {
@@ -543,16 +544,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);
}
+ internal::CopyZExtAttrs(*fn, *call);
- return value;
+ return call;
}
std::shared_ptr<DecimalLValue> LLVMGenerator::BuildDecimalLValue(llvm::Value*
value,
diff --git a/cpp/src/gandiva/llvm_generator.h b/cpp/src/gandiva/llvm_generator.h
index a60e2bf6b29..c6e4e821dfe 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 b3aa0465e6c..0d8206e0d63 100644
--- a/cpp/src/gandiva/llvm_generator_test.cc
+++ b/cpp/src/gandiva/llvm_generator_test.cc
@@ -27,7 +27,9 @@
#include "gandiva/expression.h"
#include "gandiva/func_descriptor.h"
#include "gandiva/function_registry.h"
+#include "gandiva/llvm_util_internal.h"
#include "gandiva/tests/test_util.h"
+#include "gandiva/tree_expr_builder.h"
namespace gandiva {
@@ -66,6 +68,61 @@ 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(internal::HasRetAttr(function->getAttributes(),
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<llvm::CallInst>(generator->AddFunctionCall(
+ "gdv_fn_in_expr_lookup_int32", types->i1_type(),
+ {types->i64_constant(0), types->i32_constant(0),
types->true_constant()}));
+ EXPECT_TRUE(internal::HasRetAttr(call->getAttributes(),
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<llvm::CallInst>(
+ generator->AddFunctionCall("plain_i1", types->i1_type(),
{types->true_constant()}));
+ generator->ir_builder()->CreateRetVoid();
+ EXPECT_FALSE(internal::HasRetAttr(i1_call->getAttributes(),
llvm::Attribute::ZExt));
+ EXPECT_FALSE(i1_call->getAttributes().hasParamAttr(0,
llvm::Attribute::ZExt));
+}
+
+TEST_F(TestLLVMGenerator, TestDecimalCallAttrs) {
+ ASSERT_OK_AND_ASSIGN(auto generator,
+ LLVMGenerator::Make(TestConfigWithIrDumping(), false));
+
+ // Check that the call emitted through DecimalIR has zeroext on the result
and the
+ // native bool argument after its i128 argument is split.
+ constexpr int32_t precision = 38;
+ constexpr int32_t scale = 5;
+ auto field = arrow::field("decimal", arrow::decimal128(precision, scale));
+ auto field_node = TreeExprBuilder::MakeField(field);
+ std::unordered_set<DecimalScalar128> constants{DecimalScalar128("6",
precision, scale)};
+ auto in_node = TreeExprBuilder::MakeInExpressionDecimal(field_node,
constants);
+ auto condition = TreeExprBuilder::MakeCondition(in_node);
+ ASSERT_OK(generator->Build({condition}));
+
+ EXPECT_THAT(generator->ir(),
+ testing::ContainsRegex(
+ R"(call zeroext i1 @gdv_fn_in_expr_lookup_decimal\(.*, i1
zeroext)"));
+}
+
TEST_F(TestLLVMGenerator, TestAdd) {
// Setup LLVM generator to do an arithmetic add of two vectors
ASSERT_OK_AND_ASSIGN(auto generator,
diff --git a/cpp/src/gandiva/llvm_util_internal.h
b/cpp/src/gandiva/llvm_util_internal.h
new file mode 100644
index 00000000000..e114e87fd80
--- /dev/null
+++ b/cpp/src/gandiva/llvm_util_internal.h
@@ -0,0 +1,84 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include "gandiva/llvm_includes.h"
+
+namespace gandiva::internal {
+
+inline bool HasRetAttr(const llvm::AttributeList& attributes,
+ llvm::Attribute::AttrKind kind) {
+#if LLVM_VERSION_MAJOR >= 14
+ return attributes.hasRetAttr(kind);
+#else
+ return attributes.hasAttribute(llvm::AttributeList::ReturnIndex, kind);
+#endif
+}
+
+inline llvm::AttributeList AddRetAttr(llvm::LLVMContext& context,
+ const llvm::AttributeList& attributes,
+ llvm::Attribute::AttrKind kind) {
+#if LLVM_VERSION_MAJOR >= 14
+ return attributes.addRetAttribute(context, kind);
+#else
+ return attributes.addAttribute(context, llvm::AttributeList::ReturnIndex,
kind);
+#endif
+}
+
+inline 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.setAttributes(AddRetAttr(function.getContext(),
function.getAttributes(),
+ 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);
+ }
+ }
+}
+
+inline void CopyZExtAttrs(const llvm::Function& function, llvm::CallInst&
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 (HasRetAttr(function.getAttributes(), llvm::Attribute::ZExt)) {
+ call.setAttributes(
+ AddRetAttr(call.getContext(), call.getAttributes(),
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 gandiva::internal
