Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-100239: replace BINARY_SUBSCR & family by BINARY_OP with oparg NB_SUBSCR#129379
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| return0; | ||
| } | ||
| PyListObject*list= (PyListObject*)container; | ||
| Py_ssize_tindex=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_tindex= ((PyLongObject*)sub)->long_value.ob_digit[0]; | ||
| PyListObject*list= (PyListObject*)container; | ||
| #ifdefPy_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; | ||
| #ifdefPy_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.
Closing in favour of#129700. |
Uh oh!
There was an error while loading.Please reload this page.