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

GH-35115: [C++] Moved util_avx2.cc from acero to compute #35117

Conversation

westonpace
Copy link
Member

@westonpace westonpace commented Apr 13, 2023

Rationale for this change

The file util_avx2.cc contains implementation for methods defined in src/arrow/compute/util.cc but it was placed in src/arrow/acero. This leads to undefined symbol errors when compiling against AVX2.

What changes are included in this PR?

The file util_avx2.cc is moved from acero to compute.

Are these changes tested?

Regrettably, no. See #35116

Are there any user-facing changes?

No.

@github-actions
Copy link

@github-actions
Copy link

⚠️ GitHub issue #35115 has been automatically assigned in GitHub to PR creator.

@westonpace westonpace merged commit ae633bc into apache:main Apr 18, 2023
@westonpace westonpace added the Priority: Blocker Marks a blocker for the release label Apr 18, 2023
@westonpace westonpace added this to the 12.0.0 milestone Apr 18, 2023
@icexelloss
Copy link
Contributor

Sorry missed this - LGTM

raulcd pushed a commit that referenced this pull request Apr 19, 2023
### Rationale for this change

The file util_avx2.cc contains implementation for methods defined in `src/arrow/compute/util.cc` but it was placed in `src/arrow/acero`.  This leads to undefined symbol errors when compiling against AVX2.

### What changes are included in this PR?

The file util_avx2.cc is moved from `acero` to `compute`.

### Are these changes tested?

Regrettably, no.  See #35116 

### Are there any user-facing changes?

No.
* Closes: #35115

Authored-by: Weston Pace <weston.pace@gmail.com>
Signed-off-by: Weston Pace <weston.pace@gmail.com>
@ursabot
Copy link

ursabot commented Apr 19, 2023

Benchmark runs are scheduled for baseline = 9824ffc and contender = ae633bc. ae633bc is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed] test-mac-arm
[Finished ⬇️5.61% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.66% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] ae633bc0 ec2-t3-xlarge-us-east-2
[Failed] ae633bc0 test-mac-arm
[Finished] ae633bc0 ursa-i9-9960x
[Finished] ae633bc0 ursa-thinkcentre-m75q
[Finished] 9824ffcd ec2-t3-xlarge-us-east-2
[Failed] 9824ffcd test-mac-arm
[Finished] 9824ffcd ursa-i9-9960x
[Finished] 9824ffcd ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@ursabot
Copy link

ursabot commented Apr 19, 2023

['Python', 'R'] benchmarks have high level of regressions.
ursa-i9-9960x

liujiacheng777 pushed a commit to LoongArch-Python/arrow that referenced this pull request May 11, 2023
…e#35117)

### Rationale for this change

The file util_avx2.cc contains implementation for methods defined in `src/arrow/compute/util.cc` but it was placed in `src/arrow/acero`.  This leads to undefined symbol errors when compiling against AVX2.

### What changes are included in this PR?

The file util_avx2.cc is moved from `acero` to `compute`.

### Are these changes tested?

Regrettably, no.  See apache#35116 

### Are there any user-facing changes?

No.
* Closes: apache#35115

Authored-by: Weston Pace <weston.pace@gmail.com>
Signed-off-by: Weston Pace <weston.pace@gmail.com>
ArgusLi pushed a commit to Bit-Quill/arrow that referenced this pull request May 15, 2023
…e#35117)

### Rationale for this change

The file util_avx2.cc contains implementation for methods defined in `src/arrow/compute/util.cc` but it was placed in `src/arrow/acero`.  This leads to undefined symbol errors when compiling against AVX2.

### What changes are included in this PR?

The file util_avx2.cc is moved from `acero` to `compute`.

### Are these changes tested?

Regrettably, no.  See apache#35116 

### Are there any user-facing changes?

No.
* Closes: apache#35115

Authored-by: Weston Pace <weston.pace@gmail.com>
Signed-off-by: Weston Pace <weston.pace@gmail.com>
rtpsw pushed a commit to rtpsw/arrow that referenced this pull request May 16, 2023
…e#35117)

### Rationale for this change

The file util_avx2.cc contains implementation for methods defined in `src/arrow/compute/util.cc` but it was placed in `src/arrow/acero`.  This leads to undefined symbol errors when compiling against AVX2.

### What changes are included in this PR?

The file util_avx2.cc is moved from `acero` to `compute`.

### Are these changes tested?

Regrettably, no.  See apache#35116 

### Are there any user-facing changes?

No.
* Closes: apache#35115

Authored-by: Weston Pace <weston.pace@gmail.com>
Signed-off-by: Weston Pace <weston.pace@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting committer review Awaiting committer review Component: C++ Priority: Blocker Marks a blocker for the release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[C++] util_avx2.cc is in acero but should be in compute
3 participants