Skip to content
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.

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

Re-evaluate Explicit SIMD Aggregations #5032

Closed
tustvold opened this issue Nov 2, 2023 · 4 comments · Fixed by #5100
Closed

Re-evaluate Explicit SIMD Aggregations #5032

tustvold opened this issue Nov 2, 2023 · 4 comments · Fixed by #5100
Labels
arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog help wanted

Comments

@tustvold
Copy link
Contributor

tustvold commented Nov 2, 2023

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

We currently only have explicit SIMD kernels for min, max and sum. This is a non-trivial additional complexity, and relies on features not present in stable Rust.

Describe the solution you'd like

Historically LLVM has struggled with auto-vectorisation of horizontal operations like aggregates, and this has been why we've kept these kernels, however, it is possible this has since improved. We should assess whether it is possible to eliminate the need for explicit SIMD kernels, and potentially remove them

Describe alternatives you've considered

Additional context

#4560 contains some experiments for improving the auto-vectorisation of aggregates

@tustvold tustvold added the enhancement Any new improvement worthy of a entry in the changelog label Nov 2, 2023
@tustvold tustvold changed the title Re-evaluate SIMD Aggregations Re-evaluate Explicit SIMD Aggregations Nov 2, 2023
@jhorstmann
Copy link
Contributor

I started working on this and #5031 and think I have found a way to generalize the approach from #4560 and get auto-vectorization also for MIN/MAX. Still need to clean up the code further and run more benchmarks.

@jhorstmann
Copy link
Contributor

@tustvold I must have missed or forgotten that the compare kernels and arithmetic div/mod also no longer use explicit simd. Just to confirm, all the methods in ArrowNumericType behind the simd feature would no longer be needed once aggregation stops using them?

@tustvold
Copy link
Contributor Author

Correct, it would definitely simplify things to no longer have nightly only functionality

@tustvold
Copy link
Contributor Author

tustvold commented Jan 5, 2024

label_issue.py automatically added labels {'arrow'} from #5100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate enhancement Any new improvement worthy of a entry in the changelog help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants