Skip to content

Commit 8c655c7

Browse files
Eclips4iritkatrielWolframAlph
authored andcommitted
pythongh-126835: Move optimization of constant sequence creation from codegen to CFG (python#129426)
Codegen phase has an optimization that transforms ``` LOAD_CONST x LOAD_CONST y LOAD_CONXT z BUILD_LIST/BUILD_SET (3) ``` -> ``` BUILD_LIST/BUILD_SET (0) LOAD_CONST (x, y, z) LIST_EXTEND/SET_UPDATE 1 ``` This optimization has now been moved to CFG phase to make python#128802 work. Co-authored-by: Irit Katriel <[email protected]> Co-authored-by: Yan Yanchii <[email protected]>
1 parent 1f1424b commit 8c655c7

File tree

3 files changed

+73
-49
lines changed

3 files changed

+73
-49
lines changed

Lib/test/test_dis.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,7 @@ def loop_test():
892892
%3d RESUME_CHECK 0
893893
894894
%3d BUILD_LIST 0
895-
LOAD_CONST_MORTAL 0 ((1, 2, 3))
895+
LOAD_CONST_MORTAL 1 ((1, 2, 3))
896896
LIST_EXTEND 1
897897
LOAD_SMALL_INT 3
898898
BINARY_OP 5 (*)
@@ -908,7 +908,7 @@ def loop_test():
908908
909909
%3d L2: END_FOR
910910
POP_ITER
911-
LOAD_CONST_IMMORTAL 1 (None)
911+
LOAD_CONST_IMMORTAL 0 (None)
912912
RETURN_VALUE
913913
""" % (loop_test.__code__.co_firstlineno,
914914
loop_test.__code__.co_firstlineno + 1,

Python/codegen.c

-43
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,6 @@ static int codegen_subscript(compiler *, expr_ty);
201201
static int codegen_slice_two_parts(compiler *, expr_ty);
202202
static int codegen_slice(compiler *, expr_ty);
203203

204-
static bool are_all_items_const(asdl_expr_seq *, Py_ssize_t, Py_ssize_t);
205-
206-
207204
static int codegen_with(compiler *, stmt_ty, int);
208205
static int codegen_async_with(compiler *, stmt_ty, int);
209206
static int codegen_async_for(compiler *, stmt_ty);
@@ -3210,34 +3207,6 @@ starunpack_helper_impl(compiler *c, location loc,
32103207
int build, int add, int extend, int tuple)
32113208
{
32123209
Py_ssize_t n = asdl_seq_LEN(elts);
3213-
if (!injected_arg && n > 2 && are_all_items_const(elts, 0, n)) {
3214-
PyObject *folded = PyTuple_New(n);
3215-
if (folded == NULL) {
3216-
return ERROR;
3217-
}
3218-
for (Py_ssize_t i = 0; i < n; i++) {
3219-
PyObject *val = ((expr_ty)asdl_seq_GET(elts, i))->v.Constant.value;
3220-
PyTuple_SET_ITEM(folded, i, Py_NewRef(val));
3221-
}
3222-
if (tuple && !pushed) {
3223-
ADDOP_LOAD_CONST_NEW(c, loc, folded);
3224-
} else {
3225-
if (add == SET_ADD) {
3226-
Py_SETREF(folded, PyFrozenSet_New(folded));
3227-
if (folded == NULL) {
3228-
return ERROR;
3229-
}
3230-
}
3231-
ADDOP_I(c, loc, build, pushed);
3232-
ADDOP_LOAD_CONST_NEW(c, loc, folded);
3233-
ADDOP_I(c, loc, extend, 1);
3234-
if (tuple) {
3235-
ADDOP_I(c, loc, CALL_INTRINSIC_1, INTRINSIC_LIST_TO_TUPLE);
3236-
}
3237-
}
3238-
return SUCCESS;
3239-
}
3240-
32413210
int big = n + pushed + (injected_arg ? 1 : 0) > STACK_USE_GUIDELINE;
32423211
int seen_star = 0;
32433212
for (Py_ssize_t i = 0; i < n; i++) {
@@ -3389,18 +3358,6 @@ codegen_set(compiler *c, expr_ty e)
33893358
BUILD_SET, SET_ADD, SET_UPDATE, 0);
33903359
}
33913360

3392-
static bool
3393-
are_all_items_const(asdl_expr_seq *seq, Py_ssize_t begin, Py_ssize_t end)
3394-
{
3395-
for (Py_ssize_t i = begin; i < end; i++) {
3396-
expr_ty key = (expr_ty)asdl_seq_GET(seq, i);
3397-
if (key == NULL || key->kind != Constant_kind) {
3398-
return false;
3399-
}
3400-
}
3401-
return true;
3402-
}
3403-
34043361
static int
34053362
codegen_subdict(compiler *c, expr_ty e, Py_ssize_t begin, Py_ssize_t end)
34063363
{

Python/flowgraph.c

+71-4
Original file line numberDiff line numberDiff line change
@@ -1336,6 +1336,17 @@ add_const(PyObject *newconst, PyObject *consts, PyObject *const_cache)
13361336
return (int)index;
13371337
}
13381338

1339+
static bool
1340+
is_constant_sequence(cfg_instr *inst, int n)
1341+
{
1342+
for (int i = 0; i < n; i++) {
1343+
if(!loads_const(inst[i].i_opcode)) {
1344+
return false;
1345+
}
1346+
}
1347+
return true;
1348+
}
1349+
13391350
/* Replace LOAD_CONST c1, LOAD_CONST c2 ... LOAD_CONST cn, BUILD_TUPLE n
13401351
with LOAD_CONST (c1, c2, ... cn).
13411352
The consts table must still be in list form so that the
@@ -1353,10 +1364,8 @@ fold_tuple_on_constants(PyObject *const_cache,
13531364
assert(inst[n].i_opcode == BUILD_TUPLE);
13541365
assert(inst[n].i_oparg == n);
13551366

1356-
for (int i = 0; i < n; i++) {
1357-
if (!loads_const(inst[i].i_opcode)) {
1358-
return SUCCESS;
1359-
}
1367+
if (!is_constant_sequence(inst, n)) {
1368+
return SUCCESS;
13601369
}
13611370

13621371
/* Buildup new tuple of constants */
@@ -1384,6 +1393,56 @@ fold_tuple_on_constants(PyObject *const_cache,
13841393
return SUCCESS;
13851394
}
13861395

1396+
#define MIN_CONST_SEQUENCE_SIZE 3
1397+
/* Replace LOAD_CONST c1, LOAD_CONST c2 ... LOAD_CONST cN, BUILD_LIST N
1398+
with BUILD_LIST 0, LOAD_CONST (c1, c2, ... cN), LIST_EXTEND 1,
1399+
or BUILD_SET & SET_UPDATE respectively.
1400+
*/
1401+
static int
1402+
optimize_if_const_list_or_set(PyObject *const_cache, cfg_instr* inst, int n, PyObject *consts)
1403+
{
1404+
assert(PyDict_CheckExact(const_cache));
1405+
assert(PyList_CheckExact(consts));
1406+
assert(inst[n].i_oparg == n);
1407+
1408+
int build = inst[n].i_opcode;
1409+
assert(build == BUILD_LIST || build == BUILD_SET);
1410+
int extend = build == BUILD_LIST ? LIST_EXTEND : SET_UPDATE;
1411+
1412+
if (n < MIN_CONST_SEQUENCE_SIZE || !is_constant_sequence(inst, n)) {
1413+
return SUCCESS;
1414+
}
1415+
PyObject *newconst = PyTuple_New(n);
1416+
if (newconst == NULL) {
1417+
return ERROR;
1418+
}
1419+
for (int i = 0; i < n; i++) {
1420+
int op = inst[i].i_opcode;
1421+
int arg = inst[i].i_oparg;
1422+
PyObject *constant = get_const_value(op, arg, consts);
1423+
if (constant == NULL) {
1424+
return ERROR;
1425+
}
1426+
PyTuple_SET_ITEM(newconst, i, constant);
1427+
}
1428+
if (build == BUILD_SET) {
1429+
PyObject *frozenset = PyFrozenSet_New(newconst);
1430+
if (frozenset == NULL) {
1431+
return ERROR;
1432+
}
1433+
Py_SETREF(newconst, frozenset);
1434+
}
1435+
int index = add_const(newconst, consts, const_cache);
1436+
RETURN_IF_ERROR(index);
1437+
INSTR_SET_OP1(&inst[0], build, 0);
1438+
for (int i = 1; i < n - 1; i++) {
1439+
INSTR_SET_OP0(&inst[i], NOP);
1440+
}
1441+
INSTR_SET_OP1(&inst[n-1], LOAD_CONST, index);
1442+
INSTR_SET_OP1(&inst[n], extend, 1);
1443+
return SUCCESS;
1444+
}
1445+
13871446
#define VISITED (-1)
13881447

13891448
// Replace an arbitrary run of SWAPs and NOPs with an optimal one that has the
@@ -1751,6 +1810,14 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
17511810
}
17521811
}
17531812
break;
1813+
case BUILD_LIST:
1814+
case BUILD_SET:
1815+
if (i >= oparg) {
1816+
if (optimize_if_const_list_or_set(const_cache, inst-oparg, oparg, consts) < 0) {
1817+
goto error;
1818+
}
1819+
}
1820+
break;
17541821
case POP_JUMP_IF_NOT_NONE:
17551822
case POP_JUMP_IF_NONE:
17561823
switch (target->i_opcode) {

0 commit comments

Comments
 (0)