From 9bf9dcdd3342e9e770de23439065d46e07401a5c Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 27 Dec 2021 22:12:05 +0000 Subject: [PATCH 01/14] bpo-46055: Speed up right shifts of negative integers --- Lib/test/test_long.py | 4 +++ Objects/longobject.c | 71 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_long.py b/Lib/test/test_long.py index 3c8e9e22e17a19..0ac2b2d5a46818 100644 --- a/Lib/test/test_long.py +++ b/Lib/test/test_long.py @@ -984,6 +984,10 @@ def test_medium_rshift(self): self.assertEqual((-1122) >> 9, -3) self.assertEqual(2**128 >> 9, 2**119) self.assertEqual(-2**128 >> 9, -2**119) + # Exercise corner case of the current algorithm, where the result of + # shifting a two-limb int by the limb size still has two limbs. + self.assertEqual((1 - BASE*BASE) >> SHIFT, -BASE) + self.assertEqual((BASE - 1 - BASE*BASE) >> SHIFT, -BASE) def test_big_rshift(self): self.assertEqual(42 >> 32, 0) diff --git a/Objects/longobject.c b/Objects/longobject.c index 09ae9455c5b269..e7b5705758b279 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4492,6 +4492,12 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) PyLongObject *z = NULL; Py_ssize_t newsize, hishift, i, j; twodigits accum; + digit sticky; + + /* Special-case a shift of zero. */ + if (wordshift == 0 && remshift == 0) { + return long_long((PyObject *)a); + } if (IS_MEDIUM_VALUE(a)) { stwodigits m, x; @@ -4503,17 +4509,60 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) } if (Py_SIZE(a) < 0) { - /* Right shifting negative numbers is harder */ - PyLongObject *a1, *a2; - a1 = (PyLongObject *) long_invert(a); - if (a1 == NULL) - return NULL; - a2 = (PyLongObject *) long_rshift1(a1, wordshift, remshift); - Py_DECREF(a1); - if (a2 == NULL) + /* + Right shifting negative numbers is harder. For a positive + integer a and nonnegative shift, we have: + + (-a) >> shift == -((a + 2**shift - 1) >> shift). + + With shift == wordshift*SHIFT + remshift, 0 <= remshift <= SHIFT, + (a + 2**shift - 1) >> shift is equal to + + (a + 2**shift - 1) >> wordshift*SHIFT >> remshift. + + If the bottom `wordshift` digits of a are all zero, this is the + equal to + + ((a >> wordshift*SHIFT) + 2**remshift - 1) >> remshift. + + Otherwise, it's equal to: + + ((a >> wordshift*SHIFT) + 2**remshift) >> remshift + */ + + /* It's convenient for remshift to be positive below. Note that + we dealt with the case remshift == wordshift == 0 earlier. */ + if (remshift == 0) { + remshift = PyLong_SHIFT; + wordshift -= 1; + } + assert(wordshift >= 0); + + newsize = -Py_SIZE(a) - wordshift; + if (newsize <= 0) { + return PyLong_FromLong(-1); + } + hishift = PyLong_SHIFT - remshift; + z = _PyLong_New(newsize); + if (z == NULL) { return NULL; - z = (PyLongObject *) long_invert(a2); - Py_DECREF(a2); + } + Py_SET_SIZE(z, -newsize); + + /* sticky is used to determine whether all dropped words are zero. */ + sticky = 0; + for (j = 0; j < wordshift; j++) { + sticky |= a->ob_digit[j]; + } + accum = a->ob_digit[j++] + ((digit)1U << remshift) - (sticky == 0); + accum >>= remshift; + for (i = 0; j < -Py_SIZE(a); i++, j++) { + accum += (twodigits)a->ob_digit[j] << hishift; + z->ob_digit[i] = (digit)(accum & PyLong_MASK); + accum >>= PyLong_SHIFT; + } + assert(i == newsize - 1); + z->ob_digit[i] = (digit)accum; } else { newsize = Py_SIZE(a) - wordshift; @@ -4531,8 +4580,8 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) accum >>= PyLong_SHIFT; } z->ob_digit[i] = (digit)accum; - z = maybe_small_long(long_normalize(z)); } + z = maybe_small_long(long_normalize(z)); return (PyObject *)z; } From ad69e48167b2719a5435a5915a57357a24f9add7 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 28 Dec 2021 12:30:57 +0000 Subject: [PATCH 02/14] Rework to remove duplication and move the shift==0 check earlier. --- Objects/longobject.c | 121 +++++++++++++++++++++---------------------- 1 file changed, 58 insertions(+), 63 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index e7b5705758b279..227796c80e3ef9 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4486,19 +4486,20 @@ divmod_shift(PyObject *shiftby, Py_ssize_t *wordshift, digit *remshift) return 0; } +/* Inner function for both long_rshift and _PyLong_Rshift, shifting an + integer right by a strictly positive shift. */ + static PyObject * long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) { PyLongObject *z = NULL; - Py_ssize_t newsize, hishift, i, j; + Py_ssize_t newsize, hishift, i, j, size_a; twodigits accum; - digit sticky; + int a_negative; - /* Special-case a shift of zero. */ - if (wordshift == 0 && remshift == 0) { - return long_long((PyObject *)a); - } + assert (wordshift > 0 || remshift > 0); + /* Fast path for small a. */ if (IS_MEDIUM_VALUE(a)) { stwodigits m, x; digit shift; @@ -4508,79 +4509,65 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) return _PyLong_FromSTwoDigits(x); } - if (Py_SIZE(a) < 0) { - /* - Right shifting negative numbers is harder. For a positive - integer a and nonnegative shift, we have: - - (-a) >> shift == -((a + 2**shift - 1) >> shift). - - With shift == wordshift*SHIFT + remshift, 0 <= remshift <= SHIFT, - (a + 2**shift - 1) >> shift is equal to - - (a + 2**shift - 1) >> wordshift*SHIFT >> remshift. - - If the bottom `wordshift` digits of a are all zero, this is the - equal to - - ((a >> wordshift*SHIFT) + 2**remshift - 1) >> remshift. - - Otherwise, it's equal to: - - ((a >> wordshift*SHIFT) + 2**remshift) >> remshift - */ + a_negative = Py_SIZE(a) < 0; + size_a = Py_ABS(Py_SIZE(a)); - /* It's convenient for remshift to be positive below. Note that - we dealt with the case remshift == wordshift == 0 earlier. */ + if (a_negative) { + /* For shifting negative integers, it's convenient to adjust so that + 0 < remshift <= PyLong_SHIFT. */ if (remshift == 0) { remshift = PyLong_SHIFT; wordshift -= 1; } assert(wordshift >= 0); + } - newsize = -Py_SIZE(a) - wordshift; - if (newsize <= 0) { - return PyLong_FromLong(-1); - } - hishift = PyLong_SHIFT - remshift; - z = _PyLong_New(newsize); - if (z == NULL) { - return NULL; - } + newsize = size_a - wordshift; + if (newsize <= 0) { + return PyLong_FromLong(-a_negative); + } + z = _PyLong_New(newsize); + if (z == NULL) { + return NULL; + } + hishift = PyLong_SHIFT - remshift; + + if (a_negative) { + /* + For a positive integer a and nonnegative shift, we have: + + (-a) >> shift == -((a + 2**shift - 1) >> shift). + + In the addition `a + (2**shift - 1)`, the low `wordshift` digits of + `2**shift - 1` all have value `PyLong_MASK`, so we get a carry out + from the bottom `wordshift` digits when at least one of the least + significant `wordshift` digits of `a` is nonzero. Digit `wordshift` + of `2**shift - 1` has value `PyLong_MASK >> hishift`. + */ Py_SET_SIZE(z, -newsize); - /* sticky is used to determine whether all dropped words are zero. */ - sticky = 0; + digit sticky = 0; for (j = 0; j < wordshift; j++) { sticky |= a->ob_digit[j]; } - accum = a->ob_digit[j++] + ((digit)1U << remshift) - (sticky == 0); - accum >>= remshift; - for (i = 0; j < -Py_SIZE(a); i++, j++) { - accum += (twodigits)a->ob_digit[j] << hishift; - z->ob_digit[i] = (digit)(accum & PyLong_MASK); - accum >>= PyLong_SHIFT; - } - assert(i == newsize - 1); - z->ob_digit[i] = (digit)accum; + accum = a->ob_digit[j++] + (PyLong_MASK >> hishift) + + (digit)(sticky != 0); } else { - newsize = Py_SIZE(a) - wordshift; - if (newsize <= 0) - return PyLong_FromLong(0); - hishift = PyLong_SHIFT - remshift; - z = _PyLong_New(newsize); - if (z == NULL) - return NULL; j = wordshift; - accum = a->ob_digit[j++] >> remshift; - for (i = 0; j < Py_SIZE(a); i++, j++) { - accum |= (twodigits)a->ob_digit[j] << hishift; - z->ob_digit[i] = (digit)(accum & PyLong_MASK); - accum >>= PyLong_SHIFT; - } - z->ob_digit[i] = (digit)accum; + accum = a->ob_digit[j++]; } + + accum >>= remshift; + assert(j == wordshift+1); + for (i = 0; j < size_a; i++, j++) { + accum += (twodigits)a->ob_digit[j] << hishift; + z->ob_digit[i] = (digit)(accum & PyLong_MASK); + accum >>= PyLong_SHIFT; + } + assert(i == newsize - 1); + assert(accum <= PyLong_MASK); + z->ob_digit[i] = (digit)accum; z = maybe_small_long(long_normalize(z)); return (PyObject *)z; } @@ -4597,6 +4584,9 @@ long_rshift(PyObject *a, PyObject *b) PyErr_SetString(PyExc_ValueError, "negative shift count"); return NULL; } + if (Py_SIZE(b) == 0) { + return long_long(a); + } if (Py_SIZE(a) == 0) { return PyLong_FromLong(0); } @@ -4613,9 +4603,14 @@ _PyLong_Rshift(PyObject *a, size_t shiftby) digit remshift; assert(PyLong_Check(a)); + + if (shiftby == 0) { + return long_long(a); + } if (Py_SIZE(a) == 0) { return PyLong_FromLong(0); } + wordshift = shiftby / PyLong_SHIFT; remshift = shiftby % PyLong_SHIFT; return long_rshift1((PyLongObject *)a, wordshift, remshift); From cf7de3b61ce7ef946a8ff5a067dead6dd1ff61cb Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 28 Dec 2021 12:37:14 +0000 Subject: [PATCH 03/14] Update NEWS.d entry --- .../2021-12-24-20-21-45.bpo-46055.R0QMVQ.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-12-24-20-21-45.bpo-46055.R0QMVQ.rst b/Misc/NEWS.d/next/Core and Builtins/2021-12-24-20-21-45.bpo-46055.R0QMVQ.rst index 124138806f17d0..e2bf46c6830771 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2021-12-24-20-21-45.bpo-46055.R0QMVQ.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2021-12-24-20-21-45.bpo-46055.R0QMVQ.rst @@ -1,2 +1,3 @@ -Speed up shifting operation involving integers less than -:c:macro:`PyLong_BASE`. Patch by Xinhang Xu. +Speed up integer shifts: faster left and right shifts of integers smaller than +:c:macro:`PyLong_BASE`, and faster right shifts in the general case. Patches by +Xinhang Xu and Mark Dickinson. From 41de654f314beec029806b57e5d3de9834509281 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 28 Dec 2021 12:42:24 +0000 Subject: [PATCH 04/14] Revert unrelated spacing change --- Objects/longobject.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 227796c80e3ef9..4c4b3c176c7701 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4610,7 +4610,6 @@ _PyLong_Rshift(PyObject *a, size_t shiftby) if (Py_SIZE(a) == 0) { return PyLong_FromLong(0); } - wordshift = shiftby / PyLong_SHIFT; remshift = shiftby % PyLong_SHIFT; return long_rshift1((PyLongObject *)a, wordshift, remshift); From 27843b180f4267667a5639c30a7430e1d8b3644a Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 28 Dec 2021 12:53:02 +0000 Subject: [PATCH 05/14] Fix unidiomatic decrement --- Objects/longobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 4c4b3c176c7701..4d9d885755b495 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4517,7 +4517,7 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) 0 < remshift <= PyLong_SHIFT. */ if (remshift == 0) { remshift = PyLong_SHIFT; - wordshift -= 1; + --wordshift; } assert(wordshift >= 0); } From 53a4e245f931eac4fbce4acae4c44dda55bad813 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 28 Dec 2021 13:00:50 +0000 Subject: [PATCH 06/14] Don't handle the shift==0 special-case until we need to --- Objects/longobject.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 4d9d885755b495..75653d1bf1f2a4 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4487,7 +4487,7 @@ divmod_shift(PyObject *shiftby, Py_ssize_t *wordshift, digit *remshift) } /* Inner function for both long_rshift and _PyLong_Rshift, shifting an - integer right by a strictly positive shift. */ + integer right by a nonnegative shift. */ static PyObject * long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) @@ -4497,8 +4497,6 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) twodigits accum; int a_negative; - assert (wordshift > 0 || remshift > 0); - /* Fast path for small a. */ if (IS_MEDIUM_VALUE(a)) { stwodigits m, x; @@ -4518,10 +4516,14 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) if (remshift == 0) { remshift = PyLong_SHIFT; --wordshift; + if (wordshift < 0) { + /* Can only happen if the original shift was 0. */ + return long_long(a); + } } - assert(wordshift >= 0); } + assert(wordshift >= 0); newsize = size_a - wordshift; if (newsize <= 0) { return PyLong_FromLong(-a_negative); @@ -4584,9 +4586,6 @@ long_rshift(PyObject *a, PyObject *b) PyErr_SetString(PyExc_ValueError, "negative shift count"); return NULL; } - if (Py_SIZE(b) == 0) { - return long_long(a); - } if (Py_SIZE(a) == 0) { return PyLong_FromLong(0); } @@ -4603,10 +4602,6 @@ _PyLong_Rshift(PyObject *a, size_t shiftby) digit remshift; assert(PyLong_Check(a)); - - if (shiftby == 0) { - return long_long(a); - } if (Py_SIZE(a) == 0) { return PyLong_FromLong(0); } From b3a1a8f9d09f094cf2821da9bd3453ac6d750884 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 28 Dec 2021 13:17:11 +0000 Subject: [PATCH 07/14] Fix incompatible types --- Objects/longobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 75653d1bf1f2a4..34324784c64053 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4518,7 +4518,7 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) --wordshift; if (wordshift < 0) { /* Can only happen if the original shift was 0. */ - return long_long(a); + return long_long((PyObject *)a); } } } From fd4ce81854531f419bd0617cd984c346038e3f60 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Sun, 9 Jan 2022 10:48:06 +0000 Subject: [PATCH 08/14] Comment updates --- Objects/longobject.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 34324784c64053..6866e9aaf98534 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4511,8 +4511,8 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) size_a = Py_ABS(Py_SIZE(a)); if (a_negative) { - /* For shifting negative integers, it's convenient to adjust so that - 0 < remshift <= PyLong_SHIFT. */ + /* For 'a', adjust so that 0 < remshift <= PyLong_SHIFT. This ensures + that 'newsize' is computed correctly below. */ if (remshift == 0) { remshift = PyLong_SHIFT; --wordshift; @@ -4526,6 +4526,7 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) assert(wordshift >= 0); newsize = size_a - wordshift; if (newsize <= 0) { + /* Shifting all the bits of 'a' out gives either -1 or 0. */ return PyLong_FromLong(-a_negative); } z = _PyLong_New(newsize); From 447e508c517f3e4a20520ac16a4eebb88ea37f7b Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 12 Apr 2022 09:18:12 +0100 Subject: [PATCH 09/14] Updates from review --- Objects/longobject.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 6866e9aaf98534..0ba2e7779e1068 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4487,7 +4487,8 @@ divmod_shift(PyObject *shiftby, Py_ssize_t *wordshift, digit *remshift) } /* Inner function for both long_rshift and _PyLong_Rshift, shifting an - integer right by a nonnegative shift. */ + integer right by PyLong_SHIFT*wordshift + remshift bits. + wordshift should be nonnegative. */ static PyObject * long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) @@ -4497,6 +4498,9 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) twodigits accum; int a_negative; + /* Total number of bits shifted must be nonnegative. */ + assert(wordshift >= 0); + /* Fast path for small a. */ if (IS_MEDIUM_VALUE(a)) { stwodigits m, x; @@ -4511,15 +4515,16 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) size_a = Py_ABS(Py_SIZE(a)); if (a_negative) { - /* For 'a', adjust so that 0 < remshift <= PyLong_SHIFT. This ensures + /* For 'a', adjust so that 0 < remshift <= PyLong_SHIFT, while keeping + PyLong_SHIFt * wordshift + remshift the same. This ensures that 'newsize' is computed correctly below. */ if (remshift == 0) { - remshift = PyLong_SHIFT; - --wordshift; - if (wordshift < 0) { + if (wordshift == 0) { /* Can only happen if the original shift was 0. */ return long_long((PyObject *)a); } + remshift = PyLong_SHIFT; + --wordshift; } } @@ -4550,20 +4555,18 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) Py_SET_SIZE(z, -newsize); digit sticky = 0; - for (j = 0; j < wordshift; j++) { + for (Py_ssize_t j = 0; j < wordshift; j++) { sticky |= a->ob_digit[j]; } - accum = a->ob_digit[j++] + (PyLong_MASK >> hishift) + accum = a->ob_digit[wordshift] + (PyLong_MASK >> hishift) + (digit)(sticky != 0); } else { - j = wordshift; - accum = a->ob_digit[j++]; + accum = a->ob_digit[wordshift]; } accum >>= remshift; - assert(j == wordshift+1); - for (i = 0; j < size_a; i++, j++) { + for (i = 0, j = wordshift + 1; j < size_a; i++, j++) { accum += (twodigits)a->ob_digit[j] << hishift; z->ob_digit[i] = (digit)(accum & PyLong_MASK); accum >>= PyLong_SHIFT; From 1bf40635e705cec8a47e5cf33b98000a0ecb78d1 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 12 Apr 2022 09:24:31 +0100 Subject: [PATCH 10/14] Clean up loop declarations --- Objects/longobject.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 0ba2e7779e1068..7832cbb4dd00f6 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4494,7 +4494,7 @@ static PyObject * long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) { PyLongObject *z = NULL; - Py_ssize_t newsize, hishift, i, j, size_a; + Py_ssize_t newsize, hishift, size_a; twodigits accum; int a_negative; @@ -4566,14 +4566,13 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) } accum >>= remshift; - for (i = 0, j = wordshift + 1; j < size_a; i++, j++) { + for (Py_ssize_t i = 0, j = wordshift + 1; j < size_a; i++, j++) { accum += (twodigits)a->ob_digit[j] << hishift; z->ob_digit[i] = (digit)(accum & PyLong_MASK); accum >>= PyLong_SHIFT; } - assert(i == newsize - 1); assert(accum <= PyLong_MASK); - z->ob_digit[i] = (digit)accum; + z->ob_digit[newsize - 1] = (digit)accum; z = maybe_small_long(long_normalize(z)); return (PyObject *)z; } From cfe47ac5b1c688618362bd652680da18fc5fc12d Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 12 Apr 2022 09:28:16 +0100 Subject: [PATCH 11/14] Fix comment --- Objects/longobject.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 7832cbb4dd00f6..9639a9e920cee1 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4515,9 +4515,9 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) size_a = Py_ABS(Py_SIZE(a)); if (a_negative) { - /* For 'a', adjust so that 0 < remshift <= PyLong_SHIFT, while keeping - PyLong_SHIFt * wordshift + remshift the same. This ensures - that 'newsize' is computed correctly below. */ + /* For negative 'a', adjust so that 0 < remshift <= PyLong_SHIFT, + while keeping PyLong_SHIFT*wordshift + remshift the same. This + ensures that 'newsize' is computed correctly below. */ if (remshift == 0) { if (wordshift == 0) { /* Can only happen if the original shift was 0. */ From 68e10a3d16de71eb0a49cea0272b4d24d5deaac8 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 12 Apr 2022 09:37:24 +0100 Subject: [PATCH 12/14] Remove an unnecessary 'else' branch --- Objects/longobject.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 9639a9e920cee1..b55fb4142e7bfb 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4540,6 +4540,7 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) } hishift = PyLong_SHIFT - remshift; + accum = a->ob_digit[wordshift]; if (a_negative) { /* For a positive integer a and nonnegative shift, we have: @@ -4558,11 +4559,7 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) for (Py_ssize_t j = 0; j < wordshift; j++) { sticky |= a->ob_digit[j]; } - accum = a->ob_digit[wordshift] + (PyLong_MASK >> hishift) - + (digit)(sticky != 0); - } - else { - accum = a->ob_digit[wordshift]; + accum += (PyLong_MASK >> hishift) + (digit)(sticky != 0); } accum >>= remshift; @@ -4573,6 +4570,7 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) } assert(accum <= PyLong_MASK); z->ob_digit[newsize - 1] = (digit)accum; + z = maybe_small_long(long_normalize(z)); return (PyObject *)z; } From 5d8d8ef908710372d123c4565bbd62671463d682 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 12 Apr 2022 09:41:08 +0100 Subject: [PATCH 13/14] New news entry --- .../2022-04-12-09-40-57.gh-issue-46055.IPb1HA.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-04-12-09-40-57.gh-issue-46055.IPb1HA.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-04-12-09-40-57.gh-issue-46055.IPb1HA.rst b/Misc/NEWS.d/next/Core and Builtins/2022-04-12-09-40-57.gh-issue-46055.IPb1HA.rst new file mode 100644 index 00000000000000..ed7e4113fd2e62 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-04-12-09-40-57.gh-issue-46055.IPb1HA.rst @@ -0,0 +1,2 @@ +Speed up right shift of negative integers, by removing unnecessary creation +of temporaries. Original patch by Xinhang Xu, reworked by Mark Dickinson. From 82c5ccdb7035a28aadb969a2f2958dc2b8469a95 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 12 Apr 2022 11:04:20 +0100 Subject: [PATCH 14/14] One more assert --- Objects/longobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/longobject.c b/Objects/longobject.c index c983b2d09c936b..1a38df060d0feb 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -4691,6 +4691,7 @@ long_rshift1(PyLongObject *a, Py_ssize_t wordshift, digit remshift) /* Total number of bits shifted must be nonnegative. */ assert(wordshift >= 0); + assert(remshift < PyLong_SHIFT); /* Fast path for small a. */ if (IS_MEDIUM_VALUE(a)) {