Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34.1k
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 intopython:mainfrom
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We can use _PyList_GetItemRef for both ft and normal builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seems right.@corona10 I copied this over from herehttps://github.com/python/cpython/blame/8e57877e3f43e80762196f6869526d0c1585783a/Python/bytecodes.c#L869
Do you remember why we have the two versions?
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What if the list is resized after the guard has been executed and before the action is executed. Then_PyList_GetItemRef can return NULL.
iritkatriel commentedFeb 5, 2025
Closing in favour of#129700. |
Uh oh!
There was an error while loading.Please reload this page.