Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-115999: Enable BINARY_SUBSCR_GETITEM for free-threaded build #127737

Merged
merged 18 commits into from
Dec 19, 2024
4 changes: 2 additions & 2 deletions Include/internal/pycore_opcode_metadata.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Include/internal/pycore_typeobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ typedef int (*_py_validate_type)(PyTypeObject *);
// 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);
extern int _PyType_CacheGetItemForSpecialization(PyHeapTypeObject *ht, PyObject *descriptor, uint32_t tp_version);

#ifdef __cplusplus
}
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_uop_metadata.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 18 additions & 1 deletion Lib/test/test_opcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@ def write(items):
opname = "STORE_SUBSCR_LIST_INT"
self.assert_races_do_not_crash(opname, get_items, read, write)

@requires_specialization
@requires_specialization_ft
def test_unpack_sequence_list(self):
def get_items():
items = []
Expand Down Expand Up @@ -1245,6 +1245,14 @@ def f(o, n):
f(test_obj, 1)
self.assertEqual(test_obj.b, 0)

# gh-127274: BINARY_SUBSCR_GETITEM will only cache __getitem__ methods that
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be removable after #128012 is merged.

# are deferred. We only defer functions defined at the top-level.
class CGetItem:
def __init__(self, val):
self.val = val
def __getitem__(self, item):
return self.val


class TestSpecializer(TestBase):

Expand Down Expand Up @@ -1520,6 +1528,15 @@ def binary_subscr_str_int():
self.assert_specialized(binary_subscr_str_int, "BINARY_SUBSCR_STR_INT")
self.assert_no_opcode(binary_subscr_str_int, "BINARY_SUBSCR")

def binary_subscr_getitems():
items = [CGetItem(i) for i in range(100)]
for i in range(100):
self.assertEqual(items[i][i], i)

binary_subscr_getitems()
self.assert_specialized(binary_subscr_getitems, "BINARY_SUBSCR_GETITEM")
self.assert_no_opcode(binary_subscr_getitems, "BINARY_SUBSCR")


if __name__ == "__main__":
unittest.main()
25 changes: 25 additions & 0 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -5692,6 +5692,31 @@ _PyType_CacheInitForSpecialization(PyHeapTypeObject *type, PyObject *init,
return can_cache;
}

int
_PyType_CacheGetItemForSpecialization(PyHeapTypeObject *ht, PyObject *descriptor, uint32_t tp_version)
{
if (!descriptor || !tp_version) {
return 0;
}
int can_cache;
BEGIN_TYPE_LOCK();
can_cache = ((PyTypeObject*)ht)->tp_version_tag == tp_version;
// This pointer is invalidated by PyType_Modified (see the comment on
// struct _specialization_cache):
PyFunctionObject *func = (PyFunctionObject *)descriptor;
uint32_t version = _PyFunction_GetVersionForCurrentState(func);
can_cache = can_cache && _PyFunction_IsVersionValid(version);
#ifdef Py_GIL_DISABLED
can_cache = can_cache && _PyObject_HasDeferredRefcount(descriptor);
#endif
if (can_cache) {
FT_ATOMIC_STORE_PTR_RELEASE(ht->_spec_cache.getitem, descriptor);
FT_ATOMIC_STORE_UINT32_RELAXED(ht->_spec_cache.getitem_version, version);
}
END_TYPE_LOCK();
return can_cache;
}

static void
set_flags(PyTypeObject *self, unsigned long mask, unsigned long flags)
{
Expand Down
2 changes: 1 addition & 1 deletion Programs/test_frozenmain.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 10 additions & 12 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -864,26 +864,24 @@ dummy_func(
res = PyStackRef_FromPyObjectSteal(res_o);
}

op(_BINARY_SUBSCR_CHECK_FUNC, (container, unused -- container, unused)) {
op(_BINARY_SUBSCR_CHECK_FUNC, (container, unused -- container, unused, getitem)) {
PyTypeObject *tp = Py_TYPE(PyStackRef_AsPyObjectBorrow(container));
DEOPT_IF(!PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE));
PyHeapTypeObject *ht = (PyHeapTypeObject *)tp;
PyObject *getitem = ht->_spec_cache.getitem;
DEOPT_IF(getitem == NULL);
assert(PyFunction_Check(getitem));
uint32_t cached_version = ht->_spec_cache.getitem_version;
DEOPT_IF(((PyFunctionObject *)getitem)->func_version != cached_version);
PyCodeObject *code = (PyCodeObject *)PyFunction_GET_CODE(getitem);
PyObject *getitem_o = FT_ATOMIC_LOAD_PTR_ACQUIRE(ht->_spec_cache.getitem);
DEOPT_IF(getitem_o == NULL);
assert(PyFunction_Check(getitem_o));
uint32_t cached_version = FT_ATOMIC_LOAD_UINT32_RELAXED(ht->_spec_cache.getitem_version);
DEOPT_IF(((PyFunctionObject *)getitem_o)->func_version != cached_version);
PyCodeObject *code = (PyCodeObject *)PyFunction_GET_CODE(getitem_o);
assert(code->co_argcount == 2);
DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize));
getitem = PyStackRef_FromPyObjectNew(getitem_o);
STAT_INC(BINARY_SUBSCR, hit);
}

