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.

Sign up for GitHub

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

Jump to bottom

Compile cdef public functions from torch_allocator with C ABI #1350

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Compile cdef public functions from torch_allocator with C ABI #1350

merged 1 commit into from
Sep 26, 2023

Conversation

wence-
Copy link
Contributor

@wence- wence- commented Sep 26, 2023 •
edited
Loading

Description

Since Cython 3, we must explicitly request a C ABI with -DCYTHON_EXTERN_C='extern "C"' for cdef public functions.

Before:

$ nm -D .../rmm/python/rmm/_lib/torch_allocator.cpython-310-x86_64-linux-gnu.so | grep allocate
00000000000071c0 T _Z10deallocatePvlS_
0000000000007500 T _Z8allocateliPv
...

After:

$ nm -D .../rmm/python/rmm/_lib/torch_allocator.cpython-310-x86_64-linux-gnu.so | grep allocate
0000000000007500 T allocate
00000000000071c0 T deallocate
...

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@wence-
Compile cdef public functions from torch_allocator with C ABI
db184de 
Since Cython 3, we must explicitly request a C ABI with
`-DCYTHON_EXTERN_C='extern "C"'` for `cdef public` functions.

- Closes rapidsai#1349
@wence- wence- requested a review from a team as a code owner September 26, 2023 10:04
@github-actions github-actions bot added CMake Python Related to RMM Python API labels Sep 26, 2023
@shwina shwina added bug Something isn't working non-breaking Non-breaking change labels Sep 26, 2023
bdice
bdice approved these changes Sep 26, 2023
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

This looks like a proper triage and fix. Thanks @wence-!

@shwina
Copy link
Contributor

shwina commented Sep 26, 2023

/merge

@rapids-bot rapids-bot bot merged commit c8acf76 into rapidsai:branch-23.10 Sep 26, 2023
44 checks passed
@wence- wence- deleted the wence/fix/torch-allocator-abi branch September 26, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shwina shwina shwina approved these changes

@bdice bdice bdice approved these changes

Assignees
No one assigned
Labels
bug Something isn't working CMake non-breaking Non-breaking change Python Related to RMM Python API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Since move to Cython 3, torch_allocator cdef public functions have wrong ABI
3 participants
@wence- @shwina @bdice