From c92089ca4d369e0c9436f3ae5a59cfcfbe2665b8 Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Mon, 27 Jan 2025 17:18:29 +0100 Subject: [PATCH 1/2] Enable free-threaded specialization of LOAD_CONST. --- Lib/test/test_opcache.py | 14 ++++++++++++++ Python/bytecodes.c | 15 +++++++++++++-- Python/generated_cases.c.h | 15 +++++++++++++-- Tools/cases_generator/analyzer.py | 1 + 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_opcache.py b/Lib/test/test_opcache.py index 4d7304b1c9abb6..e8ea21f8179978 100644 --- a/Lib/test/test_opcache.py +++ b/Lib/test/test_opcache.py @@ -1773,6 +1773,20 @@ def compare_op_str(): self.assert_specialized(compare_op_str, "COMPARE_OP_STR") self.assert_no_opcode(compare_op_str, "COMPARE_OP") + @cpython_only + @requires_specialization_ft + def test_load_const(self): + def load_const(): + def unused(): pass + # Currently, the empty tuple is immortal, and the otherwise + # unused nested function's code object is mortal. This test will + # have to use different values if either of that changes. + return () + + load_const() + self.assert_specialized(load_const, "LOAD_CONST_IMMORTAL") + self.assert_specialized(load_const, "LOAD_CONST_MORTAL") + self.assert_no_opcode(load_const, "LOAD_CONST") if __name__ == "__main__": unittest.main() diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 940ea9d4da6e41..a445e7c806ccd9 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -294,10 +294,21 @@ dummy_func( * marshalling can intern strings and make them immortal. */ PyObject *obj = GETITEM(FRAME_CO_CONSTS, oparg); value = PyStackRef_FromPyObjectNew(obj); -#if ENABLE_SPECIALIZATION +#if ENABLE_SPECIALIZATION_FT +#ifdef Py_GIL_DISABLED + uint8_t expected = LOAD_CONST; + _Py_atomic_compare_exchange_uint8( + &this_instr->op.code, &expected, + _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL); + // We might lose a race with instrumentation, which we don't care about. + assert(expected >= MIN_INSTRUMENTED_OPCODE || + (expected == LOAD_CONST_IMMORTAL && _Py_IsImmortal(obj)) || + (expected == LOAD_CONST_MORTAL && !_Py_IsImmortal(obj))); +#else if (this_instr->op.code == LOAD_CONST) { this_instr->op.code = _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL; } +#endif #endif } @@ -2555,7 +2566,7 @@ dummy_func( } OPCODE_DEFERRED_INC(COMPARE_OP); ADVANCE_ADAPTIVE_COUNTER(this_instr[1].counter); - #endif /* ENABLE_SPECIALIZATION */ + #endif /* ENABLE_SPECIALIZATION_FT */ } op(_COMPARE_OP, (left, right -- res)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index cbb07a89d7db83..a832784b32694c 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3326,7 +3326,7 @@ } OPCODE_DEFERRED_INC(COMPARE_OP); ADVANCE_ADAPTIVE_COUNTER(this_instr[1].counter); - #endif /* ENABLE_SPECIALIZATION */ + #endif /* ENABLE_SPECIALIZATION_FT */ } // _COMPARE_OP { @@ -6027,11 +6027,22 @@ * marshalling can intern strings and make them immortal. */ PyObject *obj = GETITEM(FRAME_CO_CONSTS, oparg); value = PyStackRef_FromPyObjectNew(obj); - #if ENABLE_SPECIALIZATION + #if ENABLE_SPECIALIZATION_FT + #ifdef Py_GIL_DISABLED + uint8_t expected = LOAD_CONST; + _Py_atomic_compare_exchange_uint8( + &this_instr->op.code, &expected, + _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL); + // We might lose a race with instrumentation, which we don't care about. + assert(expected >= MIN_INSTRUMENTED_OPCODE || + (expected == LOAD_CONST_IMMORTAL && _Py_IsImmortal(obj)) || + (expected == LOAD_CONST_MORTAL && !_Py_IsImmortal(obj))); + #else if (this_instr->op.code == LOAD_CONST) { this_instr->op.code = _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL; } #endif + #endif stack_pointer[0] = value; stack_pointer += 1; assert(WITHIN_STACK_BOUNDS()); diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index bc9c42e045a610..b9293ff4b19951 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -634,6 +634,7 @@ def has_error_without_pop(op: parser.InstDef) -> bool: "_Py_STR", "_Py_TryIncrefCompare", "_Py_TryIncrefCompareStackRef", + "_Py_atomic_compare_exchange_uint8", "_Py_atomic_load_ptr_acquire", "_Py_atomic_load_uintptr_relaxed", "_Py_set_eval_breaker_bit", From a07d32f06a76069b5e2f82b1cb53c1bfa653e8dd Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Tue, 28 Jan 2025 21:09:03 +0100 Subject: [PATCH 2/2] Fix incorrect use of compare-exchange. --- Python/bytecodes.c | 13 ++++++------- Python/generated_cases.c.h | 13 ++++++------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index a445e7c806ccd9..17809ff31331c0 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -297,13 +297,12 @@ dummy_func( #if ENABLE_SPECIALIZATION_FT #ifdef Py_GIL_DISABLED uint8_t expected = LOAD_CONST; - _Py_atomic_compare_exchange_uint8( - &this_instr->op.code, &expected, - _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL); - // We might lose a race with instrumentation, which we don't care about. - assert(expected >= MIN_INSTRUMENTED_OPCODE || - (expected == LOAD_CONST_IMMORTAL && _Py_IsImmortal(obj)) || - (expected == LOAD_CONST_MORTAL && !_Py_IsImmortal(obj))); + if (!_Py_atomic_compare_exchange_uint8( + &this_instr->op.code, &expected, + _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL)) { + // We might lose a race with instrumentation, which we don't care about. + assert(expected >= MIN_INSTRUMENTED_OPCODE); + } #else if (this_instr->op.code == LOAD_CONST) { this_instr->op.code = _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL; diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index a832784b32694c..1602b1cab12584 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -6030,13 +6030,12 @@ #if ENABLE_SPECIALIZATION_FT #ifdef Py_GIL_DISABLED uint8_t expected = LOAD_CONST; - _Py_atomic_compare_exchange_uint8( - &this_instr->op.code, &expected, - _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL); - // We might lose a race with instrumentation, which we don't care about. - assert(expected >= MIN_INSTRUMENTED_OPCODE || - (expected == LOAD_CONST_IMMORTAL && _Py_IsImmortal(obj)) || - (expected == LOAD_CONST_MORTAL && !_Py_IsImmortal(obj))); + if (!_Py_atomic_compare_exchange_uint8( + &this_instr->op.code, &expected, + _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL)) { + // We might lose a race with instrumentation, which we don't care about. + assert(expected >= MIN_INSTRUMENTED_OPCODE); + } #else if (this_instr->op.code == LOAD_CONST) { this_instr->op.code = _Py_IsImmortal(obj) ? LOAD_CONST_IMMORTAL : LOAD_CONST_MORTAL;