Skip to content

GH-55312: [C++][Compute] More flexible kernel exec generation - #55291

Open
pitrou wants to merge 1 commit into
apache:mainfrom
pitrou:kernel-generator
Open

GH-55312: [C++][Compute] More flexible kernel exec generation#55291
pitrou wants to merge 1 commit into
apache:mainfrom
pitrou:kernel-generator

Conversation

@pitrou

@pitrou pitrou commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Rationale for this change

Our kernel generation utilities in arrow/compute/kernels/codegen_internal.h are a hodge-podge of hand-written switch statements with inconsistent template parameter signatures and semantics.

This PR makes those more flexible, more consistent and more reusable.

What changes are included in this PR?

  • Rewrite kernel generation utilities to make them more generic, allowing to pass compile-time arrays of type ids
  • Add compile-time constant arrays of type ids (kAllTypeIds, kIntegerTypeIds...)
  • Add missing TypeIdTraits specializations
  • Add missing type_id static member definitions on RunEndEncodedType and ExtensionType
  • Reduce code duplication in numeric cast kernel generation

Are these changes tested?

Yes, by existing tests and new unit tests around type id traits.

Are there any user-facing changes?

No.

Was AI used for this PR?

In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.

PR code and description written by:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

@pitrou
pitrou marked this pull request as draft October 27, 2026 19:25
@pitrou

pitrou commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

@zanmato1984 This is what I had in mind in #55169 (comment)

@pitrou pitrou changed the title WIP: [C++] More flexible kernel exec generation EXP: [C++] More flexible kernel exec generation Oct 27, 2026
@pitrou
pitrou force-pushed the kernel-generator branch 6 times, most recently from 6b9e004 to 4eda8a8 Compare October 29, 2026 14:13
@pitrou
pitrou marked this pull request as ready for review October 29, 2026 14:13
@pitrou pitrou changed the title EXP: [C++] More flexible kernel exec generation GH-55312: [C++][Compute] More flexible kernel exec generation Oct 29, 2026
@pitrou

pitrou commented Oct 29, 2026

Copy link
Copy Markdown
Member Author

This is ready for review @zanmato1984 @HuaHuaY .

@zanmato1984 zanmato1984 mentioned this pull request Oct 30, 2026
Open
3 of 5 tasks
zanmato1984
zanmato1984 approved these changes Oct 30, 2026
View reviewed changes

@zanmato1984 zanmato1984 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the cleanup! The new generator interface looks good to me. I left two minor documentation comments inline.

Comment thread cpp/src/arrow/compute/kernels/codegen_internal.h
Comment on lines +1021 to +1022
// For example, GenerateNumeric<FUNCTOR>(int32()) will select a kernel
// instantiated like FUNCTOR<Int32Type>. Any additional variadic

@zanmato1984 zanmato1984 Oct 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: could we also update the overview above? It still says a fixed Type0 argument is required and precedes the selected type, whereas the default is now FUNCTOR<Type, Args...>. It would also help to mention WithType0<T> and WithFirstTypeTwice for the alternative layouts.

Comment thread cpp/src/arrow/compute/kernels/aggregate_quantile.cc
@@ -531,14 +531,14 @@ void AddQuantileKernels(VectorFunction* func) {
for (const auto& ty : NumericTypes()) {
base.signature = KernelSignature::Make({InputType(ty)}, OutputType(ResolveOutput));
// output type is determined at runtime, set template argument to nulltype

@zanmato1984 zanmato1984 Oct 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the nulltype placeholder is no longer passed here. Could we simplify this comment to “The output type is determined at runtime by ResolveOutput”?

@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zanmato1984 zanmato1984 zanmato1984 approved these changes

Assignees

No one assigned

Labels

awaiting committer review Awaiting committer review Component: C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pitrou @zanmato1984