op(_BINARY_SUBSCR_INIT_CALL, (container, sub -- new_frame: _PyInterpreterFrame* )) {
PyTypeObject *tp = Py_TYPE(PyStackRef_AsPyObjectBorrow(container));
PyHeapTypeObject *ht = (PyHeapTypeObject *)tp;
PyObject *getitem = ht->_spec_cache.getitem;
new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(getitem), 2, frame);
op(_BINARY_SUBSCR_INIT_CALL, (container, sub, getitem -- new_frame: _PyInterpreterFrame* )) {
new_frame = _PyFrame_PushUnchecked(tstate, getitem, 2, frame);
new_frame->localsplus[0] = container;
new_frame->localsplus[1] = sub;
INPUTS_DEAD();
Expand Down
32 changes: 18 additions & 14 deletions Python/executor_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 9 additions & 10 deletions Python/generated_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions Python/optimizer_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 19 additions & 16 deletions Python/specialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,7 @@ specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject* na
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_ATTR_METHOD);
return -1;
}
/* Don't specialize if PEP 523 is active */
if (_PyInterpreterState_GET()->eval_frame) {
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER);
return -1;
Expand Down Expand Up @@ -1154,6 +1155,7 @@ specialize_instance_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject* na
if (version == 0) {
return -1;
}
/* Don't specialize if PEP 523 is active */
if (_PyInterpreterState_GET()->eval_frame) {
SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OTHER);
return -1;
Expand Down Expand Up @@ -1761,43 +1763,42 @@ _Py_Specialize_BinarySubscr(
specialized_op = BINARY_SUBSCR_DICT;
goto success;
}
#ifndef Py_GIL_DISABLED
PyTypeObject *cls = Py_TYPE(container);
PyObject *descriptor = _PyType_Lookup(cls, &_Py_ID(__getitem__));
unsigned int tp_version;
PyObject *descriptor = _PyType_LookupRefAndVersion(container_type, &_Py_ID(__getitem__), &tp_version);
if (descriptor && Py_TYPE(descriptor) == &PyFunction_Type) {
if (!(container_type->tp_flags & Py_TPFLAGS_HEAPTYPE)) {
SPECIALIZATION_FAIL(BINARY_SUBSCR, SPEC_FAIL_SUBSCR_NOT_HEAP_TYPE);
Py_DECREF(descriptor);
goto fail;
}
PyFunctionObject *func = (PyFunctionObject *)descriptor;
PyCodeObject *fcode = (PyCodeObject *)func->func_code;
int kind = function_kind(fcode);
if (kind != SIMPLE_FUNCTION) {
SPECIALIZATION_FAIL(BINARY_SUBSCR, kind);
Py_DECREF(descriptor);
goto fail;
}
if (fcode->co_argcount != 2) {
SPECIALIZATION_FAIL(BINARY_SUBSCR, SPEC_FAIL_WRONG_NUMBER_ARGUMENTS);
Py_DECREF(descriptor);
goto fail;
}
uint32_t version = _PyFunction_GetVersionForCurrentState(func);
if (!_PyFunction_IsVersionValid(version)) {
SPECIALIZATION_FAIL(BINARY_SUBSCR, SPEC_FAIL_OUT_OF_VERSIONS);
goto fail;
}

PyHeapTypeObject *ht = (PyHeapTypeObject *)container_type;
/* Don't specialize if PEP 523 is active */
if (_PyInterpreterState_GET()->eval_frame) {
SPECIALIZATION_FAIL(BINARY_SUBSCR, SPEC_FAIL_OTHER);
Py_DECREF(descriptor);
goto fail;
}
PyHeapTypeObject *ht = (PyHeapTypeObject *)container_type;
// This pointer is invalidated by PyType_Modified (see the comment on
// struct _specialization_cache):
ht->_spec_cache.getitem = descriptor;
ht->_spec_cache.getitem_version = version;
specialized_op = BINARY_SUBSCR_GETITEM;
goto success;
if (_PyType_CacheGetItemForSpecialization(ht, descriptor, (uint32_t)tp_version)) {
specialized_op = BINARY_SUBSCR_GETITEM;
Py_DECREF(descriptor);
goto success;
}
}
#endif // Py_GIL_DISABLED
Py_XDECREF(descriptor);
SPECIALIZATION_FAIL(BINARY_SUBSCR,
binary_subscr_fail_kind(container_type, sub));
fail:
Expand Down Expand Up @@ -2606,6 +2607,7 @@ _Py_Specialize_ForIter(_PyStackRef iter, _Py_CODEUNIT *instr, int oparg)
assert(instr[oparg + INLINE_CACHE_ENTRIES_FOR_ITER + 1].op.code == END_FOR ||
instr[oparg + INLINE_CACHE_ENTRIES_FOR_ITER + 1].op.code == INSTRUMENTED_END_FOR
);
/* Don't specialize if PEP 523 is active */
if (_PyInterpreterState_GET()->eval_frame) {
SPECIALIZATION_FAIL(FOR_ITER, SPEC_FAIL_OTHER);
goto failure;
Expand Down Expand Up @@ -2634,6 +2636,7 @@ _Py_Specialize_Send(_PyStackRef receiver_st, _Py_CODEUNIT *instr)
assert(_PyOpcode_Caches[SEND] == INLINE_CACHE_ENTRIES_SEND);
PyTypeObject *tp = Py_TYPE(receiver);
if (tp == &PyGen_Type || tp == &PyCoro_Type) {
/* Don't specialize if PEP 523 is active */
if (_PyInterpreterState_GET()->eval_frame) {
SPECIALIZATION_FAIL(SEND, SPEC_FAIL_OTHER);
goto failure;
Expand Down
Loading