From 62b0307c5af7d9fc48051845a61bb167578a5fd5 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 9 Nov 2024 18:06:39 +0900 Subject: [PATCH 01/15] gh-115999: Add free-threaded specialization for ``TO_BOOL`` * None / bool / int / str are immutable types, so they is thread-safe. * list is mutable, but by using ``PyList_GET_SIZE`` we can make it as thread-safe. --- Lib/test/test_opcache.py | 66 +++++++++++++++++++++ Python/bytecodes.c | 6 +- Python/executor_cases.c.h | 2 +- Python/generated_cases.c.h | 6 +- Python/specialize.c | 114 +++++++++++++++++-------------------- 5 files changed, 125 insertions(+), 69 deletions(-) diff --git a/Lib/test/test_opcache.py b/Lib/test/test_opcache.py index 78e4bf44f7ea0c..32b24c4a9509c8 100644 --- a/Lib/test/test_opcache.py +++ b/Lib/test/test_opcache.py @@ -1255,6 +1255,72 @@ def g(): self.assert_specialized(g, "CONTAINS_OP_SET") self.assert_no_opcode(g, "CONTAINS_OP") + @cpython_only + @requires_specialization_ft + def test_to_bool(self): + def to_bool_bool(): + true_cnt, false_cnt = 0, 0 + elems = [e % 2 == 0 for e in range(100)] + for e in elems: + if e: + true_cnt += 1 + else: + false_cnt -= 1 + self.assertEqual(true_cnt, 50) + self.assertEqual(false_cnt, -50) + + to_bool_bool() + self.assert_specialized(to_bool_bool, "TO_BOOL_BOOL") + self.assert_no_opcode(to_bool_bool, "TO_BOOL") + + def to_bool_int(): + count = 0 + for i in range(100): + if i: + count += 1 + else: + count -= 1 + self.assertEqual(count, 98) + + to_bool_int() + self.assert_specialized(to_bool_int, "TO_BOOL_INT") + self.assert_no_opcode(to_bool_int, "TO_BOOL") + + def to_bool_list(): + count = 0 + elems = [1, 2, 3] + while elems: + count += elems.pop() + self.assertEqual(elems, []) + self.assertEqual(count, 6) + + to_bool_list() + self.assert_specialized(to_bool_list, "TO_BOOL_LIST") + self.assert_no_opcode(to_bool_list, "TO_BOOL") + + def to_bool_none(): + count = 0 + elems = [None, None, None, None] + for e in elems: + if not e: + count += 1 + self.assertEqual(count, len(elems)) + + to_bool_none() + self.assert_specialized(to_bool_none, "TO_BOOL_NONE") + self.assert_no_opcode(to_bool_none, "TO_BOOL") + + def to_bool_str(): + count = 0 + elems = ["", "foo", ""] + for e in elems: + if e: + count += 1 + self.assertEqual(count, 1) + + to_bool_str() + self.assert_specialized(to_bool_str, "TO_BOOL_STR") + self.assert_no_opcode(to_bool_str, "TO_BOOL") if __name__ == "__main__": diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 04983fd861ec59..4462871c2fa243 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -391,7 +391,7 @@ dummy_func( }; specializing op(_SPECIALIZE_TO_BOOL, (counter/1, value -- value)) { - #if ENABLE_SPECIALIZATION + #if ENABLE_SPECIALIZATION_FT if (ADAPTIVE_COUNTER_TRIGGERS(counter)) { next_instr = this_instr; _Py_Specialize_ToBool(value, next_instr); @@ -399,7 +399,7 @@ dummy_func( } OPCODE_DEFERRED_INC(TO_BOOL); ADVANCE_ADAPTIVE_COUNTER(this_instr[1].counter); - #endif /* ENABLE_SPECIALIZATION */ + #endif /* ENABLE_SPECIALIZATION_FT */ } op(_TO_BOOL, (value -- res)) { @@ -435,7 +435,7 @@ dummy_func( PyObject *value_o = PyStackRef_AsPyObjectBorrow(value); EXIT_IF(!PyList_CheckExact(value_o)); STAT_INC(TO_BOOL, hit); - res = Py_SIZE(value_o) ? PyStackRef_True : PyStackRef_False; + res = PyList_GET_SIZE(value_o) ? PyStackRef_True : PyStackRef_False; DECREF_INPUTS(); } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 494ace1bd85822..c59dc4d8bc2507 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -508,7 +508,7 @@ JUMP_TO_JUMP_TARGET(); } STAT_INC(TO_BOOL, hit); - res = Py_SIZE(value_o) ? PyStackRef_True : PyStackRef_False; + res = PyList_GET_SIZE(value_o) ? PyStackRef_True : PyStackRef_False; PyStackRef_CLOSE(value); stack_pointer[-1] = res; break; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 77bf6ad3781f17..998175548feab9 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -7746,7 +7746,7 @@ value = stack_pointer[-1]; uint16_t counter = read_u16(&this_instr[1].cache); (void)counter; - #if ENABLE_SPECIALIZATION + #if ENABLE_SPECIALIZATION_FT if (ADAPTIVE_COUNTER_TRIGGERS(counter)) { next_instr = this_instr; _PyFrame_SetStackPointer(frame, stack_pointer); @@ -7756,7 +7756,7 @@ } OPCODE_DEFERRED_INC(TO_BOOL); ADVANCE_ADAPTIVE_COUNTER(this_instr[1].counter); - #endif /* ENABLE_SPECIALIZATION */ + #endif /* ENABLE_SPECIALIZATION_FT */ } /* Skip 2 cache entries */ // _TO_BOOL @@ -7851,7 +7851,7 @@ PyObject *value_o = PyStackRef_AsPyObjectBorrow(value); DEOPT_IF(!PyList_CheckExact(value_o), TO_BOOL); STAT_INC(TO_BOOL, hit); - res = Py_SIZE(value_o) ? PyStackRef_True : PyStackRef_False; + res = PyList_GET_SIZE(value_o) ? PyStackRef_True : PyStackRef_False; PyStackRef_CLOSE(value); stack_pointer[-1] = res; DISPATCH(); diff --git a/Python/specialize.c b/Python/specialize.c index 0699e7be5e6b9c..8ee121dfad29e2 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -2663,101 +2663,91 @@ _Py_Specialize_Send(_PyStackRef receiver_st, _Py_CODEUNIT *instr) cache->counter = adaptive_counter_cooldown(); } +static int +to_bool_fail_kind(PyObject *value) +{ + if (PyByteArray_CheckExact(value)) { + return SPEC_FAIL_TO_BOOL_BYTEARRAY; + } + if (PyBytes_CheckExact(value)) { + return SPEC_FAIL_TO_BOOL_BYTES; + } + if (PyDict_CheckExact(value)) { + return SPEC_FAIL_TO_BOOL_DICT; + } + if (PyFloat_CheckExact(value)) { + return SPEC_FAIL_TO_BOOL_FLOAT; + } + if (PyMemoryView_Check(value)) { + return SPEC_FAIL_TO_BOOL_MEMORY_VIEW; + } + if (PyAnySet_CheckExact(value)) { + return SPEC_FAIL_TO_BOOL_SET; + } + if (PyTuple_CheckExact(value)) { + return SPEC_FAIL_TO_BOOL_TUPLE; + } + return SPEC_FAIL_OTHER; +} + void _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) { - assert(ENABLE_SPECIALIZATION); + assert(ENABLE_SPECIALIZATION_FT); assert(_PyOpcode_Caches[TO_BOOL] == INLINE_CACHE_ENTRIES_TO_BOOL); _PyToBoolCache *cache = (_PyToBoolCache *)(instr + 1); PyObject *value = PyStackRef_AsPyObjectBorrow(value_o); if (PyBool_Check(value)) { - instr->op.code = TO_BOOL_BOOL; - goto success; + specialize(instr, TO_BOOL_BOOL); + return; } if (PyLong_CheckExact(value)) { - instr->op.code = TO_BOOL_INT; - goto success; + specialize(instr, TO_BOOL_INT); + return; } if (PyList_CheckExact(value)) { - instr->op.code = TO_BOOL_LIST; - goto success; + specialize(instr, TO_BOOL_LIST); + return; } if (Py_IsNone(value)) { - instr->op.code = TO_BOOL_NONE; - goto success; + specialize(instr, TO_BOOL_NONE); + return; } if (PyUnicode_CheckExact(value)) { - instr->op.code = TO_BOOL_STR; - goto success; + specialize(instr, TO_BOOL_STR); + return; } if (PyType_HasFeature(Py_TYPE(value), Py_TPFLAGS_HEAPTYPE)) { PyNumberMethods *nb = Py_TYPE(value)->tp_as_number; if (nb && nb->nb_bool) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_TO_BOOL_NUMBER); - goto failure; + unspecialize(instr, SPEC_FAIL_TO_BOOL_NUMBER); + return; } PyMappingMethods *mp = Py_TYPE(value)->tp_as_mapping; if (mp && mp->mp_length) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_TO_BOOL_MAPPING); - goto failure; + unspecialize(instr, SPEC_FAIL_TO_BOOL_MAPPING); + return; } PySequenceMethods *sq = Py_TYPE(value)->tp_as_sequence; if (sq && sq->sq_length) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_TO_BOOL_SEQUENCE); - goto failure; + unspecialize(instr, SPEC_FAIL_TO_BOOL_SEQUENCE); + return; } if (!PyUnstable_Type_AssignVersionTag(Py_TYPE(value))) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_OUT_OF_VERSIONS); - goto failure; + unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); + return; } - uint32_t version = type_get_version(Py_TYPE(value), TO_BOOL); + uint32_t version = FT_ATOMIC_LOAD_UINT32_RELAXED(Py_TYPE(value)->tp_version_tag); if (version == 0) { - goto failure; + unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); + return; } - instr->op.code = TO_BOOL_ALWAYS_TRUE; + specialize(instr, TO_BOOL_ALWAYS_TRUE); write_u32(cache->version, version); assert(version); - goto success; - } -#ifdef Py_STATS - if (PyByteArray_CheckExact(value)) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_TO_BOOL_BYTEARRAY); - goto failure; - } - if (PyBytes_CheckExact(value)) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_TO_BOOL_BYTES); - goto failure; - } - if (PyDict_CheckExact(value)) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_TO_BOOL_DICT); - goto failure; - } - if (PyFloat_CheckExact(value)) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_TO_BOOL_FLOAT); - goto failure; - } - if (PyMemoryView_Check(value)) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_TO_BOOL_MEMORY_VIEW); - goto failure; - } - if (PyAnySet_CheckExact(value)) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_TO_BOOL_SET); - goto failure; - } - if (PyTuple_CheckExact(value)) { - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_TO_BOOL_TUPLE); - goto failure; + return; } - SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_OTHER); -#endif // Py_STATS -failure: - STAT_INC(TO_BOOL, failure); - instr->op.code = TO_BOOL; - cache->counter = adaptive_counter_backoff(cache->counter); - return; -success: - STAT_INC(TO_BOOL, success); - cache->counter = adaptive_counter_cooldown(); + unspecialize(instr, to_bool_fail_kind(value)); } static int From e32f2f020217cfc6b600fbd043d250f21dee448c Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 9 Nov 2024 18:13:33 +0900 Subject: [PATCH 02/15] nit --- Lib/test/test_opcache.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_opcache.py b/Lib/test/test_opcache.py index 32b24c4a9509c8..4c2107d7d34ebf 100644 --- a/Lib/test/test_opcache.py +++ b/Lib/test/test_opcache.py @@ -1265,9 +1265,9 @@ def to_bool_bool(): if e: true_cnt += 1 else: - false_cnt -= 1 + false_cnt += 1 self.assertEqual(true_cnt, 50) - self.assertEqual(false_cnt, -50) + self.assertEqual(false_cnt, 50) to_bool_bool() self.assert_specialized(to_bool_bool, "TO_BOOL_BOOL") From 67983665ded9c9395d6f78a87e7331f7a99c004b Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 9 Nov 2024 19:32:31 +0900 Subject: [PATCH 03/15] Make set cache as atomic operation --- Python/specialize.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/Python/specialize.c b/Python/specialize.c index 8ee121dfad29e2..848c5d14301b25 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -689,6 +689,21 @@ set_opcode(_Py_CODEUNIT *instr, uint8_t opcode) #endif } +static inline int +set_cache_verison(uint16_t *old_version, uint32_t new_version) +{ +#ifdef Py_GIL_DISABLED + uint32_t old_version_val = read_u32(old_version); + if (!_Py_atomic_compare_exchange_uint32((uint32_t *)old_version, &old_version_val, new_version)) { + return 0; + } + return 1; +#else + write_u32(cache, new_version); + return 1; +#endif +} + static inline void set_counter(_Py_BackoffCounter *counter, _Py_BackoffCounter value) { @@ -2742,8 +2757,17 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); return; } +#ifdef Py_GIL_DISABLED + if (read_u32(cache->version) > version) { + unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); + return; + } +#endif + if (!set_cache_verison(cache->version, version)) { + unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); + return; + } specialize(instr, TO_BOOL_ALWAYS_TRUE); - write_u32(cache->version, version); assert(version); return; } From e45778b560ddebc0a767be2151690de756988d58 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 9 Nov 2024 19:36:25 +0900 Subject: [PATCH 04/15] fix --- Python/specialize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/specialize.c b/Python/specialize.c index 848c5d14301b25..a1121cb1d9ffba 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -699,7 +699,7 @@ set_cache_verison(uint16_t *old_version, uint32_t new_version) } return 1; #else - write_u32(cache, new_version); + write_u32(old_version, new_version); return 1; #endif } From bafabd76e83b0a210f2f25b3e3e218b1b89cc4f7 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 9 Nov 2024 20:42:44 +0900 Subject: [PATCH 05/15] fix --- Python/specialize.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index a1121cb1d9ffba..64b9da2b7cc2e8 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -693,8 +693,8 @@ static inline int set_cache_verison(uint16_t *old_version, uint32_t new_version) { #ifdef Py_GIL_DISABLED - uint32_t old_version_val = read_u32(old_version); - if (!_Py_atomic_compare_exchange_uint32((uint32_t *)old_version, &old_version_val, new_version)) { + uint16_t old_version_val = _Py_atomic_load_uint16_relaxed(old_version); + if (!_Py_atomic_compare_exchange_uint16(old_version, &old_version_val, new_version)) { return 0; } return 1; From aa8a81cd8721c545a728f30b2b29d0cc9e338f63 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 9 Nov 2024 21:00:26 +0900 Subject: [PATCH 06/15] nit --- Python/specialize.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index 64b9da2b7cc2e8..337c64b4f3625f 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -690,10 +690,13 @@ set_opcode(_Py_CODEUNIT *instr, uint8_t opcode) } static inline int -set_cache_verison(uint16_t *old_version, uint32_t new_version) +set_cache_version(uint16_t *old_version, uint32_t new_version) { #ifdef Py_GIL_DISABLED uint16_t old_version_val = _Py_atomic_load_uint16_relaxed(old_version); + if (old_version_val > (uint16_t)new_version) { + return 0; + } if (!_Py_atomic_compare_exchange_uint16(old_version, &old_version_val, new_version)) { return 0; } @@ -2757,13 +2760,7 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); return; } -#ifdef Py_GIL_DISABLED - if (read_u32(cache->version) > version) { - unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); - return; - } -#endif - if (!set_cache_verison(cache->version, version)) { + if (!set_cache_version(cache->version, version)) { unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); return; } From 1f4a350932f332a4838da233063d525b9b8cf54d Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sun, 10 Nov 2024 16:20:45 +0900 Subject: [PATCH 07/15] Address code review --- Python/specialize.c | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index 337c64b4f3625f..179f0b839708a8 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -689,24 +689,6 @@ set_opcode(_Py_CODEUNIT *instr, uint8_t opcode) #endif } -static inline int -set_cache_version(uint16_t *old_version, uint32_t new_version) -{ -#ifdef Py_GIL_DISABLED - uint16_t old_version_val = _Py_atomic_load_uint16_relaxed(old_version); - if (old_version_val > (uint16_t)new_version) { - return 0; - } - if (!_Py_atomic_compare_exchange_uint16(old_version, &old_version_val, new_version)) { - return 0; - } - return 1; -#else - write_u32(old_version, new_version); - return 1; -#endif -} - static inline void set_counter(_Py_BackoffCounter *counter, _Py_BackoffCounter value) { @@ -2755,16 +2737,13 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); return; } - uint32_t version = FT_ATOMIC_LOAD_UINT32_RELAXED(Py_TYPE(value)->tp_version_tag); + uint32_t version = Py_TYPE(value)->tp_version_tag; if (version == 0) { unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); return; } - if (!set_cache_version(cache->version, version)) { - unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); - return; - } specialize(instr, TO_BOOL_ALWAYS_TRUE); + write_u32(cache->version, version); assert(version); return; } From 23e190ebcf730902346083f513c782ec8f404dc0 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sun, 10 Nov 2024 17:19:45 +0900 Subject: [PATCH 08/15] Address code review --- Include/internal/pycore_typeobject.h | 5 +++ Objects/typeobject.c | 18 +++++++++++ Python/specialize.c | 47 +++++++++++++++++----------- 3 files changed, 52 insertions(+), 18 deletions(-) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index 5debdd68fe94ca..cb96ddd588e683 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -269,6 +269,11 @@ extern unsigned int _PyType_GetVersionForCurrentState(PyTypeObject *tp); PyAPI_FUNC(void) _PyType_SetVersion(PyTypeObject *tp, unsigned int version); PyTypeObject *_PyType_LookupByVersion(unsigned int version); +// Returns 0 on success or caller-specific error on failure. +typedef int (*_py_validate_type)(PyTypeObject *); +// Returns 0 on success, -1 if no type version could be assigned, or the error returned by validate +extern int _PyType_Validate(PyTypeObject *ty, _py_validate_type validate, unsigned int *tp_version); + #ifdef __cplusplus } #endif diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 4af7f0273aae91..5642c7c7212ce5 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5645,6 +5645,24 @@ _PyType_SetFlags(PyTypeObject *self, unsigned long mask, unsigned long flags) END_TYPE_LOCK(); } +int +_PyType_Validate(PyTypeObject *ty, _py_validate_type validate, unsigned int *tp_version) +{ + int err; + BEGIN_TYPE_LOCK(); + err = validate(ty); + if (!err) { + if(assign_version_tag(_PyInterpreterState_GET(), ty)) { + *tp_version = ty->tp_version_tag; + } + else { + err = -1; + } + } + END_TYPE_LOCK(); + return err; +} + static void set_flags_recursive(PyTypeObject *self, unsigned long mask, unsigned long flags) { diff --git a/Python/specialize.c b/Python/specialize.c index 179f0b839708a8..ba990daf7d16ea 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -2690,6 +2690,25 @@ to_bool_fail_kind(PyObject *value) return SPEC_FAIL_OTHER; } +static int +check_type_always_true(PyTypeObject *ty) +{ + PyNumberMethods *nb = ty->tp_as_number; + if (nb && nb->nb_bool) { + return SPEC_FAIL_TO_BOOL_NUMBER; + } + PyMappingMethods *mp = ty->tp_as_mapping; + if (mp && mp->mp_length) { + return SPEC_FAIL_TO_BOOL_MAPPING; + } + PySequenceMethods *sq = ty->tp_as_sequence; + if (sq && sq->sq_length) { + return SPEC_FAIL_TO_BOOL_SEQUENCE; + } + return 0; +} + + void _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) { @@ -2718,33 +2737,25 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) return; } if (PyType_HasFeature(Py_TYPE(value), Py_TPFLAGS_HEAPTYPE)) { - PyNumberMethods *nb = Py_TYPE(value)->tp_as_number; - if (nb && nb->nb_bool) { - unspecialize(instr, SPEC_FAIL_TO_BOOL_NUMBER); - return; - } - PyMappingMethods *mp = Py_TYPE(value)->tp_as_mapping; - if (mp && mp->mp_length) { - unspecialize(instr, SPEC_FAIL_TO_BOOL_MAPPING); - return; - } - PySequenceMethods *sq = Py_TYPE(value)->tp_as_sequence; - if (sq && sq->sq_length) { - unspecialize(instr, SPEC_FAIL_TO_BOOL_SEQUENCE); + unsigned int version = 0; + int err = _PyType_Validate(Py_TYPE(value), check_type_always_true, &version); + if (err < 0) { + unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); return; } - if (!PyUnstable_Type_AssignVersionTag(Py_TYPE(value))) { - unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); + else if (err > 0) { + unspecialize(instr, err); return; } - uint32_t version = Py_TYPE(value)->tp_version_tag; + + assert(err == 0); if (version == 0) { unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); return; } - specialize(instr, TO_BOOL_ALWAYS_TRUE); - write_u32(cache->version, version); assert(version); + write_u32(cache->version, version); + specialize(instr, TO_BOOL_ALWAYS_TRUE); return; } unspecialize(instr, to_bool_fail_kind(value)); From 30020b9f80a135ad704c1102f02d473afe08faad Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 12 Nov 2024 22:21:09 +0900 Subject: [PATCH 09/15] Address code review --- Include/internal/pycore_typeobject.h | 9 +++++++-- Python/specialize.c | 4 ---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index cb96ddd588e683..8b473f65e2941f 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -269,9 +269,14 @@ extern unsigned int _PyType_GetVersionForCurrentState(PyTypeObject *tp); PyAPI_FUNC(void) _PyType_SetVersion(PyTypeObject *tp, unsigned int version); PyTypeObject *_PyType_LookupByVersion(unsigned int version); -// Returns 0 on success or caller-specific error on failure. +// Function pointer type for user-defined validation function that will be +// called by _PyType_Validate(). +// It should return 0 if the validation is passed, otherwise it will return -1. typedef int (*_py_validate_type)(PyTypeObject *); -// Returns 0 on success, -1 if no type version could be assigned, or the error returned by validate + +// It will verify the ``ty`` through user-defined validation function ``validate``, +// and if the validation is passed, it will set the ``tp_version`` to the next +// available version and return 0. Otherwise, it will return an error code. extern int _PyType_Validate(PyTypeObject *ty, _py_validate_type validate, unsigned int *tp_version); #ifdef __cplusplus diff --git a/Python/specialize.c b/Python/specialize.c index ba990daf7d16ea..b67e7eb201f316 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -2749,10 +2749,6 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) } assert(err == 0); - if (version == 0) { - unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); - return; - } assert(version); write_u32(cache->version, version); specialize(instr, TO_BOOL_ALWAYS_TRUE); From b1f1c71140757af519e63073fe1ff056b796e333 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 13 Nov 2024 00:01:14 +0900 Subject: [PATCH 10/15] Update comments --- Include/internal/pycore_typeobject.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index 8b473f65e2941f..7b39d07f976ee3 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -275,8 +275,8 @@ PyTypeObject *_PyType_LookupByVersion(unsigned int version); typedef int (*_py_validate_type)(PyTypeObject *); // It will verify the ``ty`` through user-defined validation function ``validate``, -// and if the validation is passed, it will set the ``tp_version`` to the next -// available version and return 0. Otherwise, it will return an error code. +// and if the validation is passed, it will set the ``tp_version`` as valid +// tp_version_tag from the ``ty``. extern int _PyType_Validate(PyTypeObject *ty, _py_validate_type validate, unsigned int *tp_version); #ifdef __cplusplus From e68dc65b363a3496e350191869b9f02635ddada5 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 16 Nov 2024 11:28:55 +0900 Subject: [PATCH 11/15] Address code review --- Python/specialize.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index 1e4a13139eb664..35ea4df81fd761 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -2716,25 +2716,26 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) assert(_PyOpcode_Caches[TO_BOOL] == INLINE_CACHE_ENTRIES_TO_BOOL); _PyToBoolCache *cache = (_PyToBoolCache *)(instr + 1); PyObject *value = PyStackRef_AsPyObjectBorrow(value_o); + uint8_t specialized_op; if (PyBool_Check(value)) { - specialize(instr, TO_BOOL_BOOL); - return; + specialized_op = TO_BOOL_BOOL; + goto success; } if (PyLong_CheckExact(value)) { - specialize(instr, TO_BOOL_INT); - return; + specialized_op = TO_BOOL_INT; + goto success; } if (PyList_CheckExact(value)) { - specialize(instr, TO_BOOL_LIST); - return; + specialized_op = TO_BOOL_LIST; + goto success; } if (Py_IsNone(value)) { - specialize(instr, TO_BOOL_NONE); - return; + specialized_op = TO_BOOL_NONE; + goto success; } if (PyUnicode_CheckExact(value)) { - specialize(instr, TO_BOOL_STR); - return; + specialized_op = TO_BOOL_STR; + goto success; } if (PyType_HasFeature(Py_TYPE(value), Py_TPFLAGS_HEAPTYPE)) { unsigned int version = 0; @@ -2751,10 +2752,13 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) assert(err == 0); assert(version); write_u32(cache->version, version); - specialize(instr, TO_BOOL_ALWAYS_TRUE); - return; + specialized_op = TO_BOOL_ALWAYS_TRUE; + goto success; } unspecialize(instr, to_bool_fail_kind(value)); + return; +success: + specialize(instr, specialized_op); } static int From b27f9166335ea5af50985234ba37f8f468210d1f Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 16 Nov 2024 11:34:14 +0900 Subject: [PATCH 12/15] Address code review --- Python/specialize.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index 35ea4df81fd761..e46b34ecc32e61 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -2716,6 +2716,7 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) assert(_PyOpcode_Caches[TO_BOOL] == INLINE_CACHE_ENTRIES_TO_BOOL); _PyToBoolCache *cache = (_PyToBoolCache *)(instr + 1); PyObject *value = PyStackRef_AsPyObjectBorrow(value_o); + int reason; uint8_t specialized_op; if (PyBool_Check(value)) { specialized_op = TO_BOOL_BOOL; @@ -2741,12 +2742,12 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) unsigned int version = 0; int err = _PyType_Validate(Py_TYPE(value), check_type_always_true, &version); if (err < 0) { - unspecialize(instr, SPEC_FAIL_OUT_OF_VERSIONS); - return; + reason = SPEC_FAIL_OUT_OF_VERSIONS; + goto fail; } else if (err > 0) { - unspecialize(instr, err); - return; + reason = err; + goto fail; } assert(err == 0); @@ -2755,7 +2756,9 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) specialized_op = TO_BOOL_ALWAYS_TRUE; goto success; } - unspecialize(instr, to_bool_fail_kind(value)); + reason = to_bool_fail_kind(value); +fail: + unspecialize(instr, reason); return; success: specialize(instr, specialized_op); From 249efd18d76ba0f7e4a011689a8ecf9d7ac351a9 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 16 Nov 2024 11:55:18 +0900 Subject: [PATCH 13/15] nit --- Python/specialize.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index e46b34ecc32e61..8a0d9f141cd6f3 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -2743,11 +2743,11 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) int err = _PyType_Validate(Py_TYPE(value), check_type_always_true, &version); if (err < 0) { reason = SPEC_FAIL_OUT_OF_VERSIONS; - goto fail; + goto failure; } else if (err > 0) { reason = err; - goto fail; + goto failure; } assert(err == 0); @@ -2757,7 +2757,7 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) goto success; } reason = to_bool_fail_kind(value); -fail: +failure: unspecialize(instr, reason); return; success: From 6bc4c90ca2dbe4d436cfe283f7ed7c6bf84b49f6 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 22 Nov 2024 00:09:06 +0900 Subject: [PATCH 14/15] Address code review --- Python/specialize.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index 3c72dc39a7ba9b..c72562e9e544cf 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -2665,6 +2665,7 @@ _Py_Specialize_Send(_PyStackRef receiver_st, _Py_CODEUNIT *instr) cache->counter = adaptive_counter_cooldown(); } +#ifdef Py_STATS static int to_bool_fail_kind(PyObject *value) { @@ -2691,6 +2692,7 @@ to_bool_fail_kind(PyObject *value) } return SPEC_FAIL_OTHER; } +#endif // Py_STATS static int check_type_always_true(PyTypeObject *ty) @@ -2710,7 +2712,6 @@ check_type_always_true(PyTypeObject *ty) return 0; } - void _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) { @@ -2718,7 +2719,6 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) assert(_PyOpcode_Caches[TO_BOOL] == INLINE_CACHE_ENTRIES_TO_BOOL); _PyToBoolCache *cache = (_PyToBoolCache *)(instr + 1); PyObject *value = PyStackRef_AsPyObjectBorrow(value_o); - int reason; uint8_t specialized_op; if (PyBool_Check(value)) { specialized_op = TO_BOOL_BOOL; @@ -2744,11 +2744,11 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) unsigned int version = 0; int err = _PyType_Validate(Py_TYPE(value), check_type_always_true, &version); if (err < 0) { - reason = SPEC_FAIL_OUT_OF_VERSIONS; + SPECIALIZATION_FAIL(instr, SPEC_FAIL_OUT_OF_VERSIONS); goto failure; } else if (err > 0) { - reason = err; + SPECIALIZATION_FAIL(instr, err); goto failure; } @@ -2758,9 +2758,10 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) specialized_op = TO_BOOL_ALWAYS_TRUE; goto success; } - reason = to_bool_fail_kind(value); + + SPECIALIZATION_FAIL(instr, to_bool_fail_kind(value)); failure: - unspecialize(instr, reason); + unspecialize(instr); return; success: specialize(instr, specialized_op); From 542e4dd15dfbe1a01b918d9e2d4a2138d5690cca Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 22 Nov 2024 00:12:35 +0900 Subject: [PATCH 15/15] Use TO_BOOL for SPECIALIZATION_FAIL --- Python/specialize.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/specialize.c b/Python/specialize.c index c72562e9e544cf..f93c797df250de 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -2744,11 +2744,11 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) unsigned int version = 0; int err = _PyType_Validate(Py_TYPE(value), check_type_always_true, &version); if (err < 0) { - SPECIALIZATION_FAIL(instr, SPEC_FAIL_OUT_OF_VERSIONS); + SPECIALIZATION_FAIL(TO_BOOL, SPEC_FAIL_OUT_OF_VERSIONS); goto failure; } else if (err > 0) { - SPECIALIZATION_FAIL(instr, err); + SPECIALIZATION_FAIL(TO_BOOL, err); goto failure; } @@ -2759,7 +2759,7 @@ _Py_Specialize_ToBool(_PyStackRef value_o, _Py_CODEUNIT *instr) goto success; } - SPECIALIZATION_FAIL(instr, to_bool_fail_kind(value)); + SPECIALIZATION_FAIL(TO_BOOL, to_bool_fail_kind(value)); failure: unspecialize(instr); return;