Skip to content

Commit 6d7a0e0

Browse files
authored
gh-87092: reduce redundancy and repetition in compiler's optimization stage (GH-96713)
1 parent 49cceeb commit 6d7a0e0

File tree

3 files changed

+83
-99
lines changed

3 files changed

+83
-99
lines changed

Lib/test/test_dis.py

+20-7
Original file line numberDiff line numberDiff line change
@@ -360,13 +360,13 @@ def bug42562():
360360
--> BINARY_OP 11 (/)
361361
POP_TOP
362362
363-
%3d >> LOAD_FAST_CHECK 1 (tb)
363+
%3d LOAD_FAST_CHECK 1 (tb)
364364
RETURN_VALUE
365365
>> PUSH_EXC_INFO
366366
367367
%3d LOAD_GLOBAL 0 (Exception)
368368
CHECK_EXC_MATCH
369-
POP_JUMP_IF_FALSE 22 (to 80)
369+
POP_JUMP_IF_FALSE 23 (to 82)
370370
STORE_FAST 0 (e)
371371
372372
%3d LOAD_FAST 0 (e)
@@ -376,7 +376,9 @@ def bug42562():
376376
LOAD_CONST 0 (None)
377377
STORE_FAST 0 (e)
378378
DELETE_FAST 0 (e)
379-
JUMP_BACKWARD 29 (to 14)
379+
380+
%3d LOAD_FAST 1 (tb)
381+
RETURN_VALUE
380382
>> LOAD_CONST 0 (None)
381383
STORE_FAST 0 (e)
382384
DELETE_FAST 0 (e)
@@ -394,6 +396,7 @@ def bug42562():
394396
TRACEBACK_CODE.co_firstlineno + 5,
395397
TRACEBACK_CODE.co_firstlineno + 3,
396398
TRACEBACK_CODE.co_firstlineno + 4,
399+
TRACEBACK_CODE.co_firstlineno + 5,
397400
TRACEBACK_CODE.co_firstlineno + 3)
398401

399402
def _fstring(a, b, c, d):
@@ -440,7 +443,7 @@ def _with(c):
440443
CALL 2
441444
POP_TOP
442445
443-
%3d >> LOAD_CONST 2 (2)
446+
%3d LOAD_CONST 2 (2)
444447
STORE_FAST 2 (y)
445448
LOAD_CONST 0 (None)
446449
RETURN_VALUE
@@ -453,7 +456,11 @@ def _with(c):
453456
POP_EXCEPT
454457
POP_TOP
455458
POP_TOP
456-
JUMP_BACKWARD 13 (to 30)
459+
460+
%3d LOAD_CONST 2 (2)
461+
STORE_FAST 2 (y)
462+
LOAD_CONST 0 (None)
463+
RETURN_VALUE
457464
>> COPY 3
458465
POP_EXCEPT
459466
RERAISE 1
@@ -465,6 +472,7 @@ def _with(c):
465472
_with.__code__.co_firstlineno + 1,
466473
_with.__code__.co_firstlineno + 3,
467474
_with.__code__.co_firstlineno + 1,
475+
_with.__code__.co_firstlineno + 3,
468476
)
469477

470478
async def _asyncwith(c):
@@ -502,7 +510,7 @@ async def _asyncwith(c):
502510
JUMP_BACKWARD_NO_INTERRUPT 4 (to 48)
503511
>> POP_TOP
504512
505-
%3d >> LOAD_CONST 2 (2)
513+
%3d LOAD_CONST 2 (2)
506514
STORE_FAST 2 (y)
507515
LOAD_CONST 0 (None)
508516
RETURN_VALUE
@@ -526,7 +534,11 @@ async def _asyncwith(c):
526534
POP_EXCEPT
527535
POP_TOP
528536
POP_TOP
529-
JUMP_BACKWARD 24 (to 58)
537+
538+
%3d LOAD_CONST 2 (2)
539+
STORE_FAST 2 (y)
540+
LOAD_CONST 0 (None)
541+
RETURN_VALUE
530542
>> COPY 3
531543
POP_EXCEPT
532544
RERAISE 1
@@ -538,6 +550,7 @@ async def _asyncwith(c):
538550
_asyncwith.__code__.co_firstlineno + 1,
539551
_asyncwith.__code__.co_firstlineno + 3,
540552
_asyncwith.__code__.co_firstlineno + 1,
553+
_asyncwith.__code__.co_firstlineno + 3,
541554
)
542555

