Skip to content

GH-43719: [C++] Clarify the way SIMD-enabled agg kernels come from the same code in different compilation units #43720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pitrou merged 16 commits into from
Sep 3, 2024
Merged

GH-43719: [C++] Clarify the way SIMD-enabled agg kernels come from the same code in different compilation units #43720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
728169c
Rename aggregate_basic_internal.h to aggregate_basic_internal.inc.cc
felipecrv Aug 15, 2024
233fa48
Use aggregate_basic_internal.inc.cc from specific compilation units
felipecrv Aug 15, 2024
79b01d5
Rename aggregate_basic_internal.inc.cc to aggregate_basic-inl.h
felipecrv Aug 15, 2024
5fb32ce
Organize the kernels
felipecrv Aug 15, 2024
dd6287d
Use Native instead of Default as non-SIMD suffix
felipecrv Aug 15, 2024
cacd070
Please the linter
felipecrv Aug 15, 2024
7aca534
Rename -inl.h to -inl.cc
felipecrv Aug 16, 2024
3359c5b
Allow -inl.{cc,h} filenames
felipecrv Aug 16, 2024
2d8abc7
Remove now unecessary extra includes
felipecrv Aug 19, 2024
ec34acd
Undo s/Default/Native
felipecrv Aug 19, 2024
ae52c5d
Rename -inl.cc to .inc
felipecrv Aug 19, 2024
8a30725
Undo lint_cpp_cli.py changes
felipecrv Aug 19, 2024
b6f4af4
fixup! Undo s/Default/Native
felipecrv Aug 19, 2024
2030b8a
Rename .inc to .inc.cc
felipecrv Aug 22, 2024
370b6a3
NOLINT the .cc inclusions
felipecrv Aug 28, 2024
447fbc9
Fix lint + make more definitions anonymous/static
pitrou Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 21 additions & 16 deletions cpp/src/arrow/compute/kernels/aggregate_basic.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
#include "arrow/util/cpu_info.h"
#include "arrow/util/hashing.h"

#include <memory>
// Include templated definitions for aggregate kernels that must compiled here
// with the SIMD level configured for this compilation unit in the build.
#include "arrow/compute/kernels/aggregate_basic.inc.cc" // NOLINT(build/include)

namespace arrow {
namespace compute {
Expand Down Expand Up @@ -276,11 +278,6 @@ struct SumImplDefault : public SumImpl<ArrowType, SimdLevel::NONE> {
using SumImpl<ArrowType, SimdLevel::NONE>::SumImpl;
};

template <typename ArrowType>
struct MeanImplDefault : public MeanImpl<ArrowType, SimdLevel::NONE> {
using MeanImpl<ArrowType, SimdLevel::NONE>::MeanImpl;
};

Result<std::unique_ptr<KernelState>> SumInit(KernelContext* ctx,
const KernelInitArgs& args) {
SumLikeInit<SumImplDefault> visitor(
Expand All @@ -289,6 +286,14 @@ Result<std::unique_ptr<KernelState>> SumInit(KernelContext* ctx,
return visitor.Create();
}

// ----------------------------------------------------------------------
// Mean implementation

template <typename ArrowType>
struct MeanImplDefault : public MeanImpl<ArrowType, SimdLevel::NONE> {
using MeanImpl<ArrowType, SimdLevel::NONE>::MeanImpl;
};

Result<std::unique_ptr<KernelState>> MeanInit(KernelContext* ctx,
const KernelInitArgs& args) {
MeanKernelInit<MeanImplDefault> visitor(
Expand Down Expand Up @@ -482,8 +487,8 @@ void AddFirstOrLastAggKernel(ScalarAggregateFunction* func,
// ----------------------------------------------------------------------
// MinMax implementation

Result<std::unique_ptr<KernelState>> MinMaxInit(KernelContext* ctx,
const KernelInitArgs& args) {
Result<std::unique_ptr<KernelState>> MinMaxInitDefault(KernelContext* ctx,
const KernelInitArgs& args) {
ARROW_ASSIGN_OR_RAISE(TypeHolder out_type,
args.kernel->signature->out_type().Resolve(ctx, args.inputs));
MinMaxInitState<SimdLevel::NONE> visitor(
Expand Down Expand Up @@ -1114,14 +1119,14 @@ void RegisterScalarAggregateBasic(FunctionRegistry* registry) {
// Add min max function
func = std::make_shared<ScalarAggregateFunction>("min_max", Arity::Unary(), min_max_doc,
&default_scalar_aggregate_options);
AddMinMaxKernels(MinMaxInit, {null(), boolean()}, func.get());
AddMinMaxKernels(MinMaxInit, NumericTypes(), func.get());
AddMinMaxKernels(MinMaxInit, TemporalTypes(), func.get());
AddMinMaxKernels(MinMaxInit, BaseBinaryTypes(), func.get());
AddMinMaxKernel(MinMaxInit, Type::FIXED_SIZE_BINARY, func.get());
AddMinMaxKernel(MinMaxInit, Type::INTERVAL_MONTHS, func.get());
AddMinMaxKernel(MinMaxInit, Type::DECIMAL128, func.get());
AddMinMaxKernel(MinMaxInit, Type::DECIMAL256, func.get());
AddMinMaxKernels(MinMaxInitDefault, {null(), boolean()}, func.get());
AddMinMaxKernels(MinMaxInitDefault, NumericTypes(), func.get());
AddMinMaxKernels(MinMaxInitDefault, TemporalTypes(), func.get());
AddMinMaxKernels(MinMaxInitDefault, BaseBinaryTypes(), func.get());
AddMinMaxKernel(MinMaxInitDefault, Type::FIXED_SIZE_BINARY, func.get());
AddMinMaxKernel(MinMaxInitDefault, Type::INTERVAL_MONTHS, func.get());
AddMinMaxKernel(MinMaxInitDefault, Type::DECIMAL128, func.get());
AddMinMaxKernel(MinMaxInitDefault, Type::DECIMAL256, func.get());
// Add the SIMD variants for min max
#if defined(ARROW_HAVE_RUNTIME_AVX2)
if (cpu_info->IsSupported(arrow::internal::CpuInfo::AVX2)) {
Expand Down
Loading