hpy icon indicating copy to clipboard operation
hpy copied to clipboard

Use standard type ssize_t to define HPy_ssize_t.

Open fangerer opened this issue 2 years ago • 3 comments

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 avatar Apr 19 '23 06:04 fangerer

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.

steve-s avatar May 02 '23 08:05 steve-s

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.

fangerer avatar May 04 '23 11:05 fangerer

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.

steve-s avatar May 04 '23 11:05 steve-s