gh-146381: Constant-fold frozendict subscript lookups via REPLACE_OPCODE_IF_EVALUATES_PURE#146490
gh-146381: Constant-fold frozendict subscript lookups via REPLACE_OPCODE_IF_EVALUATES_PURE#146490corona10 wants to merge 4 commits intopython:mainfrom
Conversation
corona10
requested review from
Fidget-Spinner,
markshannon,
savannahostrowski and
tomasr8
as code owners
|
@Fidget-Spinner |
main['_START_EXECUTOR', '_MAKE_WARM', '_SET_IP', '_CHECK_PERIODIC', '_CHECK_VALIDITY', '_ITER_CHECK_RANGE', '_GUARD_NOT_EXHAUSTED_RANGE', '_ITER_NEXT_RANGE', '_SET_IP', '_SWAP_FAST_2', '_SPILL_OR_RELOAD', '_POP_TOP', '_CHECK_VALIDITY', '_GUARD_GLOBALS_VERSION', '_LOAD_CONST_INLINE', '_LOAD_CONST_INLINE_BORROW', '_SET_IP', '_BINARY_OP_SUBSCR_DICT', '_POP_TOP_NOP', '_SPILL_OR_RELOAD', '_POP_TOP', '_CHECK_VALIDITY', '_LOAD_CONST_INLINE_BORROW', '_GUARD_NOS_INT', '_COMPARE_OP_INT', '_POP_TOP_NOP', '_POP_TOP_INT', '_GUARD_BIT_IS_SET_POP_5', '_LOAD_FAST_BORROW_1', '_LOAD_CONST_INLINE_BORROW', '_GUARD_NOS_INT', '_BINARY_OP_ADD_INT', '_POP_TOP_NOP', '_POP_TOP_NOP', '_SWAP_FAST_1', '_POP_TOP_INT', '_JUMP_TO_TOP', '_DEOPT', '_ERROR_POP_N', '_DEOPT', '_EXIT_TRACE', '_EXIT_TRACE', '_ERROR_POP_N', '_DEOPT', '_ERROR_POP_N', '_DEOPT', '_EXIT_TRACE', '_EXIT_TRACE', '_EXIT_TRACE'] PR['_START_EXECUTOR', '_MAKE_WARM', '_SET_IP', '_CHECK_PERIODIC', '_CHECK_VALIDITY', '_ITER_CHECK_RANGE', '_GUARD_NOT_EXHAUSTED_RANGE', '_ITER_NEXT_RANGE', '_SET_IP', '_SWAP_FAST_2', '_SPILL_OR_RELOAD', '_POP_TOP', '_CHECK_VALIDITY', '_GUARD_GLOBALS_VERSION', '_LOAD_CONST_INLINE', '_LOAD_CONST_INLINE_BORROW', '_SET_IP', '_BINARY_OP_SUBSCR_FROZEN_DICT', '_POP_TOP_NOP', '_SPILL_OR_RELOAD', '_POP_TOP', '_CHECK_VALIDITY', '_LOAD_CONST_INLINE_BORROW', '_INSERT_2_LOAD_CONST_INLINE_BORROW', '_POP_TOP_NOP', '_POP_TOP_INT', '_SET_IP', '_POP_TOP', '_CHECK_VALIDITY', '_LOAD_FAST_BORROW_1', '_LOAD_CONST_INLINE_BORROW', '_GUARD_NOS_INT', '_BINARY_OP_ADD_INT', '_POP_TOP_NOP', '_POP_TOP_NOP', '_SWAP_FAST_1', '_POP_TOP_INT', '_JUMP_TO_TOP', '_DEOPT', '_ERROR_POP_N', '_DEOPT', '_EXIT_TRACE', '_EXIT_TRACE', '_ERROR_POP_N', '_DEOPT', '_ERROR_POP_N', '_DEOPT', '_DEOPT', '_EXIT_TRACE'] |
Include/internal/pycore_optimizer.h
Outdated
| extern PyTypeObject *_Py_uop_sym_get_type(JitOptRef sym); | ||
| extern JitOptRef _Py_uop_sym_new_tuple(JitOptContext *ctx, int size, JitOptRef *args); | ||
| extern JitOptRef _Py_uop_sym_tuple_getitem(JitOptContext *ctx, JitOptRef sym, Py_ssize_t item); | ||
| extern JitOptRef _Py_uop_sym_frozendict_getitem(JitOptContext *ctx, JitOptRef sym, JitOptRef key); |
There was a problem hiding this comment.
I don't think we need this function.
Python/optimizer_symbols.c
Outdated
| return _Py_uop_sym_new_not_null(ctx); | ||
| } | ||
|
|
||
| JitOptRef |
There was a problem hiding this comment.
Same here, we don't need this function.
Python/optimizer_bytecodes.c
Outdated
| REPLACE_OP(this_instr, _BINARY_OP_SUBSCR_FROZEN_DICT, oparg, 0); | ||
| res = sym_frozendict_getitem(ctx, dict_st, sub_st); |
There was a problem hiding this comment.
| REPLACE_OP(this_instr, _BINARY_OP_SUBSCR_FROZEN_DICT, oparg, 0); | |
| res = sym_frozendict_getitem(ctx, dict_st, sub_st); | |
| REPLACE_OPCODE_IF_EVALUATES_PURE(dict_st, sub_st, res); |
Check out what is the generated code in executor_cases.c.h after this :).
Python/optimizer_bytecodes.c
Outdated
| op(_BINARY_OP_SUBSCR_FROZEN_DICT, (dict_st, sub_st -- res, ds, ss)) { | ||
| res = sym_new_not_null(ctx); | ||
| ds = dict_st; | ||
| ss = sub_st; | ||
| REPLACE_OPCODE_IF_EVALUATES_PURE(dict_st, sub_st, res); | ||
| } |
Lib/test/test_capi/test_opt.py
Outdated
| self.assertEqual(res, TIER2_THRESHOLD) | ||
| self.assertIsNotNone(ex) | ||
| uops = get_opnames(ex) | ||
| self.assertIn("_BINARY_OP_SUBSCR_FROZEN_DICT", uops) |
There was a problem hiding this comment.
After the correct replacement, this should just become POP_TOP_LOAD_CONST...
|
@Fidget-Spinner ['_START_EXECUTOR', '_MAKE_WARM', '_SET_IP', '_CHECK_PERIODIC', '_CHECK_VALIDITY', '_ITER_CHECK_RANGE', '_GUARD_NOT_EXHAUSTED_RANGE', '_ITER_NEXT_RANGE', '_SET_IP', '_SWAP_FAST_2', '_SPILL_OR_RELOAD', '_POP_TOP', '_CHECK_VALIDITY', '_GUARD_GLOBALS_VERSION', '_LOAD_CONST_INLINE', '_LOAD_CONST_INLINE_BORROW', '_SET_IP', '_INSERT_2_LOAD_CONST_INLINE_BORROW', '_POP_TOP_NOP', '_SPILL_OR_RELOAD', '_POP_TOP', '_CHECK_VALIDITY', '_LOAD_CONST_INLINE_BORROW', '_INSERT_2_LOAD_CONST_INLINE_BORROW', '_POP_TOP_NOP', '_POP_TOP_INT', '_SET_IP', '_POP_TOP', '_CHECK_VALIDITY', '_LOAD_FAST_BORROW_1', '_LOAD_CONST_INLINE_BORROW', '_GUARD_NOS_INT', '_BINARY_OP_ADD_INT', '_POP_TOP_NOP', '_POP_TOP_NOP', '_SWAP_FAST_1', '_POP_TOP_INT', '_JUMP_TO_TOP', '_DEOPT', '_ERROR_POP_N', '_DEOPT', '_EXIT_TRACE', '_EXIT_TRACE', '_ERROR_POP_N', '_DEOPT', '_DEOPT', '_DEOPT', '_EXIT_TRACE'] |
corona10
changed the title
3 participants
Uh oh!
There was an error while loading. Please reload this page.