Skip to content

Commit 212dacc

Browse files
committed
address review
1 parent 64eb38d commit 212dacc

File tree

3 files changed

+65
-63
lines changed

3 files changed

+65
-63
lines changed

Include/internal/pycore_compile.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ extern "C" {
2121
* Code that could use more stack than this is
2222
* rare, so the exact value is unimportant.
2323
*/
24-
#define STACK_USE_GUIDELINE 30
24+
#define _PY_STACK_USE_GUIDELINE 30
2525

2626
struct _arena; // Type defined in pycore_pyarena.h
2727
struct _mod; // Type defined in pycore_ast.h

Python/codegen.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3200,7 +3200,7 @@ starunpack_helper_impl(compiler *c, location loc,
32003200
int build, int add, int extend, int tuple)
32013201
{
32023202
Py_ssize_t n = asdl_seq_LEN(elts);
3203-
int big = n + pushed + (injected_arg ? 1 : 0) > STACK_USE_GUIDELINE;
3203+
int big = n + pushed + (injected_arg ? 1 : 0) > _PY_STACK_USE_GUIDELINE;
32043204
int seen_star = 0;
32053205
for (Py_ssize_t i = 0; i < n; i++) {
32063206
expr_ty elt = asdl_seq_GET(elts, i);
@@ -3355,7 +3355,7 @@ static int
33553355
codegen_subdict(compiler *c, expr_ty e, Py_ssize_t begin, Py_ssize_t end)
33563356
{
33573357
Py_ssize_t i, n = end - begin;
3358-
int big = n*2 > STACK_USE_GUIDELINE;
3358+
int big = n*2 > _PY_STACK_USE_GUIDELINE;
33593359
location loc = LOC(e);
33603360
if (big) {
33613361
ADDOP_I(c, loc, BUILD_MAP, 0);
@@ -3402,7 +3402,7 @@ codegen_dict(compiler *c, expr_ty e)
34023402
ADDOP_I(c, loc, DICT_UPDATE, 1);
34033403
}
34043404
else {
3405-
if (elements*2 > STACK_USE_GUIDELINE) {
3405+
if (elements*2 > _PY_STACK_USE_GUIDELINE) {
34063406
RETURN_IF_ERROR(codegen_subdict(c, e, i - elements, i + 1));
34073407
if (have_dict) {
34083408
ADDOP_I(c, loc, DICT_UPDATE, 1);
@@ -3743,7 +3743,7 @@ maybe_optimize_method_call(compiler *c, expr_ty e)
37433743
/* Check that there aren't too many arguments */
37443744
argsl = asdl_seq_LEN(args);
37453745
kwdsl = asdl_seq_LEN(kwds);
3746-
if (argsl + kwdsl + (kwdsl != 0) >= STACK_USE_GUIDELINE) {
3746+
if (argsl + kwdsl + (kwdsl != 0) >= _PY_STACK_USE_GUIDELINE) {
37473747
return 0;
37483748
}
37493749
/* Check that there are no *varargs types of arguments. */
@@ -3840,7 +3840,7 @@ codegen_joined_str(compiler *c, expr_ty e)
38403840
{
38413841
location loc = LOC(e);
38423842
Py_ssize_t value_count = asdl_seq_LEN(e->v.JoinedStr.values);
3843-
if (value_count > STACK_USE_GUIDELINE) {
3843+
if (value_count > _PY_STACK_USE_GUIDELINE) {
38443844
_Py_DECLARE_STR(empty, "");
38453845
ADDOP_LOAD_CONST_NEW(c, loc, Py_NewRef(&_Py_STR(empty)));
38463846
ADDOP_NAME(c, loc, LOAD_METHOD, &_Py_ID(join), names);
@@ -3919,7 +3919,7 @@ codegen_subkwargs(compiler *c, location loc,
39193919
Py_ssize_t i, n = end - begin;
39203920
keyword_ty kw;
39213921
assert(n > 0);
3922-
int big = n*2 > STACK_USE_GUIDELINE;
3922+
int big = n*2 > _PY_STACK_USE_GUIDELINE;
39233923
if (big) {
39243924
ADDOP_I(c, NO_LOCATION, BUILD_MAP, 0);
39253925
}
@@ -3972,7 +3972,7 @@ codegen_call_helper_impl(compiler *c, location loc,
39723972
nelts = asdl_seq_LEN(args);
39733973
nkwelts = asdl_seq_LEN(keywords);
39743974

3975-
if (nelts + nkwelts*2 > STACK_USE_GUIDELINE) {
3975+
if (nelts + nkwelts*2 > _PY_STACK_USE_GUIDELINE) {
39763976
goto ex_call;
39773977
}
39783978
for (i = 0; i < nelts; i++) {

Python/flowgraph.c

Lines changed: 57 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,17 +1345,17 @@ add_const(PyObject *newconst, PyObject *consts, PyObject *const_cache)
13451345

13461346
/*
13471347
Walk basic block backwards starting from "start" trying to collect "size" number of
1348-
subsequent instructions that load constants into instruciton array "seq" ignoring NOP's in between.
1349-
Caller must make sure that length of "seq" is sufficient to fit in at least "size" instructions.
1348+
subsequent instructions that load constants into instruciton array "instrs" ignoring NOP's in between.
1349+
Caller must make sure that length of "instrs" is sufficient to fit in at least "size" instructions.
13501350
13511351
Returns boolean indicating whether succeeded to collect requested number of instructions.
13521352
*/
13531353
static bool
1354-
get_const_instr_sequence(basicblock *bb, int start, cfg_instr **seq, int size)
1354+
get_subsequent_const_instrs(basicblock *bb, int start, cfg_instr **instrs, int size)
13551355
{
13561356
assert(start < bb->b_iused);
13571357
assert(size >= 0);
1358-
assert(size <= STACK_USE_GUIDELINE);
1358+
assert(size <= _PY_STACK_USE_GUIDELINE);
13591359

13601360
for (; start >= 0 && size > 0; start--) {
13611361
cfg_instr *instr = &bb->b_instr[start];
@@ -1365,17 +1365,19 @@ get_const_instr_sequence(basicblock *bb, int start, cfg_instr **seq, int size)
13651365
if (!loads_const(instr->i_opcode)) {
13661366
return false;
13671367
}
1368-
seq[--size] = instr;
1368+
instrs[--size] = instr;
13691369
}
13701370

13711371
return size == 0;
13721372
}
13731373

13741374
/*
1375-
Change every instruction in "instrs" NOP and set it's location to NO_LOCATION.
1375+
Change every instruction in "instrs" NOP and set its location to NO_LOCATION.
13761376
Caller must make sure "instrs" has at least "size" elements.
13771377
*/
1378-
static void nop_out(basicblock *bb, cfg_instr **instrs, int size) {
1378+
static void
1379+
nop_out(basicblock *bb, cfg_instr **instrs, int size)
1380+
{
13791381
for (int i = 0; i < size; i++) {
13801382
cfg_instr *instr = instrs[i];
13811383
assert(instr->i_opcode != NOP);
@@ -1421,34 +1423,34 @@ fold_tuple_of_constants(basicblock *bb, int i, PyObject *consts, PyObject *const
14211423
assert(instr->i_opcode == BUILD_TUPLE);
14221424

14231425
int seq_size = instr->i_oparg;
1424-
if (seq_size > STACK_USE_GUIDELINE) {
1426+
if (seq_size > _PY_STACK_USE_GUIDELINE) {
14251427
return SUCCESS;
14261428
}
14271429

1428-
cfg_instr *seq[STACK_USE_GUIDELINE];
1429-
if (!get_const_instr_sequence(bb, i-1, seq, seq_size)) {
1430+
cfg_instr *const_instrs[_PY_STACK_USE_GUIDELINE];
1431+
if (!get_subsequent_const_instrs(bb, i-1, const_instrs, seq_size)) {
14301432
/* not a const sequence */
14311433
return SUCCESS;
14321434
}
14331435

1434-
PyObject *newconst = PyTuple_New((Py_ssize_t)seq_size);
1435-
if (newconst == NULL) {
1436+
PyObject *const_tuple = PyTuple_New((Py_ssize_t)seq_size);
1437+
if (const_tuple == NULL) {
14361438
return ERROR;
14371439
}
14381440

14391441
for (int i = 0; i < seq_size; i++) {
1440-
cfg_instr *inst = seq[i];
1442+
cfg_instr *inst = const_instrs[i];
14411443
assert(loads_const(inst->i_opcode));
1442-
PyObject *constant = get_const_value(inst->i_opcode, inst->i_oparg, consts);
1443-
if (constant == NULL) {
1444-
Py_DECREF(newconst);
1444+
PyObject *element = get_const_value(inst->i_opcode, inst->i_oparg, consts);
1445+
if (element == NULL) {
1446+
Py_DECREF(const_tuple);
14451447
return ERROR;
14461448
}
1447-
PyTuple_SET_ITEM(newconst, i, constant);
1449+
PyTuple_SET_ITEM(const_tuple, i, element);
14481450
}
14491451

1450-
nop_out(bb, seq, seq_size);
1451-
return instr_make_load_const(instr, newconst, consts, const_cache);
1452+
nop_out(bb, const_instrs, seq_size);
1453+
return instr_make_load_const(instr, const_tuple, consts, const_cache);
14521454
}
14531455

14541456
#define MIN_CONST_SEQUENCE_SIZE 3
@@ -1475,49 +1477,49 @@ optimize_lists_and_sets(basicblock *bb, int i, int nextop,
14751477

14761478
bool contains_or_iter = nextop == GET_ITER || nextop == CONTAINS_OP;
14771479
int seq_size = instr->i_oparg;
1478-
if (seq_size > STACK_USE_GUIDELINE ||
1480+
if (seq_size > _PY_STACK_USE_GUIDELINE ||
14791481
(seq_size < MIN_CONST_SEQUENCE_SIZE && !contains_or_iter))
14801482
{
14811483
return SUCCESS;
14821484
}
14831485

1484-
cfg_instr *seq[STACK_USE_GUIDELINE];
1485-
if (!get_const_instr_sequence(bb, i-1, seq, seq_size)) { /* not a const sequence */
1486+
cfg_instr *const_instrs[_PY_STACK_USE_GUIDELINE];
1487+
if (!get_subsequent_const_instrs(bb, i-1, const_instrs, seq_size)) { /* not a const sequence */
14861488
if (contains_or_iter && instr->i_opcode == BUILD_LIST) {
14871489
/* iterate over a tuple instead of list */
14881490
INSTR_SET_OP1(instr, BUILD_TUPLE, instr->i_oparg);
14891491
}
14901492
return SUCCESS;
14911493
}
14921494

1493-
PyObject *newconst = PyTuple_New((Py_ssize_t)seq_size);
1494-
if (newconst == NULL) {
1495+
PyObject *const_result = PyTuple_New((Py_ssize_t)seq_size);
1496+
if (const_result == NULL) {
14951497
return ERROR;
14961498
}
14971499

14981500
for (int i = 0; i < seq_size; i++) {
1499-
cfg_instr *inst = seq[i];
1501+
cfg_instr *inst = const_instrs[i];
15001502
assert(loads_const(inst->i_opcode));
1501-
PyObject *constant = get_const_value(inst->i_opcode, inst->i_oparg, consts);
1502-
if (constant == NULL) {
1503-
Py_DECREF(newconst);
1503+
PyObject *element = get_const_value(inst->i_opcode, inst->i_oparg, consts);
1504+
if (element == NULL) {
1505+
Py_DECREF(const_result);
15041506
return ERROR;
15051507
}
1506-
PyTuple_SET_ITEM(newconst, i, constant);
1508+
PyTuple_SET_ITEM(const_result, i, element);
15071509
}
15081510

15091511
if (instr->i_opcode == BUILD_SET) {
1510-
PyObject *frozenset = PyFrozenSet_New(newconst);
1512+
PyObject *frozenset = PyFrozenSet_New(const_result);
15111513
if (frozenset == NULL) {
1512-
Py_DECREF(newconst);
1514+
Py_DECREF(const_result);
15131515
return ERROR;
15141516
}
1515-
Py_SETREF(newconst, frozenset);
1517+
Py_SETREF(const_result, frozenset);
15161518
}
15171519

1518-
int index = add_const(newconst, consts, const_cache);
1520+
int index = add_const(const_result, consts, const_cache);
15191521
RETURN_IF_ERROR(index);
1520-
nop_out(bb, seq, seq_size);
1522+
nop_out(bb, const_instrs, seq_size);
15211523

15221524
if (contains_or_iter) {
15231525
INSTR_SET_OP1(instr, LOAD_CONST, index);
@@ -1721,30 +1723,30 @@ fold_const_binop(basicblock *bb, int i, PyObject *consts, PyObject *const_cache)
17211723
cfg_instr *binop = &bb->b_instr[i];
17221724
assert(binop->i_opcode == BINARY_OP);
17231725

1724-
cfg_instr *seq[BINOP_OPERAND_COUNT];
1725-
if (!get_const_instr_sequence(bb, i-1, seq, BINOP_OPERAND_COUNT)) {
1726+
cfg_instr *operands_instrs[BINOP_OPERAND_COUNT];
1727+
if (!get_subsequent_const_instrs(bb, i-1, operands_instrs, BINOP_OPERAND_COUNT)) {
17261728
/* not a const sequence */
17271729
return SUCCESS;
17281730
}
17291731

1730-
cfg_instr *first = seq[0];
1731-
assert(loads_const(first->i_opcode));
1732-
PyObject *left = get_const_value(first->i_opcode, first->i_oparg, consts);
1733-
if (left == NULL) {
1732+
cfg_instr *lhs_instr = operands_instrs[0];
1733+
assert(loads_const(lhs_instr->i_opcode));
1734+
PyObject *lhs = get_const_value(lhs_instr->i_opcode, lhs_instr->i_oparg, consts);
1735+
if (lhs == NULL) {
17341736
return ERROR;
17351737
}
17361738

1737-
cfg_instr *second = seq[1];
1738-
assert(loads_const(second->i_opcode));
1739-
PyObject *right = get_const_value(second->i_opcode, second->i_oparg, consts);
1740-
if (right == NULL) {
1741-
Py_DECREF(left);
1739+
cfg_instr *rhs_instr = operands_instrs[1];
1740+
assert(loads_const(rhs_instr->i_opcode));
1741+
PyObject *rhs = get_const_value(rhs_instr->i_opcode, rhs_instr->i_oparg, consts);
1742+
if (rhs == NULL) {
1743+
Py_DECREF(lhs);
17421744
return ERROR;
17431745
}
17441746

1745-
PyObject *newconst = eval_const_binop(left, binop->i_oparg, right);
1746-
Py_DECREF(left);
1747-
Py_DECREF(right);
1747+
PyObject *newconst = eval_const_binop(lhs, binop->i_oparg, rhs);
1748+
Py_DECREF(lhs);
1749+
Py_DECREF(rhs);
17481750
if (newconst == NULL) {
17491751
if (PyErr_ExceptionMatches(PyExc_KeyboardInterrupt)) {
17501752
return ERROR;
@@ -1753,7 +1755,7 @@ fold_const_binop(basicblock *bb, int i, PyObject *consts, PyObject *const_cache)
17531755
return SUCCESS;
17541756
}
17551757

1756-
nop_out(bb, seq, BINOP_OPERAND_COUNT);
1758+
nop_out(bb, operands_instrs, BINOP_OPERAND_COUNT);
17571759
return instr_make_load_const(binop, newconst, consts, const_cache);
17581760
}
17591761

@@ -1803,16 +1805,16 @@ fold_const_unaryop(basicblock *bb, int i, PyObject *consts, PyObject *const_cach
18031805
assert(PyList_CheckExact(consts));
18041806
cfg_instr *unaryop = &bb->b_instr[i];
18051807

1806-
cfg_instr *instr;
1807-
if (!get_const_instr_sequence(bb, i - 1, &instr, UNARYOP_OPERAND_COUNT)) {
1808+
cfg_instr *operand_instr;
1809+
if (!get_subsequent_const_instrs(bb, i-1, &operand_instr, UNARYOP_OPERAND_COUNT)) {
18081810
/* not a const */
18091811
return SUCCESS;
18101812
}
18111813

1812-
assert(loads_const(instr->i_opcode));
1814+
assert(loads_const(operand_instr->i_opcode));
18131815
PyObject *operand = get_const_value(
1814-
instr->i_opcode,
1815-
instr->i_oparg,
1816+
operand_instr->i_opcode,
1817+
operand_instr->i_oparg,
18161818
consts
18171819
);
18181820
if (operand == NULL) {
@@ -1832,7 +1834,7 @@ fold_const_unaryop(basicblock *bb, int i, PyObject *consts, PyObject *const_cach
18321834
if (unaryop->i_opcode == UNARY_NOT) {
18331835
assert(PyBool_Check(newconst));
18341836
}
1835-
nop_out(bb, &instr, UNARYOP_OPERAND_COUNT);
1837+
nop_out(bb, &operand_instr, UNARYOP_OPERAND_COUNT);
18361838
return instr_make_load_const(unaryop, newconst, consts, const_cache);
18371839
}
18381840

0 commit comments

Comments
 (0)