From 20e61eedb6bc536cced7c1a408ea824699e88287 Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Wed, 5 Feb 2025 21:34:49 +0100 Subject: [PATCH 01/12] Make `is_constant_sequence` NOP aware --- Python/flowgraph.c | 97 ++++++++++++++++++++++++---------------------- 1 file changed, 51 insertions(+), 46 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 12eedc33e42621..58f69609f94741 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -1339,14 +1339,21 @@ add_const(PyObject *newconst, PyObject *consts, PyObject *const_cache) } static bool -is_constant_sequence(cfg_instr *inst, int n) +is_constant_sequence(basicblock *bb, int start, int seq_size, int *seq_start) { - for (int i = 0; i < n; i++) { - if(!loads_const(inst[i].i_opcode)) { + assert(start < bb->b_iused); + while (start >= 0 && seq_size > 0) { + cfg_instr *instr = &bb->b_instr[start--]; + if (instr->i_opcode == NOP) { + continue; + } + if (!loads_const(instr->i_opcode)) { return false; } + seq_size--; } - return true; + *seq_start = start + 1; + return seq_size == 0; } /* Replace LOAD_CONST c1, LOAD_CONST c2 ... LOAD_CONST cn, BUILD_TUPLE n @@ -1356,42 +1363,45 @@ is_constant_sequence(cfg_instr *inst, int n) Called with codestr pointing to the first LOAD_CONST. */ static int -fold_tuple_on_constants(PyObject *const_cache, - cfg_instr *inst, - int n, PyObject *consts) +fold_tuple_on_constants(basicblock *bb, int n, PyObject *consts, PyObject *const_cache) { /* Pre-conditions */ assert(PyDict_CheckExact(const_cache)); assert(PyList_CheckExact(consts)); - assert(inst[n].i_opcode == BUILD_TUPLE); - assert(inst[n].i_oparg == n); + cfg_instr *instr = &bb->b_instr[n]; + assert(instr->i_opcode == BUILD_TUPLE); - if (!is_constant_sequence(inst, n)) { + int seq_size = instr->i_oparg; + int seq_start; + if (!is_constant_sequence(bb, n-1, seq_size, &seq_start)) { return SUCCESS; } /* Buildup new tuple of constants */ - PyObject *newconst = PyTuple_New(n); + PyObject *newconst = PyTuple_New(seq_size); if (newconst == NULL) { return ERROR; } - for (int i = 0; i < n; i++) { - int op = inst[i].i_opcode; - int arg = inst[i].i_oparg; + for (int i = seq_start, tuple_i = 0; i < n; i++) { + int op = bb->b_instr[i].i_opcode; + if (op == NOP) { + continue; + } + int arg = bb->b_instr[i].i_oparg; PyObject *constant = get_const_value(op, arg, consts); if (constant == NULL) { return ERROR; } - PyTuple_SET_ITEM(newconst, i, constant); + PyTuple_SET_ITEM(newconst, tuple_i++, constant); } int index = add_const(newconst, consts, const_cache); if (index < 0) { return ERROR; } - for (int i = 0; i < n; i++) { - INSTR_SET_OP0(&inst[i], NOP); + for (int i = seq_start; i < n; i++) { + INSTR_SET_OP0(&bb->b_instr[i], NOP); } - INSTR_SET_OP1(&inst[n], LOAD_CONST, index); + INSTR_SET_OP1(&bb->b_instr[n], LOAD_CONST, index); return SUCCESS; } @@ -1401,32 +1411,35 @@ fold_tuple_on_constants(PyObject *const_cache, or BUILD_SET & SET_UPDATE respectively. */ static int -optimize_if_const_list_or_set(PyObject *const_cache, cfg_instr* inst, int n, PyObject *consts) +optimize_if_const_list_or_set(basicblock *bb, int n, PyObject *consts, PyObject *const_cache) { assert(PyDict_CheckExact(const_cache)); assert(PyList_CheckExact(consts)); - assert(inst[n].i_oparg == n); - - int build = inst[n].i_opcode; - assert(build == BUILD_LIST || build == BUILD_SET); - int extend = build == BUILD_LIST ? LIST_EXTEND : SET_UPDATE; - - if (n < MIN_CONST_SEQUENCE_SIZE || !is_constant_sequence(inst, n)) { + cfg_instr *instr = &bb->b_instr[n]; + assert(instr->i_opcode == BUILD_LIST || instr->i_opcode == BUILD_SET); + int seq_size = instr->i_oparg; + int seq_start; + if (seq_size < MIN_CONST_SEQUENCE_SIZE || !is_constant_sequence(bb, n-1, seq_size, &seq_start)) { return SUCCESS; } - PyObject *newconst = PyTuple_New(n); + PyObject *newconst = PyTuple_New(seq_size); if (newconst == NULL) { return ERROR; } - for (int i = 0; i < n; i++) { - int op = inst[i].i_opcode; - int arg = inst[i].i_oparg; + for (int i = seq_start, tuple_i = 0; i < n; i++) { + int op = bb->b_instr[i].i_opcode; + if (op == NOP) { + continue; + } + int arg = bb->b_instr[i].i_oparg; PyObject *constant = get_const_value(op, arg, consts); if (constant == NULL) { return ERROR; } - PyTuple_SET_ITEM(newconst, i, constant); + PyTuple_SET_ITEM(newconst, tuple_i++, constant); } + int build = instr->i_opcode; + int extend = build == BUILD_LIST ? LIST_EXTEND : SET_UPDATE; if (build == BUILD_SET) { PyObject *frozenset = PyFrozenSet_New(newconst); if (frozenset == NULL) { @@ -1436,12 +1449,12 @@ optimize_if_const_list_or_set(PyObject *const_cache, cfg_instr* inst, int n, PyO } int index = add_const(newconst, consts, const_cache); RETURN_IF_ERROR(index); - INSTR_SET_OP1(&inst[0], build, 0); - for (int i = 1; i < n - 1; i++) { - INSTR_SET_OP0(&inst[i], NOP); + INSTR_SET_OP1(&bb->b_instr[seq_start], build, 0); + for (int i = seq_start + 1; i < n - 1; i++) { + INSTR_SET_OP0(&bb->b_instr[i], NOP); } - INSTR_SET_OP1(&inst[n-1], LOAD_CONST, index); - INSTR_SET_OP1(&inst[n], extend, 1); + INSTR_SET_OP1(&bb->b_instr[n-1], LOAD_CONST, index); + INSTR_SET_OP1(&bb->b_instr[n], extend, 1); return SUCCESS; } @@ -1894,19 +1907,11 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts) continue; } } - if (i >= oparg) { - if (fold_tuple_on_constants(const_cache, inst-oparg, oparg, consts)) { - goto error; - } - } + RETURN_IF_ERROR(fold_tuple_on_constants(bb, i, consts, const_cache)); break; case BUILD_LIST: case BUILD_SET: - if (i >= oparg) { - if (optimize_if_const_list_or_set(const_cache, inst-oparg, oparg, consts) < 0) { - goto error; - } - } + RETURN_IF_ERROR(optimize_if_const_list_or_set(bb, i, consts, const_cache)); break; case POP_JUMP_IF_NOT_NONE: case POP_JUMP_IF_NONE: From 8b6d294d9ad337cc4a14f33f1e1489c42292d127 Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Thu, 6 Feb 2025 16:10:33 +0100 Subject: [PATCH 02/12] implement `get_constant_sequence` --- Python/flowgraph.c | 133 +++++++++++++++++++++++++-------------------- 1 file changed, 74 insertions(+), 59 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 58f69609f94741..f716235c1ef378 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -264,7 +264,7 @@ basicblock_insert_instruction(basicblock *block, int pos, cfg_instr *instr) { } /* For debugging purposes only */ -#if 0 +#if 1 static void dump_instr(cfg_instr *i) { @@ -1338,22 +1338,67 @@ add_const(PyObject *newconst, PyObject *consts, PyObject *const_cache) return (int)index; } -static bool -is_constant_sequence(basicblock *bb, int start, int seq_size, int *seq_start) +/* + Walk basic block upwards starting from "start" trying to collect "size" number of + subsequent constants from instructions loading constants into new tuple ignoring NOP's in between. + + Returns -1 on error and sets "seq" to NULL. + Returns 0 on success and sets "seq" to NULL if failed to collect requested number of constants. + Returns 0 on success and sets "seq" to resulting tuple if succeeded to collect requested number of constants. +*/ +static int +get_constant_sequence(basicblock *bb, int start, int size, + PyObject *consts, PyObject **seq) { assert(start < bb->b_iused); - while (start >= 0 && seq_size > 0) { - cfg_instr *instr = &bb->b_instr[start--]; + *seq = NULL; + PyObject *res = PyTuple_New((Py_ssize_t)size); + if (res == NULL) { + return -1; + } + for (; start >= 0 && size > 0; start--) { + cfg_instr *instr = &bb->b_instr[start]; if (instr->i_opcode == NOP) { continue; } if (!loads_const(instr->i_opcode)) { - return false; + break; } - seq_size--; + PyObject *constant = get_const_value(instr->i_opcode, instr->i_oparg, consts); + if (constant == NULL) { + Py_DECREF(res); + return -1; + } + PyTuple_SET_ITEM(res, --size, constant); + } + if (size > 0) { + Py_DECREF(res); } - *seq_start = start + 1; - return seq_size == 0; + else { + *seq = res; + } + return 0; +} + +/* + Walk basic block upwards starting from "start" and change "count" number of + non-NOP instructions to NOP's, returning index of first instruction before + last placed NOP. Returns -1 if last placed NOP was first instruction in the block. +*/ +static int +nop_out(basicblock *bb, int start, int count) +{ + assert(start < bb->b_iused); + for (; count > 0; start--) { + assert(start >= 0); + if (bb->b_instr[start].i_opcode == NOP) { + continue; + } + INSTR_SET_OP0(&bb->b_instr[start], NOP); + count--; + } + assert(start >= -1); + return start; } /* Replace LOAD_CONST c1, LOAD_CONST c2 ... LOAD_CONST cn, BUILD_TUPLE n @@ -1363,45 +1408,24 @@ is_constant_sequence(basicblock *bb, int start, int seq_size, int *seq_start) Called with codestr pointing to the first LOAD_CONST. */ static int -fold_tuple_on_constants(basicblock *bb, int n, PyObject *consts, PyObject *const_cache) +fold_tuple_of_constants(basicblock *bb, int n, PyObject *consts, PyObject *const_cache) { /* Pre-conditions */ assert(PyDict_CheckExact(const_cache)); assert(PyList_CheckExact(consts)); cfg_instr *instr = &bb->b_instr[n]; assert(instr->i_opcode == BUILD_TUPLE); - int seq_size = instr->i_oparg; - int seq_start; - if (!is_constant_sequence(bb, n-1, seq_size, &seq_start)) { - return SUCCESS; - } - - /* Buildup new tuple of constants */ - PyObject *newconst = PyTuple_New(seq_size); + PyObject *newconst; + RETURN_IF_ERROR(get_constant_sequence(bb, n-1, seq_size, consts, &newconst)); if (newconst == NULL) { - return ERROR; - } - for (int i = seq_start, tuple_i = 0; i < n; i++) { - int op = bb->b_instr[i].i_opcode; - if (op == NOP) { - continue; - } - int arg = bb->b_instr[i].i_oparg; - PyObject *constant = get_const_value(op, arg, consts); - if (constant == NULL) { - return ERROR; - } - PyTuple_SET_ITEM(newconst, tuple_i++, constant); + return SUCCESS; } + assert(PyTuple_GET_SIZE(newconst) == seq_size); int index = add_const(newconst, consts, const_cache); - if (index < 0) { - return ERROR; - } - for (int i = seq_start; i < n; i++) { - INSTR_SET_OP0(&bb->b_instr[i], NOP); - } + RETURN_IF_ERROR(index); INSTR_SET_OP1(&bb->b_instr[n], LOAD_CONST, index); + (void)nop_out(bb, n-1, seq_size); return SUCCESS; } @@ -1418,43 +1442,34 @@ optimize_if_const_list_or_set(basicblock *bb, int n, PyObject *consts, PyObject cfg_instr *instr = &bb->b_instr[n]; assert(instr->i_opcode == BUILD_LIST || instr->i_opcode == BUILD_SET); int seq_size = instr->i_oparg; - int seq_start; - if (seq_size < MIN_CONST_SEQUENCE_SIZE || !is_constant_sequence(bb, n-1, seq_size, &seq_start)) { + if (seq_size < MIN_CONST_SEQUENCE_SIZE) { return SUCCESS; } - PyObject *newconst = PyTuple_New(seq_size); + PyObject *newconst; + RETURN_IF_ERROR(get_constant_sequence(bb, n-1, seq_size, consts, &newconst)); if (newconst == NULL) { - return ERROR; - } - for (int i = seq_start, tuple_i = 0; i < n; i++) { - int op = bb->b_instr[i].i_opcode; - if (op == NOP) { - continue; - } - int arg = bb->b_instr[i].i_oparg; - PyObject *constant = get_const_value(op, arg, consts); - if (constant == NULL) { - return ERROR; - } - PyTuple_SET_ITEM(newconst, tuple_i++, constant); + return SUCCESS; } + assert(PyTuple_GET_SIZE(newconst) == seq_size); int build = instr->i_opcode; int extend = build == BUILD_LIST ? LIST_EXTEND : SET_UPDATE; if (build == BUILD_SET) { PyObject *frozenset = PyFrozenSet_New(newconst); if (frozenset == NULL) { + Py_DECREF(newconst); return ERROR; } Py_SETREF(newconst, frozenset); } int index = add_const(newconst, consts, const_cache); RETURN_IF_ERROR(index); - INSTR_SET_OP1(&bb->b_instr[seq_start], build, 0); - for (int i = seq_start + 1; i < n - 1; i++) { - INSTR_SET_OP0(&bb->b_instr[i], NOP); - } - INSTR_SET_OP1(&bb->b_instr[n-1], LOAD_CONST, index); INSTR_SET_OP1(&bb->b_instr[n], extend, 1); + INSTR_SET_OP1(&bb->b_instr[n-1], LOAD_CONST, index); + int i = nop_out(bb, n-2, seq_size-2); + for (; i > 0 && bb->b_instr[i].i_opcode == NOP; i--); + assert(i >= 0); + assert(loads_const(bb->b_instr[i].i_opcode)); + INSTR_SET_OP1(&bb->b_instr[i], build, 0); return SUCCESS; } @@ -1907,7 +1922,7 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts) continue; } } - RETURN_IF_ERROR(fold_tuple_on_constants(bb, i, consts, const_cache)); + RETURN_IF_ERROR(fold_tuple_of_constants(bb, i, consts, const_cache)); break; case BUILD_LIST: case BUILD_SET: From 0845cd7f1b98ba0378379249d883ce0655016b99 Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Thu, 6 Feb 2025 16:19:16 +0100 Subject: [PATCH 03/12] disable debugging code --- Python/flowgraph.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index f716235c1ef378..3e7db43586f625 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -264,7 +264,7 @@ basicblock_insert_instruction(basicblock *block, int pos, cfg_instr *instr) { } /* For debugging purposes only */ -#if 1 +#if 0 static void dump_instr(cfg_instr *i) { From d3768ac8d267fcd751a3a53a0fe6e751e69f3dc5 Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Thu, 6 Feb 2025 18:57:04 +0100 Subject: [PATCH 04/12] address review --- Python/flowgraph.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 3e7db43586f625..49782d8b046f40 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -1339,12 +1339,12 @@ add_const(PyObject *newconst, PyObject *consts, PyObject *const_cache) } /* - Walk basic block upwards starting from "start" trying to collect "size" number of + Walk basic block backwards starting from "start" trying to collect "size" number of subsequent constants from instructions loading constants into new tuple ignoring NOP's in between. - Returns -1 on error and sets "seq" to NULL. - Returns 0 on success and sets "seq" to NULL if failed to collect requested number of constants. - Returns 0 on success and sets "seq" to resulting tuple if succeeded to collect requested number of constants. + Returns ERROR on error and sets "seq" to NULL. + Returns SUCCESS on success and sets "seq" to NULL if failed to collect requested number of constants. + Returns SUCCESS on success and sets "seq" to resulting tuple if succeeded to collect requested number of constants. */ static int get_constant_sequence(basicblock *bb, int start, int size, @@ -1354,7 +1354,7 @@ get_constant_sequence(basicblock *bb, int start, int size, *seq = NULL; PyObject *res = PyTuple_New((Py_ssize_t)size); if (res == NULL) { - return -1; + return ERROR; } for (; start >= 0 && size > 0; start--) { cfg_instr *instr = &bb->b_instr[start]; @@ -1367,7 +1367,7 @@ get_constant_sequence(basicblock *bb, int start, int size, PyObject *constant = get_const_value(instr->i_opcode, instr->i_oparg, consts); if (constant == NULL) { Py_DECREF(res); - return -1; + return ERROR; } PyTuple_SET_ITEM(res, --size, constant); } @@ -1377,15 +1377,14 @@ get_constant_sequence(basicblock *bb, int start, int size, else { *seq = res; } - return 0; + return SUCCESS; } /* - Walk basic block upwards starting from "start" and change "count" number of - non-NOP instructions to NOP's, returning index of first instruction before - last placed NOP. Returns -1 if last placed NOP was first instruction in the block. + Walk basic block backwards starting from "start" and change "count" number of + non-NOP instructions to NOP's. */ -static int +static void nop_out(basicblock *bb, int start, int count) { assert(start < bb->b_iused); @@ -1398,7 +1397,6 @@ nop_out(basicblock *bb, int start, int count) count--; } assert(start >= -1); - return start; } /* Replace LOAD_CONST c1, LOAD_CONST c2 ... LOAD_CONST cn, BUILD_TUPLE n @@ -1425,7 +1423,7 @@ fold_tuple_of_constants(basicblock *bb, int n, PyObject *consts, PyObject *const int index = add_const(newconst, consts, const_cache); RETURN_IF_ERROR(index); INSTR_SET_OP1(&bb->b_instr[n], LOAD_CONST, index); - (void)nop_out(bb, n-1, seq_size); + nop_out(bb, n-1, seq_size); return SUCCESS; } @@ -1465,16 +1463,13 @@ optimize_if_const_list_or_set(basicblock *bb, int n, PyObject *consts, PyObject RETURN_IF_ERROR(index); INSTR_SET_OP1(&bb->b_instr[n], extend, 1); INSTR_SET_OP1(&bb->b_instr[n-1], LOAD_CONST, index); - int i = nop_out(bb, n-2, seq_size-2); - for (; i > 0 && bb->b_instr[i].i_opcode == NOP; i--); - assert(i >= 0); - assert(loads_const(bb->b_instr[i].i_opcode)); - INSTR_SET_OP1(&bb->b_instr[i], build, 0); + INSTR_SET_OP1(&bb->b_instr[n-2], build, 0); + nop_out(bb, n-3, seq_size-2); return SUCCESS; } /* - Walk basic block upwards starting from "start" to collect instruction pair + Walk basic block backwards starting from "start" to collect instruction pair that loads consts skipping NOP's in between. */ static bool From 96ff172a6221743710ae73590be152ed39b24e36 Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Sat, 8 Feb 2025 08:51:25 +0100 Subject: [PATCH 05/12] address review --- Python/flowgraph.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 49782d8b046f40..3ce5be41ac3ef2 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -264,7 +264,7 @@ basicblock_insert_instruction(basicblock *block, int pos, cfg_instr *instr) { } /* For debugging purposes only */ -#if 0 +#if 1 static void dump_instr(cfg_instr *i) { @@ -1396,7 +1396,6 @@ nop_out(basicblock *bb, int start, int count) INSTR_SET_OP0(&bb->b_instr[start], NOP); count--; } - assert(start >= -1); } /* Replace LOAD_CONST c1, LOAD_CONST c2 ... LOAD_CONST cn, BUILD_TUPLE n @@ -1417,13 +1416,14 @@ fold_tuple_of_constants(basicblock *bb, int n, PyObject *consts, PyObject *const PyObject *newconst; RETURN_IF_ERROR(get_constant_sequence(bb, n-1, seq_size, consts, &newconst)); if (newconst == NULL) { + /* not a const sequence */ return SUCCESS; } - assert(PyTuple_GET_SIZE(newconst) == seq_size); + assert(PyTuple_CheckExact(newconst) && PyTuple_GET_SIZE(newconst) == seq_size); int index = add_const(newconst, consts, const_cache); RETURN_IF_ERROR(index); - INSTR_SET_OP1(&bb->b_instr[n], LOAD_CONST, index); nop_out(bb, n-1, seq_size); + INSTR_SET_OP1(&bb->b_instr[n], LOAD_CONST, index); return SUCCESS; } @@ -1446,9 +1446,10 @@ optimize_if_const_list_or_set(basicblock *bb, int n, PyObject *consts, PyObject PyObject *newconst; RETURN_IF_ERROR(get_constant_sequence(bb, n-1, seq_size, consts, &newconst)); if (newconst == NULL) { + /* not a const sequence */ return SUCCESS; } - assert(PyTuple_GET_SIZE(newconst) == seq_size); + assert(PyTuple_CheckExact(newconst) && PyTuple_GET_SIZE(newconst) == seq_size); int build = instr->i_opcode; int extend = build == BUILD_LIST ? LIST_EXTEND : SET_UPDATE; if (build == BUILD_SET) { @@ -1461,10 +1462,11 @@ optimize_if_const_list_or_set(basicblock *bb, int n, PyObject *consts, PyObject } int index = add_const(newconst, consts, const_cache); RETURN_IF_ERROR(index); - INSTR_SET_OP1(&bb->b_instr[n], extend, 1); - INSTR_SET_OP1(&bb->b_instr[n-1], LOAD_CONST, index); + nop_out(bb, n-1, seq_size); + assert(n >= 2); INSTR_SET_OP1(&bb->b_instr[n-2], build, 0); - nop_out(bb, n-3, seq_size-2); + INSTR_SET_OP1(&bb->b_instr[n-1], LOAD_CONST, index); + INSTR_SET_OP1(&bb->b_instr[n], extend, 1); return SUCCESS; } From ed7431d7816daeef4871583ce7b05748c4910baa Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Sat, 8 Feb 2025 11:38:36 +0100 Subject: [PATCH 06/12] add tests --- Lib/test/test_peepholer.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index a9e55a1af51648..d743ddef9640ab 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -1062,6 +1062,41 @@ def test_conditional_jump_forward_non_const_condition(self): consts=[0, 1, 2, 3, 4], expected_consts=[0, 2, 3]) + def test_multiple_foldings(self): + before = [ + ('LOAD_SMALL_INT', 1, 0), + ('LOAD_SMALL_INT', 2, 0), + ('BUILD_TUPLE', 1, 0), + ('LOAD_SMALL_INT', 0, 0), + ('BINARY_SUBSCR', None, 0), + ('BUILD_TUPLE', 2, 0), + ('RETURN_VALUE', None, 0) + ] + after = [ + ('LOAD_CONST', 1, 0), + ('RETURN_VALUE', None, 0) + ] + self.cfg_optimization_test(before, after, consts=[], expected_consts=[(2,), (1, 2)]) + + def test_fold_tuple_of_constants_nops(self): + before = [ + ('NOP', None, 0), + ('LOAD_SMALL_INT', 1, 0), + ('NOP', None, 0), + ('LOAD_SMALL_INT', 2, 0), + ('NOP', None, 0), + ('NOP', None, 0), + ('LOAD_SMALL_INT', 3, 0), + ('NOP', None, 0), + ('BUILD_TUPLE', 3, 0), + ('RETURN_VALUE', None, 0), + ] + after = [ + ('LOAD_CONST', 0, 0), + ('RETURN_VALUE', None, 0), + ] + self.cfg_optimization_test(before, after, consts=[], expected_consts=[(1, 2, 3)]) + def test_conditional_jump_forward_const_condition(self): # The unreachable branch of the jump is removed, the jump # becomes redundant and is replaced by a NOP (for the lineno) From f539224f9281e40bb9e7b75220c15930c8c8a25d Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Sat, 8 Feb 2025 11:39:55 +0100 Subject: [PATCH 07/12] disable debug code --- Python/flowgraph.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 3ce5be41ac3ef2..308cdac3c44608 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -264,7 +264,7 @@ basicblock_insert_instruction(basicblock *block, int pos, cfg_instr *instr) { } /* For debugging purposes only */ -#if 1 +#if 0 static void dump_instr(cfg_instr *i) { From 5866faa9cee56c3bc3b348ce25872c3781c7138a Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Sat, 8 Feb 2025 12:23:23 +0100 Subject: [PATCH 08/12] update tests --- Lib/test/test_peepholer.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index d743ddef9640ab..423df4fa4680af 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -36,6 +36,13 @@ def count_instr_recursively(f, opname): return count +def get_binop_argval(arg): + for i, nb_op in enumerate(opcode._nb_ops): + if arg == nb_op[0]: + return i + assert False, f"{arg} is not a valid BINARY_OP argument." + + class TestTranforms(BytecodeTestCase): def check_jump_targets(self, code): @@ -518,8 +525,7 @@ def test_folding_subscript(self): ('("a" * 10)[10]', True), ('(1, (1, 2))[2:6][0][2-1]', True), ] - subscr_argval = 26 - assert opcode._nb_ops[subscr_argval][0] == 'NB_SUBSCR' + subscr_argval = get_binop_argval('NB_SUBSCR') for expr, has_error in tests: with self.subTest(expr=expr, has_error=has_error): code = compile(expr, '', 'single') @@ -1068,7 +1074,7 @@ def test_multiple_foldings(self): ('LOAD_SMALL_INT', 2, 0), ('BUILD_TUPLE', 1, 0), ('LOAD_SMALL_INT', 0, 0), - ('BINARY_SUBSCR', None, 0), + ('BINARY_OP', get_binop_argval('NB_SUBSCR'), 0), ('BUILD_TUPLE', 2, 0), ('RETURN_VALUE', None, 0) ] From 578d66b4d1bb233391c133c97182e435ad511443 Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Sat, 8 Feb 2025 14:54:07 +0100 Subject: [PATCH 09/12] update tests --- Lib/test/test_peepholer.py | 133 ++++++++++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index 423df4fa4680af..f7b0a3007b1f3a 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -1084,7 +1084,18 @@ def test_multiple_foldings(self): ] self.cfg_optimization_test(before, after, consts=[], expected_consts=[(2,), (1, 2)]) - def test_fold_tuple_of_constants_nops(self): + def test_build_empty_tuple(self): + before = [ + ('BUILD_TUPLE', 0, 0), + ('RETURN_VALUE', None, 0), + ] + after = [ + ('LOAD_CONST', 0, 0), + ('RETURN_VALUE', None, 0), + ] + self.cfg_optimization_test(before, after, consts=[], expected_consts=[()]) + + def test_fold_tuple_of_constants(self): before = [ ('NOP', None, 0), ('LOAD_SMALL_INT', 1, 0), @@ -1103,6 +1114,126 @@ def test_fold_tuple_of_constants_nops(self): ] self.cfg_optimization_test(before, after, consts=[], expected_consts=[(1, 2, 3)]) + # not enough consts + same = [ + ('LOAD_SMALL_INT', 1, 0), + ('LOAD_SMALL_INT', 2, 0), + ('BUILD_TUPLE', 3, 0), + ('RETURN_VALUE', None, 0) + ] + self.cfg_optimization_test(same, same, consts=[]) + + # not all consts + same = [ + ('LOAD_SMALL_INT', 1, 0), + ('LOAD_NAME', 0, 0), + ('LOAD_SMALL_INT', 2, 0), + ('BUILD_TUPLE', 3, 0), + ('RETURN_VALUE', None, 0) + ] + self.cfg_optimization_test(same, same, consts=[]) + + def test_optimize_if_const_list(self): + before = [ + ('NOP', None, 0), + ('LOAD_SMALL_INT', 1, 0), + ('NOP', None, 0), + ('LOAD_SMALL_INT', 2, 0), + ('NOP', None, 0), + ('NOP', None, 0), + ('LOAD_SMALL_INT', 3, 0), + ('NOP', None, 0), + ('BUILD_LIST', 3, 0), + ('RETURN_VALUE', None, 0), + ] + after = [ + ('BUILD_LIST', 0, 0), + ('LOAD_CONST', 0, 0), + ('LIST_EXTEND', 1, 0), + ('RETURN_VALUE', None, 0), + ] + self.cfg_optimization_test(before, after, consts=[], expected_consts=[(1, 2, 3)]) + + # need minimum 3 consts to optimize + same = [ + ('LOAD_SMALL_INT', 1, 0), + ('LOAD_SMALL_INT', 2, 0), + ('BUILD_LIST', 2, 0), + ('RETURN_VALUE', None, 0), + ] + self.cfg_optimization_test(same, same, consts=[]) + + # not enough consts + same = [ + ('LOAD_SMALL_INT', 1, 0), + ('LOAD_SMALL_INT', 2, 0), + ('LOAD_SMALL_INT', 3, 0), + ('BUILD_LIST', 4, 0), + ('RETURN_VALUE', None, 0), + ] + self.cfg_optimization_test(same, same, consts=[]) + + # not all consts + same = [ + ('LOAD_SMALL_INT', 1, 0), + ('LOAD_NAME', 0, 0), + ('LOAD_SMALL_INT', 3, 0), + ('BUILD_LIST', 3, 0), + ('RETURN_VALUE', None, 0), + ] + self.cfg_optimization_test(same, same, consts=[]) + + def test_optimize_if_const_set(self): + before = [ + ('NOP', None, 0), + ('LOAD_SMALL_INT', 1, 0), + ('NOP', None, 0), + ('LOAD_SMALL_INT', 2, 0), + ('NOP', None, 0), + ('NOP', None, 0), + ('LOAD_SMALL_INT', 3, 0), + ('NOP', None, 0), + ('BUILD_SET', 3, 0), + ('RETURN_VALUE', None, 0), + ] + after = [ + ('BUILD_SET', 0, 0), + ('LOAD_CONST', 0, 0), + ('SET_UPDATE', 1, 0), + ('RETURN_VALUE', None, 0), + ] + self.cfg_optimization_test(before, after, consts=[], expected_consts=[frozenset({1, 2, 3})]) + + # need minimum 3 consts to optimize + same = [ + ('LOAD_SMALL_INT', 1, 0), + ('LOAD_SMALL_INT', 2, 0), + ('BUILD_SET', 2, 0), + ('RETURN_VALUE', None, 0), + ] + self.cfg_optimization_test(same, same, consts=[]) + + # not enough consts + same = [ + ('LOAD_SMALL_INT', 1, 0), + ('LOAD_SMALL_INT', 2, 0), + ('LOAD_SMALL_INT', 3, 0), + ('BUILD_SET', 4, 0), + ('RETURN_VALUE', None, 0), + ] + self.cfg_optimization_test(same, same, consts=[]) + + # not all consts + same = [ + ('LOAD_SMALL_INT', 1, 0), + ('LOAD_NAME', 0, 0), + ('LOAD_SMALL_INT', 3, 0), + ('BUILD_SET', 3, 0), + ('RETURN_VALUE', None, 0), + ] + self.cfg_optimization_test(same, same, consts=[]) + + def test_conditional_jump_forward_const_condition(self): # The unreachable branch of the jump is removed, the jump # becomes redundant and is replaced by a NOP (for the lineno) From cd92614a0391b57279a28c0d8ca583924b6ab252 Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Sun, 9 Feb 2025 18:16:24 +0100 Subject: [PATCH 10/12] update tests --- Lib/test/test_peepholer.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index f7b0a3007b1f3a..c89ae5712765a3 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -1068,6 +1068,34 @@ def test_conditional_jump_forward_non_const_condition(self): consts=[0, 1, 2, 3, 4], expected_consts=[0, 2, 3]) + def test_build_list_stack_use_guideline(self): + def f(): + return [ + 0, 1, 2, 3, 4, + 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, + 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, + 25, 26, 27, 28, 29, + 30, 31, 32, 33, 34, + 35, 36, 37, 38, 39 + ] + self.assertEqual(f(), list(range(40))) + + def test_build_set_stack_use_guideline(self): + def f(): + return { + 0, 1, 2, 3, 4, + 5, 6, 7, 8, 9, + 10, 11, 12, 13, 14, + 15, 16, 17, 18, 19, + 20, 21, 22, 23, 24, + 25, 26, 27, 28, 29, + 30, 31, 32, 33, 34, + 35, 36, 37, 38, 39 + } + self.assertEqual(f(), frozenset(range(40))) + def test_multiple_foldings(self): before = [ ('LOAD_SMALL_INT', 1, 0), From 1574122176472fb12c5d9e4d54b7ab0848216aaa Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Sun, 9 Feb 2025 17:27:04 +0000 Subject: [PATCH 11/12] Update Lib/test/test_peepholer.py --- Lib/test/test_peepholer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index c89ae5712765a3..06f33d92920293 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -1068,7 +1068,7 @@ def test_conditional_jump_forward_non_const_condition(self): consts=[0, 1, 2, 3, 4], expected_consts=[0, 2, 3]) - def test_build_list_stack_use_guideline(self): + def test_list_exceeding_stack_use_guideline(self): def f(): return [ 0, 1, 2, 3, 4, From 16c66f9adbfc411bfbf82d8f921dbac27a3d9da8 Mon Sep 17 00:00:00 2001 From: Yan Yanchii Date: Sun, 9 Feb 2025 18:32:03 +0100 Subject: [PATCH 12/12] Update Lib/test/test_peepholer.py Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> --- Lib/test/test_peepholer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index 06f33d92920293..ed92e5257df39e 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -1082,7 +1082,7 @@ def f(): ] self.assertEqual(f(), list(range(40))) - def test_build_set_stack_use_guideline(self): + def test_set_exceeding_stack_use_guideline(self): def f(): return { 0, 1, 2, 3, 4,