From f7f031f1d707ac685663e5a6e2874f8972e5b174 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 6 Jul 2024 10:01:15 +0300 Subject: [PATCH 1/6] gh-102471: convert decimal module to use PyLong_Export API (PEP 757) --- Modules/_decimal/_decimal.c | 55 +++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index c564813036e504..7df4b0c8b4fc74 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -30,7 +30,6 @@ #endif #include -#include "pycore_long.h" // _PyLong_IsZero() #include "pycore_pystate.h" // _PyThreadState_GET() #include "pycore_typeobject.h" #include "complexobject.h" @@ -2323,38 +2322,46 @@ static PyObject * dec_from_long(decimal_state *state, PyTypeObject *type, PyObject *v, const mpd_context_t *ctx, uint32_t *status) { - PyObject *dec; - PyLongObject *l = (PyLongObject *)v; + PyObject *dec = PyDecType_New(state, type); - dec = PyDecType_New(state, type); if (dec == NULL) { return NULL; } - if (_PyLong_IsZero(l)) { - _dec_settriple(dec, MPD_POS, 0, 0); - return dec; - } - - uint8_t sign = _PyLong_IsNegative(l) ? MPD_NEG : MPD_POS; + PyLongExport export_long; - if (_PyLong_IsCompact(l)) { - _dec_settriple(dec, sign, l->long_value.ob_digit[0], 0); - mpd_qfinalize(MPD(dec), ctx, status); - return dec; + if (PyLong_Export(v, &export_long) == -1) { + Py_DECREF(dec); + return NULL; } - size_t len = _PyLong_DigitCount(l); + if (export_long.digits) { + const PyLongLayout *layout = PyLong_GetNativeLayout(); + const uint32_t base = (uint32_t)1 << layout->bits_per_digit; + const uint8_t sign = export_long.negative ? MPD_NEG : MPD_POS; + const Py_ssize_t len = export_long.ndigits; -#if PYLONG_BITS_IN_DIGIT == 30 - mpd_qimport_u32(MPD(dec), l->long_value.ob_digit, len, sign, PyLong_BASE, - ctx, status); -#elif PYLONG_BITS_IN_DIGIT == 15 - mpd_qimport_u16(MPD(dec), l->long_value.ob_digit, len, sign, PyLong_BASE, - ctx, status); -#else - #error "PYLONG_BITS_IN_DIGIT should be 15 or 30" -#endif + if (base > UINT16_MAX) { + mpd_qimport_u32(MPD(dec), export_long.digits, len, sign, + base, ctx, status); + } + else { + mpd_qimport_u16(MPD(dec), export_long.digits, len, sign, + base, ctx, status); + } + PyLong_FreeExport(&export_long); + } + else { + const int64_t value = export_long.value; + if (-(int64_t)UINT32_MAX <= value && value <= (int64_t)UINT32_MAX) { + _dec_settriple(dec, value < 0 ? MPD_NEG : MPD_POS, + (uint32_t)Py_ABS(value), 0); + mpd_qfinalize(MPD(dec), ctx, status); + } + else { + mpd_qset_i64(MPD(dec), value, ctx, status); + } + } return dec; } From bf687e5e3d278294be4425b7cde7e54a3406e533 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Thu, 26 Dec 2024 08:46:46 +0300 Subject: [PATCH 2/6] XXX try PyLong_Export without value --- Modules/_decimal/_decimal.c | 10 +++++++++- Objects/longobject.c | 39 +++++++++---------------------------- 2 files changed, 18 insertions(+), 31 deletions(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 7df4b0c8b4fc74..1b8ded554f598a 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -2335,9 +2335,17 @@ dec_from_long(decimal_state *state, PyTypeObject *type, PyObject *v, return NULL; } if (export_long.digits) { + const uint8_t sign = export_long.negative ? MPD_NEG : MPD_POS; + + if (PyUnstable_Long_IsCompact((PyLongObject *)v)) { + _dec_settriple(dec, sign, ((uint32_t *)export_long.digits)[0], 0); + mpd_qfinalize(MPD(dec), ctx, status); + PyLong_FreeExport(&export_long); + return dec; + } + const PyLongLayout *layout = PyLong_GetNativeLayout(); const uint32_t base = (uint32_t)1 << layout->bits_per_digit; - const uint8_t sign = export_long.negative ? MPD_NEG : MPD_POS; const Py_ssize_t len = export_long.ndigits; if (base > UINT16_MAX) { diff --git a/Objects/longobject.c b/Objects/longobject.c index bd7ff68d0899c6..c64eb4079b81d7 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -6842,36 +6842,15 @@ PyLong_Export(PyObject *obj, PyLongExport *export_long) return -1; } - // Fast-path: try to convert to a int64_t - int overflow; -#if SIZEOF_LONG == 8 - long value = PyLong_AsLongAndOverflow(obj, &overflow); -#else - // Windows has 32-bit long, so use 64-bit long long instead - long long value = PyLong_AsLongLongAndOverflow(obj, &overflow); -#endif - Py_BUILD_ASSERT(sizeof(value) == sizeof(int64_t)); - // the function cannot fail since obj is a PyLongObject - assert(!(value == -1 && PyErr_Occurred())); - - if (!overflow) { - export_long->value = value; - export_long->negative = 0; - export_long->ndigits = 0; - export_long->digits = NULL; - export_long->_reserved = 0; - } - else { - PyLongObject *self = (PyLongObject*)obj; - export_long->value = 0; - export_long->negative = _PyLong_IsNegative(self); - export_long->ndigits = _PyLong_DigitCount(self); - if (export_long->ndigits == 0) { - export_long->ndigits = 1; - } - export_long->digits = self->long_value.ob_digit; - export_long->_reserved = (Py_uintptr_t)Py_NewRef(obj); - } + PyLongObject *self = (PyLongObject*)obj; + export_long->value = 0; + export_long->negative = _PyLong_IsNegative(self); + export_long->ndigits = _PyLong_DigitCount(self); + if (export_long->ndigits == 0) { + export_long->ndigits = 1; + } + export_long->digits = self->long_value.ob_digit; + export_long->_reserved = (Py_uintptr_t)Py_NewRef(obj); return 0; } From 021ab8ec834efe39d684044026214b2da336e65e Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Thu, 26 Dec 2024 08:47:34 +0300 Subject: [PATCH 3/6] Revert "XXX try PyLong_Export without value" This reverts commit bf687e5e3d278294be4425b7cde7e54a3406e533. --- Modules/_decimal/_decimal.c | 10 +--------- Objects/longobject.c | 39 ++++++++++++++++++++++++++++--------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 1b8ded554f598a..7df4b0c8b4fc74 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -2335,17 +2335,9 @@ dec_from_long(decimal_state *state, PyTypeObject *type, PyObject *v, return NULL; } if (export_long.digits) { - const uint8_t sign = export_long.negative ? MPD_NEG : MPD_POS; - - if (PyUnstable_Long_IsCompact((PyLongObject *)v)) { - _dec_settriple(dec, sign, ((uint32_t *)export_long.digits)[0], 0); - mpd_qfinalize(MPD(dec), ctx, status); - PyLong_FreeExport(&export_long); - return dec; - } - const PyLongLayout *layout = PyLong_GetNativeLayout(); const uint32_t base = (uint32_t)1 << layout->bits_per_digit; + const uint8_t sign = export_long.negative ? MPD_NEG : MPD_POS; const Py_ssize_t len = export_long.ndigits; if (base > UINT16_MAX) { diff --git a/Objects/longobject.c b/Objects/longobject.c index c64eb4079b81d7..bd7ff68d0899c6 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -6842,15 +6842,36 @@ PyLong_Export(PyObject *obj, PyLongExport *export_long) return -1; } - PyLongObject *self = (PyLongObject*)obj; - export_long->value = 0; - export_long->negative = _PyLong_IsNegative(self); - export_long->ndigits = _PyLong_DigitCount(self); - if (export_long->ndigits == 0) { - export_long->ndigits = 1; - } - export_long->digits = self->long_value.ob_digit; - export_long->_reserved = (Py_uintptr_t)Py_NewRef(obj); + // Fast-path: try to convert to a int64_t + int overflow; +#if SIZEOF_LONG == 8 + long value = PyLong_AsLongAndOverflow(obj, &overflow); +#else + // Windows has 32-bit long, so use 64-bit long long instead + long long value = PyLong_AsLongLongAndOverflow(obj, &overflow); +#endif + Py_BUILD_ASSERT(sizeof(value) == sizeof(int64_t)); + // the function cannot fail since obj is a PyLongObject + assert(!(value == -1 && PyErr_Occurred())); + + if (!overflow) { + export_long->value = value; + export_long->negative = 0; + export_long->ndigits = 0; + export_long->digits = NULL; + export_long->_reserved = 0; + } + else { + PyLongObject *self = (PyLongObject*)obj; + export_long->value = 0; + export_long->negative = _PyLong_IsNegative(self); + export_long->ndigits = _PyLong_DigitCount(self); + if (export_long->ndigits == 0) { + export_long->ndigits = 1; + } + export_long->digits = self->long_value.ob_digit; + export_long->_reserved = (Py_uintptr_t)Py_NewRef(obj); + } return 0; } From 5764614b14ebae65dddb16c156ef4e9afdffcbca Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Thu, 26 Dec 2024 15:09:21 +0300 Subject: [PATCH 4/6] address review: add asserts --- Modules/_decimal/_decimal.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 7df4b0c8b4fc74..4217da0f875ff0 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -2340,7 +2340,12 @@ dec_from_long(decimal_state *state, PyTypeObject *type, PyObject *v, const uint8_t sign = export_long.negative ? MPD_NEG : MPD_POS; const Py_ssize_t len = export_long.ndigits; - if (base > UINT16_MAX) { + assert(layout->bits_per_digit <= 32); + assert(layout->digits_order == -1); + assert(layout->digit_endianness == (PY_LITTLE_ENDIAN ? -1 : 1)); + assert(layout->digit_size == 2 || layout->digit_size == 4); + + if (layout->digit_size == 4) { mpd_qimport_u32(MPD(dec), export_long.digits, len, sign, base, ctx, status); } From 3381ddcd29c9c0107a7d720e3b90135a502461b5 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sun, 5 Jan 2025 12:37:08 +0300 Subject: [PATCH 5/6] address review: drop const (seems wrong for me) --- Modules/_decimal/_decimal.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 4217da0f875ff0..7b135b12819d7f 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -2336,9 +2336,9 @@ dec_from_long(decimal_state *state, PyTypeObject *type, PyObject *v, } if (export_long.digits) { const PyLongLayout *layout = PyLong_GetNativeLayout(); - const uint32_t base = (uint32_t)1 << layout->bits_per_digit; - const uint8_t sign = export_long.negative ? MPD_NEG : MPD_POS; - const Py_ssize_t len = export_long.ndigits; + uint32_t base = (uint32_t)1 << layout->bits_per_digit; + uint8_t sign = export_long.negative ? MPD_NEG : MPD_POS; + Py_ssize_t len = export_long.ndigits; assert(layout->bits_per_digit <= 32); assert(layout->digits_order == -1); @@ -2356,7 +2356,7 @@ dec_from_long(decimal_state *state, PyTypeObject *type, PyObject *v, PyLong_FreeExport(&export_long); } else { - const int64_t value = export_long.value; + int64_t value = export_long.value; if (-(int64_t)UINT32_MAX <= value && value <= (int64_t)UINT32_MAX) { _dec_settriple(dec, value < 0 ? MPD_NEG : MPD_POS, From 3eb506f893f3f5924b40ab3c4ace945628c9e213 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sun, 5 Jan 2025 13:13:48 +0300 Subject: [PATCH 6/6] Address review: just mpd_qset_i64 --- Modules/_decimal/_decimal.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/Modules/_decimal/_decimal.c b/Modules/_decimal/_decimal.c index 7b135b12819d7f..0def463c7d8b9e 100644 --- a/Modules/_decimal/_decimal.c +++ b/Modules/_decimal/_decimal.c @@ -2356,16 +2356,7 @@ dec_from_long(decimal_state *state, PyTypeObject *type, PyObject *v, PyLong_FreeExport(&export_long); } else { - int64_t value = export_long.value; - - if (-(int64_t)UINT32_MAX <= value && value <= (int64_t)UINT32_MAX) { - _dec_settriple(dec, value < 0 ? MPD_NEG : MPD_POS, - (uint32_t)Py_ABS(value), 0); - mpd_qfinalize(MPD(dec), ctx, status); - } - else { - mpd_qset_i64(MPD(dec), value, ctx, status); - } + mpd_qset_i64(MPD(dec), export_long.value, ctx, status); } return dec; }