Repository navigation
GH-131798: Remove JIT guards for dict, frozenset, list, set, and tuple - #132289
brandtbucher merged 7 commits into
Conversation
Sorry, something went wrong.
|
@Zheaoli and @tomasr8, you might be interested in checking this out. Specifically, see the changes in Python/bytecodes.c which use the same syntax as Python/optimizer_bytecodes.c, but describes the actual implementations of these instructions. You can see that this PR breaks up the old, larger instr definitions into smaller op definitions combined into a macro. This doesn't change the semantics of the instruction itself, but allows the JIT to remove parts of the instruction as it sees fit (in this case, separating out the type checks allows the JIT to remove them). |
Sorry, something went wrong.
Fidget-Spinner
left a comment
There was a problem hiding this comment.
Just one concern.
Sorry, something went wrong.
| op(_TO_BOOL_LIST, (value -- res)) { | ||
| int already_bool = optimize_to_bool(this_instr, ctx, value, &res); | ||
| if (!already_bool) { | ||
| sym_set_type(value, &PyList_Type); |
There was a problem hiding this comment.
Does CONTAIN_OP_SET and CONTAIN_OP_DICT need to remove their sym setting type as well? Or is that not even happening at the moment?
Sorry, something went wrong.
There was a problem hiding this comment.
Not happening currently,. _CONTAINS_OP_SET can't even set anything, since it handles both set and frozenset (this might be worth re-evaluating, or splitting up).
Sorry, something went wrong.
Thanks! I think I might need some time to understand this PR! Thanks for your patience! |
Sorry, something went wrong.
|
Going to go ahead and merge this to reduce the risk of merge conflicts, but let me know if you have any questions @Zheaoli! |
Sorry, something went wrong.
According to the stats, this removes:
It also fixes a bug that I encountered in the optimizer's logic for _UNPACK_SEQUENCE_TUPLE, which puts items on the stack in reverse order.