From 8abc68816ea7765e52511411a135590942134aa1 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 6 Dec 2022 22:20:41 +0100 Subject: [PATCH 1/4] add _PyLongNegative_or_multi_digit_int --- Include/internal/pycore_long.h | 9 +++++++++ Python/bytecodes.c | 8 +++----- Python/generated_cases.c.h | 8 +++----- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h index 30c97b7edc98e1..3d1a1dd6b3b1db 100644 --- a/Include/internal/pycore_long.h +++ b/Include/internal/pycore_long.h @@ -110,6 +110,15 @@ PyAPI_FUNC(char*) _PyLong_FormatBytesWriter( int base, int alternate); +/* Return 1 if the argument is negative or a multi-digit int */ +static inline int +_PyLong_Negative_or_multi_digit_int(PyObject* sub) { + // this method uses the twos-complement representation + assert(PyLong_CheckExact(sub)); + Py_ssize_t signed_magnitude = Py_SIZE(sub); + return ((size_t)signed_magnitude) > 1; +} + #ifdef __cplusplus } #endif diff --git a/Python/bytecodes.c b/Python/bytecodes.c index d0480ac01eb610..0841ba9cbb3945 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -383,8 +383,7 @@ dummy_func( DEOPT_IF(!PyList_CheckExact(list), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyList_Size(list) - Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyList_GET_SIZE(list), BINARY_SUBSCR); @@ -402,8 +401,7 @@ dummy_func( DEOPT_IF(!PyTuple_CheckExact(tuple), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyTuple_Size(list) - Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyTuple_GET_SIZE(tuple), BINARY_SUBSCR); @@ -507,7 +505,7 @@ dummy_func( DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - DEOPT_IF(((size_t)Py_SIZE(sub)) > 1, STORE_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), STORE_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 0805386866b318..24602bd0a1b474 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -367,8 +367,7 @@ DEOPT_IF(!PyList_CheckExact(list), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyList_Size(list) - Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyList_GET_SIZE(list), BINARY_SUBSCR); @@ -393,8 +392,7 @@ DEOPT_IF(!PyTuple_CheckExact(tuple), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyTuple_Size(list) - Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyTuple_GET_SIZE(tuple), BINARY_SUBSCR); @@ -518,7 +516,7 @@ DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - DEOPT_IF(((size_t)Py_SIZE(sub)) > 1, STORE_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), STORE_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); From 4c3f05dca9d79811fede93194f069aca8b5f89f1 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 7 Dec 2022 22:20:35 +0100 Subject: [PATCH 2/4] rename to _PyLong_IsPositiveSingleDigit --- Include/internal/pycore_long.h | 4 ++-- Python/bytecodes.c | 6 +++--- Python/generated_cases.c.h | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h index 3d1a1dd6b3b1db..ae2664a3b0cfe1 100644 --- a/Include/internal/pycore_long.h +++ b/Include/internal/pycore_long.h @@ -112,11 +112,11 @@ PyAPI_FUNC(char*) _PyLong_FormatBytesWriter( /* Return 1 if the argument is negative or a multi-digit int */ static inline int -_PyLong_Negative_or_multi_digit_int(PyObject* sub) { +_PyLong_IsPositiveSingleDigit(PyObject* sub) { // this method uses the twos-complement representation assert(PyLong_CheckExact(sub)); Py_ssize_t signed_magnitude = Py_SIZE(sub); - return ((size_t)signed_magnitude) > 1; + return ((size_t)signed_magnitude) <= 1; } #ifdef __cplusplus diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 0841ba9cbb3945..feb7a23bd6ebbf 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -383,7 +383,7 @@ dummy_func( DEOPT_IF(!PyList_CheckExact(list), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyList_Size(list) - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); + DEOPT_IF(!_PyLong_IsPositiveSingleDigit(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyList_GET_SIZE(list), BINARY_SUBSCR); @@ -401,7 +401,7 @@ dummy_func( DEOPT_IF(!PyTuple_CheckExact(tuple), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyTuple_Size(list) - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); + DEOPT_IF(!_PyLong_IsPositiveSingleDigit(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyTuple_GET_SIZE(tuple), BINARY_SUBSCR); @@ -505,7 +505,7 @@ dummy_func( DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), STORE_SUBSCR); + DEOPT_IF(!_PyLong_IsPositiveSingleDigit(sub), STORE_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 24602bd0a1b474..d27273655e225d 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -367,7 +367,7 @@ DEOPT_IF(!PyList_CheckExact(list), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyList_Size(list) - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); + DEOPT_IF(!_PyLong_IsPositiveSingleDigit(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyList_GET_SIZE(list), BINARY_SUBSCR); @@ -392,7 +392,7 @@ DEOPT_IF(!PyTuple_CheckExact(tuple), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyTuple_Size(list) - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); + DEOPT_IF(!_PyLong_IsPositiveSingleDigit(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyTuple_GET_SIZE(tuple), BINARY_SUBSCR); @@ -516,7 +516,7 @@ DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), STORE_SUBSCR); + DEOPT_IF(!_PyLong_IsPositiveSingleDigit(sub), STORE_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); From 1cf708f32a53b19a714534a6d1e1f26559176a1f Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 7 Dec 2022 22:25:26 +0100 Subject: [PATCH 3/4] update docstring --- Include/internal/pycore_long.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h index ae2664a3b0cfe1..3a2ce4a2ba0dcd 100644 --- a/Include/internal/pycore_long.h +++ b/Include/internal/pycore_long.h @@ -110,7 +110,7 @@ PyAPI_FUNC(char*) _PyLong_FormatBytesWriter( int base, int alternate); -/* Return 1 if the argument is negative or a multi-digit int */ +/* Return 1 if the argument is positive single digit int */ static inline int _PyLong_IsPositiveSingleDigit(PyObject* sub) { // this method uses the twos-complement representation From 3733912046433d2b99ad7e78f4966772e0657e52 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 10 Dec 2022 21:47:05 +0100 Subject: [PATCH 4/4] address review comments --- Include/internal/pycore_long.h | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h index 3a2ce4a2ba0dcd..8c1d017bb95e4e 100644 --- a/Include/internal/pycore_long.h +++ b/Include/internal/pycore_long.h @@ -113,10 +113,20 @@ PyAPI_FUNC(char*) _PyLong_FormatBytesWriter( /* Return 1 if the argument is positive single digit int */ static inline int _PyLong_IsPositiveSingleDigit(PyObject* sub) { - // this method uses the twos-complement representation + /* 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_magnitude = Py_SIZE(sub); - return ((size_t)signed_magnitude) <= 1; + Py_ssize_t signed_size = Py_SIZE(sub); + return ((size_t)signed_size) <= 1; } #ifdef __cplusplus