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

Use standard type ssize_t to define HPy_ssize_t. #426

Open
fangerer opened this issue Apr 19, 2023 · 3 comments
Open

Use standard type ssize_t to define HPy_ssize_t. #426

fangerer opened this issue Apr 19, 2023 · 3 comments
Assignees
Milestone

Comments

@fangerer
Copy link
Contributor

This is the follow-up issue of #343 .

We aim to use fixed-width integers in the ABI to improve compatibility and portability. However, HPy assumes in several and crucial places that sizeof(HPy_ssize_t) == sizeof(Py_ssize_t).

In February 2023's dev call, we decided to not use a fixed-width integer like int64_t for HPy_ssize_t now since it is a bit more involved.

Instead, we will use do typedef ssize_t HPy_ssize_t for now.

@fangerer fangerer added this to the ABI version 2 milestone Apr 19, 2023
@fangerer fangerer self-assigned this Apr 19, 2023
@steve-s
Copy link
Contributor

steve-s commented May 2, 2023

My main motivation for the original issue was that the ABI would be precisely specified including the bit width of all the arguments/return values/struct fields. Unfortunately, I don't remember exactly the discussion from the linked dev call, but just for the record: I think that switching HPy_ssize_t to ssize_t does not bring anything w.r.t. ABI stability. Maybe it has some other advantages? I want to make sure we do not purse something that does not solve the underlying motivation for using fixed size integers.

@fangerer
Copy link
Contributor Author

fangerer commented May 4, 2023

Sorry for the late answer. I saw your comment but forgot ...

I think that switching HPy_ssize_t to ssize_t does not bring anything w.r.t. ABI stability

That's correct. IIRC, Antonio suggested this just to get away from Py_ssize_t (kind of as a first step). And Py_ssize_t just exists because older compilers just don't have ssize_t (only size_t).

I still plan to do something like:

typedef int64_t HPy_ssize_t

The main problem really is the assumption sizeof(HPy_ssize_t) == sizeof(Py_ssize_t).
IMO, it is possible to use int64_t but if the assumption is then violated, we need to do proper conversions. I would assume that in most cases, the assumption still holds and there won't be any additional overhead but that just not guaranteed.

When converting, we also have the problem that we need to raise an error if the conversion fails (e.g. in case of an overflow). This needs to happen in the generated CPython trampolines which is a performance-critical place.

Btw. I already started to do all that. I will push the branch.

@steve-s
Copy link
Contributor

steve-s commented May 4, 2023

When converting, we also have the problem that we need to raise an error if the conversion fails (e.g. in case of an overflow).

Right, thinking about it: however maybe not all such functions can even raise now, so that would be API change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants