gh-145214: Narrow _GUARD_TOS_ANY_{SET,DICT} by using probable type#145215
gh-145214: Narrow _GUARD_TOS_ANY_{SET,DICT} by using probable type#145215corona10 wants to merge 3 commits intopython:mainfrom
Conversation
Python/optimizer_bytecodes.c
Outdated
| PyTypeObject *tp = sym_get_type(tos); | ||
| if (tp == &PySet_Type || tp == &PyFrozenSet_Type) { | ||
| ADD_OP(_NOP, 0, 0); | ||
| sym_set_type(tos, tp); |
There was a problem hiding this comment.
Why set the type when it is already known?
There was a problem hiding this comment.
Okay, now I get it means that setting the type at this moment is meaningless.
markshannon
left a comment
There was a problem hiding this comment.
What we could do here is narrow the guard, if the probable type is either a set or frozen set.
So, leaving the existing check alone, add the following:
tp = sym_get_probable_type(tos);
if (tp == &PySet_Type) {
ADD_OP(_GUARD_TOS_SET, 0, 0);
}
Likewise for frozen sets, and we could also do the same for dict guards.
|
I just added a new commit:
I believe this should work, but I’m not entirely sure that I implemented _Py_uop_sym_get_probable_type correctly. |
| } | ||
|
|
||
| PyTypeObject * | ||
| _Py_uop_sym_get_probable_type(JitOptRef ref) |
| if (sym_matches_type(tos, &PySet_Type)) { | ||
| ADD_OP(_NOP, 0, 0); | ||
| } | ||
| sym_set_type(tos, &PySet_Type); |
There was a problem hiding this comment.
We also don't need sym_set_type here? Because we checked the type in sym_matches_type.
There was a problem hiding this comment.
I’m not sure why we mark the types of tuples and other objects this way.
Maybe we need to better understand the original intention behind it.
cpython/Python/optimizer_bytecodes.c
Lines 1323 to 1349 in e234662
Co-authored-by: Ken Jin <kenjin@python.org>
Uh oh!
There was an error while loading. Please reload this page.