gh-100239: replace BINARY_SUBSCR & family by BINARY_OP with oparg NB_SUBSCR - #129379
gh-100239: replace BINARY_SUBSCR & family by BINARY_OP with oparg NB_SUBSCR#129379iritkatriel wants to merge 11 commits into
Conversation
| return 0; | ||
| } | ||
| PyListObject* list = (PyListObject*)container; | ||
| Py_ssize_t index = index_obj->long_value.ob_digit[0]; |
There was a problem hiding this comment.
Do we need to check the index in the guard? We need to check it is nonnegative, but the other checks are handled also in the action by _PyList_GetItemRef. There will be no deopt, but that is no issue perhaps.
Sorry, something went wrong.
There was a problem hiding this comment.
We use the guard both to decide whether to optimise with this action, and then we call it from the action to decide whether to deopt. So once we pass the guard we should know that we're not deopting.
Sorry, something went wrong.
| STAT_INC(BINARY_SUBSCR, hit); | ||
| Py_ssize_t index = ((PyLongObject*)sub)->long_value.ob_digit[0]; | ||
| PyListObject* list = (PyListObject*)container; | ||
| #ifdef Py_GIL_DISABLED |
There was a problem hiding this comment.
We can use _PyList_GetItemRef for both ft and normal builds.
Sorry, something went wrong.
There was a problem hiding this comment.
Seems right. @corona10 I copied this over from here https://github.com/python/cpython/blame/8e57877e3f43e80762196f6869526d0c1585783a/Python/bytecodes.c#L869
Do you remember why we have the two versions?
Sorry, something went wrong.
| PyListObject* list = (PyListObject*)container; | ||
| #ifdef Py_GIL_DISABLED | ||
| PyObject *res_o = _PyList_GetItemRef(list, index); | ||
| assert(res_o != NULL); |
There was a problem hiding this comment.
What if the list is resized after the guard has been executed and before the action is executed. Then _PyList_GetItemRef can return NULL.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.