Skip to content

Conversation

@raulcd
Copy link
Member

@raulcd raulcd commented Apr 4, 2025

Rationale for this change

The code under arrow/gpu/ isn't included in libarrow.dll. We need to use different *_EXPORT macros for separated DLL.

What changes are included in this PR?

Create a new ARROW_CUDA_EXPORT macro to export cuda symbols.

Are these changes tested?

No new tests but should be tested via the cuda archery jobs.

Are there any user-facing changes?

No

@raulcd
Copy link
Member Author

raulcd commented Apr 4, 2025

@github-actions crossbow submit -g cuda

@github-actions
Copy link

github-actions bot commented Apr 4, 2025

Revision: 6a791cba771fa311205131724356a54a4307ae5e

Submitted crossbow builds: ursacomputing/crossbow @ actions-93a53001c3

Task Status
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-cuda-python-ubuntu-22.04-cuda-11.7.1 GitHub Actions

@raulcd raulcd marked this pull request as ready for review April 4, 2025 07:44
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

But could you merge this after #46006 is merged and add Cflags.private: -DARROW_CUDA_STATIC to arrow-cuda.pc.in?

Related changes for adding Cflags.private: -DARROW_CUDA_STATIC to arrow-cuda.pc.in:

https://github.com/apache/arrow/pull/46006/files#diff-1bba462ab050e89360fd88110a689e85ee037749cea091a1848ab574381d3795R549-R554

https://github.com/apache/arrow/pull/46006/files#diff-7b9288301707cebf803cc90e8b4404277196b7041708f0219e466dcc9de8bc73R27-R28

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Apr 4, 2025
@raulcd
Copy link
Member Author

raulcd commented Apr 4, 2025

I'll wait until we merge #46006 (and rebase) to add the other relevant CMake changes, and will update arrow-cuda.pc.in with the:

Cflags:@ARROW_CUDA_PC_CFLAGS@
Cflags.private:@ARROW_CUDA_PC_CFLAGS_PRIVATE@

@raulcd
Copy link
Member Author

raulcd commented Apr 7, 2025

@github-actions crossbow submit -g cuda

@github-actions
Copy link

github-actions bot commented Apr 7, 2025

Revision: 9aa1470

Submitted crossbow builds: ursacomputing/crossbow @ actions-2bef8745df

Task Status
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-cuda-python-ubuntu-22.04-cuda-11.7.1 GitHub Actions

@raulcd raulcd requested a review from kou April 7, 2025 11:53
@raulcd
Copy link
Member Author

raulcd commented Apr 7, 2025

@kou this should be ready for re-review

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@kou kou merged commit 770eaf4 into apache:main Apr 8, 2025
37 checks passed
@kou kou removed the awaiting merge Awaiting merge label Apr 8, 2025
@github-actions github-actions bot added the awaiting merge Awaiting merge label Apr 8, 2025
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 770eaf4.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.

zanmato1984 pushed a commit to zanmato1984/arrow that referenced this pull request Apr 15, 2025
…or libarrow_cuda (apache#46030)

### Rationale for this change

The code under `arrow/gpu/` isn't included in `libarrow.dll`. We need to use different `*_EXPORT` macros for separated DLL. 

### What changes are included in this PR?

Create a new `ARROW_CUDA_EXPORT` macro to export cuda symbols.

### Are these changes tested?

No new tests but should be tested via the cuda archery jobs.

### Are there any user-facing changes?

No
* GitHub Issue: apache#46025

Authored-by: Raúl Cumplido <raulcumplido@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants