Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
GH-101291: Rearrange the size bits in PyLongObject#102464
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
0ec07e4292b9d05c54894029aaa4b56e6da91269fcc48e825449c0e2c5ba6014b3a3e89ef9d2c9c408c1548d6563e3fefd391fb51df8c7d3bc14fa654c6f1bce6bfb24c1956b301158b1aa1891bf2a9af169f52190f9072f143443a0d661e145a2e4638a98f7f5acc0b06bb6fa19b0a787f49b2f764aa89843ac0d6cb917469d26fFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -82,8 +82,6 @@ PyObject *_PyLong_Add(PyLongObject *left, PyLongObject *right); | ||
| PyObject *_PyLong_Multiply(PyLongObject *left, PyLongObject *right); | ||
| PyObject *_PyLong_Subtract(PyLongObject *left, PyLongObject *right); | ||
| int _PyLong_AssignValue(PyObject **target, Py_ssize_t value); | ||
| /* Used by Python/mystrtoul.c, _PyBytes_FromHex(), | ||
| _PyBytes_DecodeEscape(), etc. */ | ||
| PyAPI_DATA(unsigned char) _PyLong_DigitValue[256]; | ||
| @@ -110,25 +108,155 @@ PyAPI_FUNC(char*) _PyLong_FormatBytesWriter( | ||
| int base, | ||
| int alternate); | ||
| /* Return 1 if the argument is positive single digit int */ | ||
| /* Long value tag bits: | ||
| * 0-1: Sign bits value = (1-sign), ie. negative=2, positive=0, zero=1. | ||
| * 2: Reserved for immortality bit | ||
Comment on lines
+112
to
+113
| ||
| * 3+ Unsigned digit count | ||
| */ | ||
| #define SIGN_MASK 3 | ||
| #define SIGN_ZERO 1 | ||
| #define SIGN_NEGATIVE 2 | ||
| #define NON_SIZE_BITS 3 | ||
| /* All *compact" values are guaranteed to fit into | ||
| * a Py_ssize_t with at least one bit to spare. | ||
| * In other words, for 64 bit machines, compact | ||
| * will be signed 63 (or fewer) bit values | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe also add that compact values have at most one digit? I've seen some code depending on that (e.g. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not with tagged ints. In theory a compact int could have 5 digits. (63 bit compact ints, and 15 bit digits). For a sensible implementation, a compact int will be one or two digits. | ||
| */ | ||
| /* Return 1 if the argument is compact int */ | ||
| static inline int | ||
| _PyLong_IsNonNegativeCompact(const PyLongObject* op) { | ||
| assert(PyLong_Check(op)); | ||
| return op->long_value.lv_tag <= (1 << NON_SIZE_BITS); | ||
gvanrossum marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page.
| ||
| } | ||
| static inline int | ||
| _PyLong_IsCompact(const PyLongObject* op) { | ||
| assert(PyLong_Check(op)); | ||
| return op->long_value.lv_tag < (2 << NON_SIZE_BITS); | ||
| } | ||
| static inline int | ||
| _PyLong_IsPositiveSingleDigit(PyObject* sub) { | ||
| /* For a positive single digit int, the value of Py_SIZE(sub) is 0 or 1. | ||
| We perform a fast check using a single comparison by casting from int | ||
| to uint which casts negative numbers to large positive numbers. | ||
| For details see Section 14.2 "Bounds Checking" in the Agner Fog | ||
| optimization manual found at: | ||
| https://www.agner.org/optimize/optimizing_cpp.pdf | ||
| The function is not affected by -fwrapv, -fno-wrapv and -ftrapv | ||
| compiler options of GCC and clang | ||
| */ | ||
| assert(PyLong_CheckExact(sub)); | ||
| Py_ssize_t signed_size = Py_SIZE(sub); | ||
| return ((size_t)signed_size) <= 1; | ||
| _PyLong_BothAreCompact(const PyLongObject* a, const PyLongObject* b) { | ||
| assert(PyLong_Check(a)); | ||
| assert(PyLong_Check(b)); | ||
| return (a->long_value.lv_tag | b->long_value.lv_tag) < (2 << NON_SIZE_BITS); | ||
| } | ||
| /* Returns a *compact* value, iff `_PyLong_IsCompact` is true for `op`. | ||
| * | ||
| * "Compact" values have at least one bit to spare, | ||
| * so that addition and subtraction can be performed on the values | ||
| * without risk of overflow. | ||
| */ | ||
| static inline Py_ssize_t | ||
| _PyLong_CompactValue(const PyLongObject *op) | ||
| { | ||
| assert(PyLong_Check(op)); | ||
| assert(_PyLong_IsCompact(op)); | ||
| Py_ssize_t sign = 1 - (op->long_value.lv_tag & SIGN_MASK); | ||
| return sign * (Py_ssize_t)op->long_value.ob_digit[0]; | ||
| } | ||
| static inline bool | ||
| _PyLong_IsZero(const PyLongObject *op) | ||
| { | ||
| return (op->long_value.lv_tag & SIGN_MASK) == SIGN_ZERO; | ||
| } | ||
| static inline bool | ||
| _PyLong_IsNegative(const PyLongObject *op) | ||
| { | ||
| return (op->long_value.lv_tag & SIGN_MASK) == SIGN_NEGATIVE; | ||
| } | ||
| static inline bool | ||
| _PyLong_IsPositive(const PyLongObject *op) | ||
| { | ||
| return (op->long_value.lv_tag & SIGN_MASK) == 0; | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not have MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want these functions to be the only way to determine the sign. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, fine. Next question: maybe we also need a | ||
| } | ||
| static inline Py_ssize_t | ||
| _PyLong_DigitCount(const PyLongObject *op) | ||
| { | ||
| assert(PyLong_Check(op)); | ||
| return op->long_value.lv_tag >> NON_SIZE_BITS; | ||
| } | ||
| /* Equivalent to _PyLong_DigitCount(op) * _PyLong_NonCompactSign(op) */ | ||
| static inline Py_ssize_t | ||
| _PyLong_SignedDigitCount(const PyLongObject *op) | ||
| { | ||
| assert(PyLong_Check(op)); | ||
| Py_ssize_t sign = 1 - (op->long_value.lv_tag & SIGN_MASK); | ||
| return sign * (Py_ssize_t)(op->long_value.lv_tag >> NON_SIZE_BITS); | ||
| } | ||
| static inline int | ||
| _PyLong_CompactSign(const PyLongObject *op) | ||
| { | ||
| assert(PyLong_Check(op)); | ||
| assert(_PyLong_IsCompact(op)); | ||
| return 1 - (op->long_value.lv_tag & SIGN_MASK); | ||
| } | ||
Comment on lines
+196
to
+203
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be the new implementation of Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They sure look identical to me. Maybe Mark has plans and maybe the compiler would optimize this anyway? could just become MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want the freedom to implement the "compact" and non-compact forms differently.
| ||
| static inline int | ||
| _PyLong_NonCompactSign(const PyLongObject *op) | ||
| { | ||
| assert(PyLong_Check(op)); | ||
| assert(!_PyLong_IsCompact(op)); | ||
| return 1 - (op->long_value.lv_tag & SIGN_MASK); | ||
| } | ||
| /* Do a and b have the same sign? */ | ||
| static inline int | ||
| _PyLong_SameSign(const PyLongObject *a, const PyLongObject *b) | ||
| { | ||
| return (a->long_value.lv_tag & SIGN_MASK) == (b->long_value.lv_tag & SIGN_MASK); | ||
| } | ||
| #define TAG_FROM_SIGN_AND_SIZE(sign, size) ((1 - (sign)) | ((size) << NON_SIZE_BITS)) | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I also haven't checked the assembly here, but I don't really know what happens when OR-ing a signed 64-bit int with a signed 32-bit int, and if this is doing work that's not strictly necessary. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is only in Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So maybe add a comment that this macro should only be used with literal or size_t arguments? | ||
| static inline void | ||
| _PyLong_SetSignAndDigitCount(PyLongObject *op, int sign, Py_ssize_t size) | ||
| { | ||
| assert(size >= 0); | ||
| assert(-1 <= sign && sign <= 1); | ||
| assert(sign != 0 || size == 0); | ||
| op->long_value.lv_tag = TAG_FROM_SIGN_AND_SIZE(sign, (size_t)size); | ||
| } | ||
| static inline void | ||
| _PyLong_SetDigitCount(PyLongObject *op, Py_ssize_t size) | ||
| { | ||
| assert(size >= 0); | ||
| op->long_value.lv_tag = (((size_t)size) << NON_SIZE_BITS) | (op->long_value.lv_tag & SIGN_MASK); | ||
| } | ||
| #define NON_SIZE_MASK ~((1 << NON_SIZE_BITS) - 1) | ||
| static inline void | ||
| _PyLong_FlipSign(PyLongObject *op) { | ||
| unsigned int flipped_sign = 2 - (op->long_value.lv_tag & SIGN_MASK); | ||
| op->long_value.lv_tag &= NON_SIZE_MASK; | ||
| op->long_value.lv_tag |= flipped_sign; | ||
| } | ||
| #define _PyLong_DIGIT_INIT(val) \ | ||
| { \ | ||
| .ob_base = _PyObject_IMMORTAL_INIT(&PyLong_Type), \ | ||
| .long_value = { \ | ||
| .lv_tag = TAG_FROM_SIGN_AND_SIZE( \ | ||
| (val) == 0 ? 0 : ((val) < 0 ? -1 : 1), \ | ||
| (val) == 0 ? 0 : 1), \ | ||
| { ((val) >= 0 ? (val) : -(val)) }, \ | ||
| } \ | ||
| } | ||
| #define _PyLong_FALSE_TAG TAG_FROM_SIGN_AND_SIZE(0, 0) | ||
| #define _PyLong_TRUE_TAG TAG_FROM_SIGN_AND_SIZE(1, 1) | ||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| Rearrage bits in first field (after header) of PyLongObject. | ||
| * Bits 0 and 1: 1 - sign. I.e. 0 for positive numbers, 1 for zero and 2 for negative numbers. | ||
| * Bit 2 reserved (probably for the immortal bit) | ||
| * Bits 3+ the unsigned size. | ||
| This makes a few operations slightly more efficient, and will enable a more | ||
| compact and faster 2s-complement representation of most ints in future. |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -32,6 +32,8 @@ Copyright (C) 1994 Steen Lumholt. | ||||||||||||
| # include "pycore_fileutils.h" // _Py_stat() | ||||||||||||
| #endif | ||||||||||||
| #include "pycore_long.h" | ||||||||||||
| #ifdef MS_WINDOWS | ||||||||||||
| #include <windows.h> | ||||||||||||
| #endif | ||||||||||||
| @@ -886,7 +888,8 @@ asBignumObj(PyObject *value) | ||||||||||||
| const char *hexchars; | ||||||||||||
| mp_int bigValue; | ||||||||||||
| neg = Py_SIZE(value) < 0; | ||||||||||||
| assert(PyLong_Check(value)); | ||||||||||||
| neg = _PyLong_IsNegative((PyLongObject *)value); | ||||||||||||
Comment on lines
+891
to
+892
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please put the blank line back.
Suggested change
| ||||||||||||
| hexstr = _PyLong_Format(value, 16); | ||||||||||||
| if (hexstr == NULL) | ||||||||||||
| return NULL; | ||||||||||||
| @@ -1950,7 +1953,7 @@ _tkinter_tkapp_getboolean(TkappObject *self, PyObject *arg) | ||||||||||||
| int v; | ||||||||||||
| if (PyLong_Check(arg)) { /* int or bool */ | ||||||||||||
| return PyBool_FromLong(Py_SIZE(arg) != 0); | ||||||||||||
| return PyBool_FromLong(!_PyLong_IsZero((PyLongObject *)arg)); | ||||||||||||
| } | ||||||||||||
| if (PyTclObject_Check(arg)) { | ||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't update this comment that documents
_longobject, it's still talking aboutob_sizeandPyVarObject