543556

Lib/test/test_peepholer.py

-2
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,6 @@ def f(x):
344344
self.assertEqual(len(returns), 1)
345345
self.check_lnotab(f)
346346

347-
@unittest.skip("Following gh-92228 the return has two predecessors "
348-
"and that prevents jump elimination.")
349347
def test_elim_jump_to_return(self):
350348
# JUMP_FORWARD to RETURN --> RETURN
351349
def f(cond, true_value, false_value):

Python/compile.c

+63-90
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ static int compiler_match(struct compiler *, stmt_ty);
498498
static int compiler_pattern_subpattern(struct compiler *, pattern_ty,
499499
pattern_context *);
500500

501-
static void clean_basic_block(basicblock *bb);
501+
static void remove_redundant_nops(basicblock *bb);
502502

503503
static PyCodeObject *assemble(struct compiler *, int addNone);
504504

@@ -7364,7 +7364,7 @@ mark_cold(basicblock *entryblock) {
73647364
for (int i = 0; i < b->b_iused; i++) {
73657365
struct instr *instr = &b->b_instr[i];
73667366
if (is_jump(instr)) {
7367-
assert(i == b->b_iused-1);
7367+
assert(i == b->b_iused - 1);
73687368
basicblock *target = b->b_instr[i].i_target;
73697369
if (!target->b_warm && !target->b_visited) {
73707370
*sp++ = target;
@@ -8271,9 +8271,6 @@ dump_basicblock(const basicblock *b)
82718271
#endif
82728272

82738273

8274-
static int
8275-
normalize_basic_block(basicblock *bb);
8276-
82778274
static int
82788275
calculate_jump_targets(basicblock *entryblock);
82798276

@@ -8325,7 +8322,7 @@ insert_instruction(basicblock *block, int pos, struct instr *instr) {
83258322
if (basicblock_next_instr(block) < 0) {
83268323
return -1;
83278324
}
8328-
for (int i = block->b_iused-1; i > pos; i--) {
8325+
for (int i = block->b_iused - 1; i > pos; i--) {
83298326
block->b_instr[i] = block->b_instr[i-1];
83308327
}
83318328
block->b_instr[pos] = *instr;
@@ -8488,23 +8485,32 @@ fix_cell_offsets(struct compiler *c, basicblock *entryblock, int *fixedmap)
84888485
static void
84898486
propagate_line_numbers(basicblock *entryblock);
84908487

8491-
static void
8492-
eliminate_empty_basic_blocks(cfg_builder *g);
8493-
84948488
#ifndef NDEBUG
84958489
static bool
84968490
no_redundant_jumps(cfg_builder *g) {
84978491
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
84988492
struct instr *last = basicblock_last_instr(b);
84998493
if (last != NULL) {
8500-
if (last->i_opcode == JUMP || last->i_opcode == JUMP_NO_INTERRUPT) {
8494+
if (IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) {
85018495
assert(last->i_target != b->b_next);
8502-
return false;
8496+
if (last->i_target == b->b_next) {
8497+
return false;
8498+
}
85038499
}
85048500
}
85058501
}
85068502
return true;
85078503
}
8504+
8505+
static bool
8506+
no_empty_basic_blocks(cfg_builder *g) {
8507+
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
8508+
if (b->b_iused == 0) {
8509+
return false;
8510+
}
8511+
}
8512+
return true;
8513+
}
85088514
#endif
85098515

85108516
static int
@@ -8514,28 +8520,22 @@ remove_redundant_jumps(cfg_builder *g) {
85148520
* of that jump. If it is, then the jump instruction is redundant and
85158521
* can be deleted.
85168522
*/
8517-
int removed = 0;
8523+
assert(no_empty_basic_blocks(g));
85188524
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
85198525
struct instr *last = basicblock_last_instr(b);
8520-
if (last != NULL) {
8521-
assert(!IS_ASSEMBLER_OPCODE(last->i_opcode));
8522-
if (last->i_opcode == JUMP ||
8523-
last->i_opcode == JUMP_NO_INTERRUPT) {
8524-
if (last->i_target == NULL) {
8525-
PyErr_SetString(PyExc_SystemError, "jump with NULL target");
8526-
return -1;
8527-
}
8528-
if (last->i_target == b->b_next) {
8529-
assert(b->b_next->b_iused);
8530-
last->i_opcode = NOP;
8531-
removed++;
8532-
}
8526+
assert(last != NULL);
8527+
assert(!IS_ASSEMBLER_OPCODE(last->i_opcode));
8528+
if (IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) {
8529+
if (last->i_target == NULL) {
8530+
PyErr_SetString(PyExc_SystemError, "jump with NULL target");
8531+
return -1;
8532+
}
8533+
if (last->i_target == b->b_next) {
8534+
assert(b->b_next->b_iused);
8535+
last->i_opcode = NOP;
85338536
}
85348537
}
85358538
}
8536-
if (removed) {
8537-
eliminate_empty_basic_blocks(g);
8538-
}
85398539
return 0;
85408540
}
85418541

@@ -8643,7 +8643,7 @@ assemble(struct compiler *c, int addNone)
86438643
goto error;
86448644
}
86458645
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
8646-
clean_basic_block(b);
8646+
remove_redundant_nops(b);
86478647
}
86488648

86498649
/* Order of basic blocks must have been determined by now */
@@ -9003,10 +9003,7 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
90039003
int oparg = inst->i_oparg;
90049004
int nextop = i+1 < bb->b_iused ? bb->b_instr[i+1].i_opcode : 0;
90059005
if (HAS_TARGET(inst->i_opcode)) {
9006-
/* Skip over empty basic blocks. */
9007-
while (inst->i_target->b_iused == 0) {
9008-
inst->i_target = inst->i_target->b_next;
9009-
}
9006+
assert(inst->i_target->b_iused > 0);
90109007
target = &inst->i_target->b_instr[0];
90119008
assert(!IS_ASSEMBLER_OPCODE(target->i_opcode));
90129009
}
@@ -9230,50 +9227,36 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
92309227
return -1;
92319228
}
92329229

9233-
static bool
9234-
basicblock_has_lineno(const basicblock *bb) {
9235-
for (int i = 0; i < bb->b_iused; i++) {
9236-
if (bb->b_instr[i].i_loc.lineno > 0) {
9237-
return true;
9238-
}
9239-
}
9240-
return false;
9241-
}
9242-
9243-
/* If this block ends with an unconditional jump to an exit block,
9244-
* then remove the jump and extend this block with the target.
9230+
/* If this block ends with an unconditional jump to a small exit block, then
9231+
* remove the jump and extend this block with the target.
9232+
* Returns 1 if extended, 0 if no change, and -1 on error.
92459233
*/
92469234
static int
9247-
extend_block(basicblock *bb) {
9235+
inline_small_exit_blocks(basicblock *bb) {
92489236
struct instr *last = basicblock_last_instr(bb);
92499237
if (last == NULL) {
92509238
return 0;
92519239
}
9252-
if (last->i_opcode != JUMP &&
9253-
last->i_opcode != JUMP_FORWARD &&
9254-
last->i_opcode != JUMP_BACKWARD) {
9240+
if (!IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) {
92559241
return 0;
92569242
}
9257-
if (basicblock_exits_scope(last->i_target) && last->i_target->b_iused <= MAX_COPY_SIZE) {
9258-
basicblock *to_copy = last->i_target;
9259-
if (basicblock_has_lineno(to_copy)) {
9260-
/* copy only blocks without line number (like implicit 'return None's) */
9261-
return 0;
9262-
}
9243+
basicblock *target = last->i_target;
9244+
if (basicblock_exits_scope(target) && target->b_iused <= MAX_COPY_SIZE) {
92639245
last->i_opcode = NOP;
9264-
for (int i = 0; i < to_copy->b_iused; i++) {
9246+
for (int i = 0; i < target->b_iused; i++) {
92659247
int index = basicblock_next_instr(bb);
92669248
if (index < 0) {
92679249
return -1;
92689250
}
9269-
bb->b_instr[index] = to_copy->b_instr[i];
9251+
bb->b_instr[index] = target->b_instr[i];
92709252
}
9253+
return 1;
92719254
}
92729255
return 0;
92739256
}
92749257

92759258
static void
9276-
clean_basic_block(basicblock *bb) {
9259+
remove_redundant_nops(basicblock *bb) {
92779260
/* Remove NOPs when legal to do so. */
92789261
int dest = 0;
92799262
int prev_lineno = -1;
@@ -9324,24 +9307,17 @@ clean_basic_block(basicblock *bb) {
93249307
}
93259308

93269309
static int
9327-
normalize_basic_block(basicblock *bb) {
9328-
/* Skip over empty blocks.
9329-
* Raise SystemError if jump or exit is not last instruction in the block. */
9330-
for (int i = 0; i < bb->b_iused; i++) {
9331-
int opcode = bb->b_instr[i].i_opcode;
9332-
assert(!IS_ASSEMBLER_OPCODE(opcode));
9333-
int is_jump = IS_JUMP_OPCODE(opcode);
9334-
int is_exit = IS_SCOPE_EXIT_OPCODE(opcode);
9335-
if (is_exit || is_jump) {
9336-
if (i != bb->b_iused-1) {
9337-
PyErr_SetString(PyExc_SystemError, "malformed control flow graph.");
9338-
return -1;
9339-
}
9340-
}
9341-
if (is_jump) {
9342-
/* Skip over empty basic blocks. */
9343-
while (bb->b_instr[i].i_target->b_iused == 0) {
9344-
bb->b_instr[i].i_target = bb->b_instr[i].i_target->b_next;
9310+
check_cfg(cfg_builder *g) {
9311+
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
9312+
/* Raise SystemError if jump or exit is not last instruction in the block. */
9313+
for (int i = 0; i < b->b_iused; i++) {
9314+
int opcode = b->b_instr[i].i_opcode;
9315+
assert(!IS_ASSEMBLER_OPCODE(opcode));
9316+
if (IS_TERMINATOR_OPCODE(opcode)) {
9317+
if (i != b->b_iused - 1) {
9318+
PyErr_SetString(PyExc_SystemError, "malformed control flow graph.");
9319+
return -1;
9320+
}
93459321
}
93469322
}
93479323
}
@@ -9512,25 +9488,25 @@ static int
95129488
optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache)
95139489
{
95149490
assert(PyDict_CheckExact(const_cache));
9515-
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
9516-
if (normalize_basic_block(b)) {
9517-
return -1;
9518-
}
9491+
if (check_cfg(g) < 0) {
9492+
return -1;
95199493
}
9494+
eliminate_empty_basic_blocks(g);
95209495
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
9521-
if (extend_block(b) < 0) {
9496+
if (inline_small_exit_blocks(b) < 0) {
95229497
return -1;
95239498
}
95249499
}
9500+
assert(no_empty_basic_blocks(g));
95259501
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
95269502
if (optimize_basic_block(const_cache, b, consts)) {
95279503
return -1;
95289504
}
9529-
clean_basic_block(b);
9505+
remove_redundant_nops(b);
95309506
assert(b->b_predecessors == 0);
95319507
}
95329508
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
9533-
if (extend_block(b) < 0) {
9509+
if (inline_small_exit_blocks(b) < 0) {
95349510
return -1;
95359511
}
95369512
}
@@ -9545,7 +9521,7 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache)
95459521
}
95469522
eliminate_empty_basic_blocks(g);
95479523
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
9548-
clean_basic_block(b);
9524+
remove_redundant_nops(b);
95499525
}
95509526
if (remove_redundant_jumps(g) < 0) {
95519527
return -1;
@@ -9627,12 +9603,9 @@ duplicate_exits_without_lineno(cfg_builder *g)
96279603
}
96289604
}
96299605
}
9630-
/* Eliminate empty blocks */
9631-
for (basicblock *b = entryblock; b != NULL; b = b->b_next) {
9632-
while (b->b_next && b->b_next->b_iused == 0) {
9633-
b->b_next = b->b_next->b_next;
9634-
}
9635-
}
9606+
9607+
assert(no_empty_basic_blocks(g));
9608+
96369609
/* Any remaining reachable exit blocks without line number can only be reached by
96379610
* fall through, and thus can only have a single predecessor */
96389611
for (basicblock *b = entryblock; b != NULL; b = b->b_next) {

0 commit comments

Comments
 (0)