From 728169c259d8e4c5fbedae176acdc6f450715ecb Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Carvalho Date: Thu, 15 Aug 2024 14:38:19 -0300 Subject: [PATCH 01/16] Rename aggregate_basic_internal.h to aggregate_basic_internal.inc.cc --- cpp/src/arrow/compute/kernels/aggregate_basic.cc | 2 +- cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc | 2 +- cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc | 2 +- ...gregate_basic_internal.h => aggregate_basic_internal.inc.cc} | 0 4 files changed, 3 insertions(+), 3 deletions(-) rename cpp/src/arrow/compute/kernels/{aggregate_basic_internal.h => aggregate_basic_internal.inc.cc} (100%) diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic.cc b/cpp/src/arrow/compute/kernels/aggregate_basic.cc index c5e0e6fd6e9..831d211670c 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic.cc @@ -16,7 +16,7 @@ // under the License. #include "arrow/compute/api_aggregate.h" -#include "arrow/compute/kernels/aggregate_basic_internal.h" +#include "arrow/compute/kernels/aggregate_basic_internal.inc.cc" // XXX: fix #include "arrow/compute/kernels/aggregate_internal.h" #include "arrow/compute/kernels/common_internal.h" #include "arrow/compute/kernels/util_internal.h" diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc b/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc index 03b45107eec..50c72dc1a96 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/compute/kernels/aggregate_basic_internal.h" +#include "arrow/compute/kernels/aggregate_basic_internal.inc.cc" // XXX: fix namespace arrow { namespace compute { diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc b/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc index 05356e0aa5e..09711c02405 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/compute/kernels/aggregate_basic_internal.h" +#include "arrow/compute/kernels/aggregate_basic_internal.inc.cc" // XXX: fix namespace arrow { namespace compute { diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic_internal.h b/cpp/src/arrow/compute/kernels/aggregate_basic_internal.inc.cc similarity index 100% rename from cpp/src/arrow/compute/kernels/aggregate_basic_internal.h rename to cpp/src/arrow/compute/kernels/aggregate_basic_internal.inc.cc From 233fa480937a1a95cb15e878ca602cec00a2e6ad Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Carvalho Date: Thu, 15 Aug 2024 15:38:21 -0300 Subject: [PATCH 02/16] Use aggregate_basic_internal.inc.cc from specific compilation units --- .../arrow/compute/kernels/aggregate_basic.cc | 17 +++++- .../compute/kernels/aggregate_basic_avx2.cc | 20 +++++++ .../compute/kernels/aggregate_basic_avx512.cc | 22 +++++++- .../kernels/aggregate_basic_internal.h | 52 +++++++++++++++++++ .../kernels/aggregate_basic_internal.inc.cc | 46 +++------------- 5 files changed, 116 insertions(+), 41 deletions(-) create mode 100644 cpp/src/arrow/compute/kernels/aggregate_basic_internal.h diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic.cc b/cpp/src/arrow/compute/kernels/aggregate_basic.cc index 831d211670c..1bd19ab8e7a 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic.cc @@ -15,15 +15,28 @@ // specific language governing permissions and limitations // under the License. +#include +#include +#include +#include + #include "arrow/compute/api_aggregate.h" -#include "arrow/compute/kernels/aggregate_basic_internal.inc.cc" // XXX: fix +#include "arrow/compute/kernels/aggregate_basic_internal.h" #include "arrow/compute/kernels/aggregate_internal.h" +#include "arrow/compute/kernels/codegen_internal.h" #include "arrow/compute/kernels/common_internal.h" #include "arrow/compute/kernels/util_internal.h" +#include "arrow/type.h" +#include "arrow/type_traits.h" +#include "arrow/util/align_util.h" +#include "arrow/util/bit_block_counter.h" #include "arrow/util/cpu_info.h" +#include "arrow/util/decimal.h" #include "arrow/util/hashing.h" -#include +// 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_internal.inc.cc" namespace arrow { namespace compute { diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc b/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc index 50c72dc1a96..33c89a4182f 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc @@ -15,6 +15,26 @@ // specific language governing permissions and limitations // under the License. +#include +#include +#include + +#include "arrow/compute/api_aggregate.h" +#include "arrow/compute/kernels/aggregate_basic_internal.h" +#include "arrow/compute/kernels/aggregate_internal.h" +#include "arrow/compute/kernels/codegen_internal.h" +#include "arrow/compute/kernels/common_internal.h" +#include "arrow/compute/kernels/util_internal.h" +#include "arrow/type.h" +#include "arrow/type_traits.h" +#include "arrow/util/align_util.h" +#include "arrow/util/bit_block_counter.h" +#include "arrow/util/cpu_info.h" +#include "arrow/util/decimal.h" +#include "arrow/util/hashing.h" + +// 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_internal.inc.cc" // XXX: fix namespace arrow { diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc b/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc index 09711c02405..9d598a2135e 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc @@ -15,7 +15,27 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/compute/kernels/aggregate_basic_internal.inc.cc" // XXX: fix +#include +#include +#include + +#include "arrow/compute/api_aggregate.h" +#include "arrow/compute/kernels/aggregate_basic_internal.h" +#include "arrow/compute/kernels/aggregate_internal.h" +#include "arrow/compute/kernels/codegen_internal.h" +#include "arrow/compute/kernels/common_internal.h" +#include "arrow/compute/kernels/util_internal.h" +#include "arrow/type.h" +#include "arrow/type_traits.h" +#include "arrow/util/align_util.h" +#include "arrow/util/bit_block_counter.h" +#include "arrow/util/cpu_info.h" +#include "arrow/util/decimal.h" +#include "arrow/util/hashing.h" + +// 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_internal.inc.cc" namespace arrow { namespace compute { diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic_internal.h b/cpp/src/arrow/compute/kernels/aggregate_basic_internal.h new file mode 100644 index 00000000000..f111e56f1d8 --- /dev/null +++ b/cpp/src/arrow/compute/kernels/aggregate_basic_internal.h @@ -0,0 +1,52 @@ +// 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 +#include + +#include "arrow/compute/kernel.h" +#include "arrow/compute/kernels/codegen_internal.h" +#include "arrow/compute/type_fwd.h" +#include "arrow/type_fwd.h" + +namespace arrow::compute::internal { + +void AddBasicAggKernels(KernelInit init, + const std::vector>& types, + std::shared_ptr out_ty, ScalarAggregateFunction* func, + SimdLevel::type simd_level = SimdLevel::NONE); + +void AddMinMaxKernels(KernelInit init, + const std::vector>& types, + ScalarAggregateFunction* func, + SimdLevel::type simd_level = SimdLevel::NONE); +void AddMinMaxKernel(KernelInit init, internal::detail::GetTypeId get_id, + ScalarAggregateFunction* func, + SimdLevel::type simd_level = SimdLevel::NONE); + +// SIMD variants for kernels +void AddSumAvx2AggKernels(ScalarAggregateFunction* func); +void AddMeanAvx2AggKernels(ScalarAggregateFunction* func); +void AddMinMaxAvx2AggKernels(ScalarAggregateFunction* func); + +void AddSumAvx512AggKernels(ScalarAggregateFunction* func); +void AddMeanAvx512AggKernels(ScalarAggregateFunction* func); +void AddMinMaxAvx512AggKernels(ScalarAggregateFunction* func); + +} // namespace arrow::compute::internal diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic_internal.inc.cc b/cpp/src/arrow/compute/kernels/aggregate_basic_internal.inc.cc index f08e7aaa538..cbe0bb72fde 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic_internal.inc.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic_internal.inc.cc @@ -15,46 +15,15 @@ // specific language governing permissions and limitations // under the License. -#pragma once - -#include -#include -#include - -#include "arrow/compute/api_aggregate.h" -#include "arrow/compute/kernels/aggregate_internal.h" -#include "arrow/compute/kernels/codegen_internal.h" -#include "arrow/compute/kernels/common_internal.h" -#include "arrow/compute/kernels/util_internal.h" -#include "arrow/type.h" -#include "arrow/type_traits.h" -#include "arrow/util/align_util.h" -#include "arrow/util/bit_block_counter.h" -#include "arrow/util/decimal.h" +// .inc.cc file to be included in compilation unit where kernels are meant to +// be compiled auto-vectorized by the compiler with different SIMD levels passed +// as compiler flags. +// +// It contains no includes to avoid double inclusion in the compilation unit +// that includes this .inc.cc file. namespace arrow::compute::internal { - -void AddBasicAggKernels(KernelInit init, - const std::vector>& types, - std::shared_ptr out_ty, ScalarAggregateFunction* func, - SimdLevel::type simd_level = SimdLevel::NONE); - -void AddMinMaxKernels(KernelInit init, - const std::vector>& types, - ScalarAggregateFunction* func, - SimdLevel::type simd_level = SimdLevel::NONE); -void AddMinMaxKernel(KernelInit init, internal::detail::GetTypeId get_id, - ScalarAggregateFunction* func, - SimdLevel::type simd_level = SimdLevel::NONE); - -// SIMD variants for kernels -void AddSumAvx2AggKernels(ScalarAggregateFunction* func); -void AddMeanAvx2AggKernels(ScalarAggregateFunction* func); -void AddMinMaxAvx2AggKernels(ScalarAggregateFunction* func); - -void AddSumAvx512AggKernels(ScalarAggregateFunction* func); -void AddMeanAvx512AggKernels(ScalarAggregateFunction* func); -void AddMinMaxAvx512AggKernels(ScalarAggregateFunction* func); +namespace { // ---------------------------------------------------------------------- // Sum implementation @@ -1033,4 +1002,5 @@ struct MinMaxInitState { } }; +} // namespace } // namespace arrow::compute::internal From 79b01d55dd49c3988e752298bf1d2566a807a0fa Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Carvalho Date: Thu, 15 Aug 2024 18:25:40 -0300 Subject: [PATCH 03/16] Rename aggregate_basic_internal.inc.cc to aggregate_basic-inl.h --- ...ggregate_basic_internal.inc.cc => aggregate_basic-inl.h} | 6 +++--- cpp/src/arrow/compute/kernels/aggregate_basic.cc | 2 +- cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc | 2 +- cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) rename cpp/src/arrow/compute/kernels/{aggregate_basic_internal.inc.cc => aggregate_basic-inl.h} (99%) diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic_internal.inc.cc b/cpp/src/arrow/compute/kernels/aggregate_basic-inl.h similarity index 99% rename from cpp/src/arrow/compute/kernels/aggregate_basic_internal.inc.cc rename to cpp/src/arrow/compute/kernels/aggregate_basic-inl.h index cbe0bb72fde..7eaaf7836ff 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic_internal.inc.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic-inl.h @@ -15,12 +15,12 @@ // specific language governing permissions and limitations // under the License. -// .inc.cc file to be included in compilation unit where kernels are meant to -// be compiled auto-vectorized by the compiler with different SIMD levels passed +// -inl.h file to be included in compilation unit where kernels are meant to be +// compiled auto-vectorized by the compiler with different SIMD levels passed // as compiler flags. // // It contains no includes to avoid double inclusion in the compilation unit -// that includes this .inc.cc file. +// that includes this -inl.h file. namespace arrow::compute::internal { namespace { diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic.cc b/cpp/src/arrow/compute/kernels/aggregate_basic.cc index 1bd19ab8e7a..1358b0341ed 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic.cc @@ -36,7 +36,7 @@ // 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_internal.inc.cc" +#include "arrow/compute/kernels/aggregate_basic-inl.h" namespace arrow { namespace compute { diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc b/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc index 33c89a4182f..a597b0a0cf9 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic_avx2.cc @@ -35,7 +35,7 @@ // 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_internal.inc.cc" // XXX: fix +#include "arrow/compute/kernels/aggregate_basic-inl.h" namespace arrow { namespace compute { diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc b/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc index 9d598a2135e..544a2091e37 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc +++ b/cpp/src/arrow/compute/kernels/aggregate_basic_avx512.cc @@ -35,7 +35,7 @@ // 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_internal.inc.cc" +#include "arrow/compute/kernels/aggregate_basic-inl.h" namespace arrow { namespace compute { From 5fb32ce2f4bc16aa816d6edb6dc8972218fb749d Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Carvalho Date: Thu, 15 Aug 2024 19:51:28 -0300 Subject: [PATCH 04/16] Organize the kernels --- .../compute/kernels/aggregate_basic-inl.h | 215 +++++++++--------- .../arrow/compute/kernels/aggregate_basic.cc | 33 +-- .../compute/kernels/aggregate_basic_avx2.cc | 33 +-- .../compute/kernels/aggregate_basic_avx512.cc | 37 +-- .../kernels/aggregate_basic_internal.h | 7 +- 5 files changed, 171 insertions(+), 154 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/aggregate_basic-inl.h b/cpp/src/arrow/compute/kernels/aggregate_basic-inl.h index 7eaaf7836ff..37ab9a9e611 100644 --- a/cpp/src/arrow/compute/kernels/aggregate_basic-inl.h +++ b/cpp/src/arrow/compute/kernels/aggregate_basic-inl.h @@ -140,6 +140,56 @@ struct NullSumImpl : public NullImpl { } }; +template