-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
MINOR: [C++][Acero] Replace acero-specific append avx2 cmake macro with generic one #44240
Conversation
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format?
or
In the case of PARQUET issues on JIRA the title also supports:
See also: |
I assume cmake-only changes can be a minor PR. If I'm wrong, please let me known and I will file an issue for it. Thanks. |
@github-actions crossbow submit -g cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, let's just ensure this doesn't break CI builds
Revision: 2f1d594 Submitted crossbow builds: ursacomputing/crossbow @ actions-cf5eb235b2 |
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit a576421. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
Macro
append_acero_runtime_avx2_src
is duplicated with genericappend_runtime_avx2_src
.What changes are included in this PR?
Remove
append_acero_runtime_avx2_src
and replace it withappend_runtime_avx2_src
.Are these changes tested?
CI should be enough.
Are there any user-facing changes?
None.