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

pyo3-ffi: expose PyObject_Vectorcall(Method) on the stable abi on 3.12+ #4853

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

Icxolu
Copy link
Contributor

@Icxolu Icxolu commented Jan 11, 2025

This exposes PyObject_Vectorcall(Method) on the stable abi on Python 3.12+ (extracted from #4379, with minor modification on PY_VECTORCALL_ARGUMENTS_OFFSET cfgs) and enables some of our vectorcall optimizations on the limited api.

@Icxolu Icxolu force-pushed the vectorcall branch 2 times, most recently from b7d6c64 to 2df776f Compare January 11, 2025 14:27
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks for extracting this out; I will try to rebase #4379 after this merges and assess how much is left / still needs to be split from there.

LGTM with one suggestion for a test...

src/types/tuple.rs Show resolved Hide resolved
@Icxolu Icxolu force-pushed the vectorcall branch 4 times, most recently from 76cafe3 to a742dd7 Compare January 14, 2025 21:12
@Icxolu Icxolu enabled auto-merge January 14, 2025 21:26
@Icxolu Icxolu added this pull request to the merge queue Jan 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 14, 2025
@Icxolu Icxolu added CI-build-full CI-no-fail-fast If one job fails, allow the rest to keep testing labels Jan 14, 2025
@Icxolu
Copy link
Contributor Author

Icxolu commented Jan 14, 2025

Hmm, looks like something does not work for Python 3.9 and 3.10, due to "undefined reference to PyObject_Vectorcall" (and on macos _PyObject_Vectorcall for some reason). Currently I'm not sure why. According to the docs it should be available from Python 3.9 and from 3.12 in the limited api.

@davidhewitt
Copy link
Member

Ugh, I think it was only exposed as a symbol from 3.11: python/cpython#28893

Before then it was a static inline function so the existing definition we had in cpython/abstract.rs was correct. We probably need to restore some of that code and adjust the cfgs.

(I guess this was my error from the original PR, sorry about that!)

@Icxolu
Copy link
Contributor Author

Icxolu commented Jan 15, 2025

Thanks for the hint! I adjusted the cfgs, lets see if this works.

@Icxolu Icxolu added this pull request to the merge queue Jan 15, 2025
Merged via the queue into PyO3:main with commit ad5f6d4 Jan 15, 2025
79 of 81 checks passed
@Icxolu Icxolu deleted the vectorcall branch January 15, 2025 19:06
@davidhewitt
Copy link
Member

Hooray! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-build-full CI-no-fail-fast If one job fails, allow the rest to keep testing performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants