From bfd13f19dc80680b8cbbb80ac8d74b4929c18960 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 24 Jan 2024 14:21:06 +0000 Subject: [PATCH 1/7] gh-107901: replace POP_BLOCK instruction by NOPs earlier --- Python/flowgraph.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 2fc90b8877b475..3781f9bbfd1241 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -263,8 +263,9 @@ dump_instr(cfg_instr *i) if (HAS_TARGET(i->i_opcode)) { sprintf(arg, "target: %p [%d] ", i->i_target, i->i_oparg); } - fprintf(stderr, "line: %d, %s (%d) %s%s\n", - i->i_loc.lineno, _PyOpcode_OpName[i->i_opcode], i->i_opcode, arg, jump); + fprintf(stderr, "line: %d%s, %s (%d) %s%s\n", + i->i_loc.lineno, i->i_loc_propagated ? "(<-)" : "", + _PyOpcode_OpName[i->i_opcode], i->i_opcode, arg, jump); } static inline int @@ -938,6 +939,15 @@ label_exception_targets(basicblock *entryblock) { PyMem_Free(except_stack); } } + + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { + for (int i = 0; i < b->b_iused; i++) { + cfg_instr *instr = &b->b_instr[i]; + if (instr->i_opcode == POP_BLOCK) { + INSTR_SET_OP0(instr, NOP); + } + } + } #ifdef Py_DEBUG for (basicblock *b = entryblock; b != NULL; b = b->b_next) { assert(b->b_exceptstack == NULL); @@ -1006,8 +1016,9 @@ basicblock_remove_redundant_nops(basicblock *bb) { int dest = 0; int prev_lineno = -1; for (int src = 0; src < bb->b_iused; src++) { - int lineno = bb->b_instr[src].i_loc.lineno; - if (bb->b_instr[src].i_opcode == NOP) { + cfg_instr *src_instr = &bb->b_instr[src]; + int lineno = src_instr->i_loc.lineno; + if (src_instr->i_opcode == NOP) { /* Eliminate no-op if it doesn't have a line number */ if (lineno < 0) { continue; @@ -1018,30 +1029,35 @@ basicblock_remove_redundant_nops(basicblock *bb) { } /* or, if the next instruction has same line number or no line number */ if (src < bb->b_iused - 1) { - int next_lineno = bb->b_instr[src+1].i_loc.lineno; + cfg_instr *next_instr = &bb->b_instr[src + 1]; + int next_lineno = next_instr->i_loc.lineno; if (next_lineno == lineno) { + next_instr->i_loc_propagated = src_instr->i_loc_propagated; continue; } if (next_lineno < 0) { - bb->b_instr[src+1].i_loc = bb->b_instr[src].i_loc; + next_instr->i_loc = src_instr->i_loc; + next_instr->i_loc_propagated = src_instr->i_loc_propagated; continue; } } else { - basicblock *next = next_nonempty_block(bb->b_next); /* or if last instruction in BB and next BB has same line number */ + basicblock *next = next_nonempty_block(bb->b_next); if (next) { - location next_loc = NO_LOCATION; + cfg_instr *next_instr = NULL; for (int next_i=0; next_i < next->b_iused; next_i++) { - cfg_instr *instr = &next->b_instr[next_i]; - if (instr->i_opcode == NOP && instr->i_loc.lineno == NO_LOCATION.lineno) { + next_instr = &next->b_instr[next_i]; + if (next_instr->i_opcode == NOP && + next_instr->i_loc.lineno == NO_LOCATION.lineno) + { /* Skip over NOPs without location, they will be removed */ continue; } - next_loc = instr->i_loc; break; } - if (lineno == next_loc.lineno) { + if (next_instr && lineno == next_instr->i_loc.lineno) { + next_instr->i_loc_propagated = src_instr->i_loc_propagated; continue; } } @@ -2304,7 +2320,7 @@ convert_pseudo_ops(cfg_builder *g) for (basicblock *b = entryblock; b != NULL; b = b->b_next) { for (int i = 0; i < b->b_iused; i++) { cfg_instr *instr = &b->b_instr[i]; - if (is_block_push(instr) || instr->i_opcode == POP_BLOCK) { + if (is_block_push(instr)) { INSTR_SET_OP0(instr, NOP); } else if (instr->i_opcode == LOAD_CLOSURE) { From 63e5c9cdb85d773b58628d13f282b40e97eb2e5a Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 24 Jan 2024 14:48:40 +0000 Subject: [PATCH 2/7] propagate only if false --- Python/flowgraph.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 3781f9bbfd1241..a019745e5eaf44 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -1032,12 +1032,16 @@ basicblock_remove_redundant_nops(basicblock *bb) { cfg_instr *next_instr = &bb->b_instr[src + 1]; int next_lineno = next_instr->i_loc.lineno; if (next_lineno == lineno) { - next_instr->i_loc_propagated = src_instr->i_loc_propagated; + if (!src_instr->i_loc_propagated) { + next_instr->i_loc_propagated = 0; + } continue; } if (next_lineno < 0) { next_instr->i_loc = src_instr->i_loc; - next_instr->i_loc_propagated = src_instr->i_loc_propagated; + if (!src_instr->i_loc_propagated) { + next_instr->i_loc_propagated = 0; + } continue; } } @@ -1057,7 +1061,9 @@ basicblock_remove_redundant_nops(basicblock *bb) { break; } if (next_instr && lineno == next_instr->i_loc.lineno) { - next_instr->i_loc_propagated = src_instr->i_loc_propagated; + if (!src_instr->i_loc_propagated) { + next_instr->i_loc_propagated = 0; + } continue; } } From 5accf19462c76d4b39917d900f49e0110c021ae3 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 25 Jan 2024 12:56:42 +0000 Subject: [PATCH 3/7] Revert "propagate only if false" This reverts commit 63e5c9cdb85d773b58628d13f282b40e97eb2e5a. --- Python/flowgraph.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index a019745e5eaf44..3781f9bbfd1241 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -1032,16 +1032,12 @@ basicblock_remove_redundant_nops(basicblock *bb) { cfg_instr *next_instr = &bb->b_instr[src + 1]; int next_lineno = next_instr->i_loc.lineno; if (next_lineno == lineno) { - if (!src_instr->i_loc_propagated) { - next_instr->i_loc_propagated = 0; - } + next_instr->i_loc_propagated = src_instr->i_loc_propagated; continue; } if (next_lineno < 0) { next_instr->i_loc = src_instr->i_loc; - if (!src_instr->i_loc_propagated) { - next_instr->i_loc_propagated = 0; - } + next_instr->i_loc_propagated = src_instr->i_loc_propagated; continue; } } @@ -1061,9 +1057,7 @@ basicblock_remove_redundant_nops(basicblock *bb) { break; } if (next_instr && lineno == next_instr->i_loc.lineno) { - if (!src_instr->i_loc_propagated) { - next_instr->i_loc_propagated = 0; - } + next_instr->i_loc_propagated = src_instr->i_loc_propagated; continue; } } From 0d8e0dd731300ca941527ec40221be3742875df1 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 25 Jan 2024 12:56:51 +0000 Subject: [PATCH 4/7] Revert "gh-107901: replace POP_BLOCK instruction by NOPs earlier" This reverts commit bfd13f19dc80680b8cbbb80ac8d74b4929c18960. --- Python/flowgraph.c | 42 +++++++++++++----------------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 3781f9bbfd1241..2fc90b8877b475 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -263,9 +263,8 @@ dump_instr(cfg_instr *i) if (HAS_TARGET(i->i_opcode)) { sprintf(arg, "target: %p [%d] ", i->i_target, i->i_oparg); } - fprintf(stderr, "line: %d%s, %s (%d) %s%s\n", - i->i_loc.lineno, i->i_loc_propagated ? "(<-)" : "", - _PyOpcode_OpName[i->i_opcode], i->i_opcode, arg, jump); + fprintf(stderr, "line: %d, %s (%d) %s%s\n", + i->i_loc.lineno, _PyOpcode_OpName[i->i_opcode], i->i_opcode, arg, jump); } static inline int @@ -939,15 +938,6 @@ label_exception_targets(basicblock *entryblock) { PyMem_Free(except_stack); } } - - for (basicblock *b = entryblock; b != NULL; b = b->b_next) { - for (int i = 0; i < b->b_iused; i++) { - cfg_instr *instr = &b->b_instr[i]; - if (instr->i_opcode == POP_BLOCK) { - INSTR_SET_OP0(instr, NOP); - } - } - } #ifdef Py_DEBUG for (basicblock *b = entryblock; b != NULL; b = b->b_next) { assert(b->b_exceptstack == NULL); @@ -1016,9 +1006,8 @@ basicblock_remove_redundant_nops(basicblock *bb) { int dest = 0; int prev_lineno = -1; for (int src = 0; src < bb->b_iused; src++) { - cfg_instr *src_instr = &bb->b_instr[src]; - int lineno = src_instr->i_loc.lineno; - if (src_instr->i_opcode == NOP) { + int lineno = bb->b_instr[src].i_loc.lineno; + if (bb->b_instr[src].i_opcode == NOP) { /* Eliminate no-op if it doesn't have a line number */ if (lineno < 0) { continue; @@ -1029,35 +1018,30 @@ basicblock_remove_redundant_nops(basicblock *bb) { } /* or, if the next instruction has same line number or no line number */ if (src < bb->b_iused - 1) { - cfg_instr *next_instr = &bb->b_instr[src + 1]; - int next_lineno = next_instr->i_loc.lineno; + int next_lineno = bb->b_instr[src+1].i_loc.lineno; if (next_lineno == lineno) { - next_instr->i_loc_propagated = src_instr->i_loc_propagated; continue; } if (next_lineno < 0) { - next_instr->i_loc = src_instr->i_loc; - next_instr->i_loc_propagated = src_instr->i_loc_propagated; + bb->b_instr[src+1].i_loc = bb->b_instr[src].i_loc; continue; } } else { - /* or if last instruction in BB and next BB has same line number */ basicblock *next = next_nonempty_block(bb->b_next); + /* or if last instruction in BB and next BB has same line number */ if (next) { - cfg_instr *next_instr = NULL; + location next_loc = NO_LOCATION; for (int next_i=0; next_i < next->b_iused; next_i++) { - next_instr = &next->b_instr[next_i]; - if (next_instr->i_opcode == NOP && - next_instr->i_loc.lineno == NO_LOCATION.lineno) - { + cfg_instr *instr = &next->b_instr[next_i]; + if (instr->i_opcode == NOP && instr->i_loc.lineno == NO_LOCATION.lineno) { /* Skip over NOPs without location, they will be removed */ continue; } + next_loc = instr->i_loc; break; } - if (next_instr && lineno == next_instr->i_loc.lineno) { - next_instr->i_loc_propagated = src_instr->i_loc_propagated; + if (lineno == next_loc.lineno) { continue; } } @@ -2320,7 +2304,7 @@ convert_pseudo_ops(cfg_builder *g) for (basicblock *b = entryblock; b != NULL; b = b->b_next) { for (int i = 0; i < b->b_iused; i++) { cfg_instr *instr = &b->b_instr[i]; - if (is_block_push(instr)) { + if (is_block_push(instr) || instr->i_opcode == POP_BLOCK) { INSTR_SET_OP0(instr, NOP); } else if (instr->i_opcode == LOAD_CLOSURE) { From 7004b2612af55c3113c07cd4ee0e3f66c314b1e9 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 25 Jan 2024 13:00:53 +0000 Subject: [PATCH 5/7] gh-107901: compiler replaces POP_BLOCK instruction by NOPs before optimisations --- Python/flowgraph.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index de831358eb9ac8..4b60ca496aad79 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -953,6 +953,15 @@ label_exception_targets(basicblock *entryblock) { PyMem_Free(except_stack); } } + + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { + for (int i = 0; i < b->b_iused; i++) { + cfg_instr *instr = &b->b_instr[i]; + if (instr->i_opcode == POP_BLOCK) { + INSTR_SET_OP0(instr, NOP); + } + } + } #ifdef Py_DEBUG for (basicblock *b = entryblock; b != NULL; b = b->b_next) { assert(b->b_exceptstack == NULL); @@ -2313,7 +2322,7 @@ convert_pseudo_ops(cfg_builder *g) for (basicblock *b = entryblock; b != NULL; b = b->b_next) { for (int i = 0; i < b->b_iused; i++) { cfg_instr *instr = &b->b_instr[i]; - if (is_block_push(instr) || instr->i_opcode == POP_BLOCK) { + if (is_block_push(instr)) { INSTR_SET_OP0(instr, NOP); } else if (instr->i_opcode == LOAD_CLOSURE) { From 111fc78806d35a60402f0136928a966760be8fee Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 25 Jan 2024 15:54:25 +0000 Subject: [PATCH 6/7] convert in main loop --- Python/flowgraph.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 4b60ca496aad79..2061819d859def 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -903,6 +903,7 @@ label_exception_targets(basicblock *entryblock) { } else if (instr->i_opcode == POP_BLOCK) { handler = pop_except_block(except_stack); + INSTR_SET_OP0(instr, NOP); } else if (is_jump(instr)) { instr->i_except = handler; @@ -954,14 +955,6 @@ label_exception_targets(basicblock *entryblock) { } } - for (basicblock *b = entryblock; b != NULL; b = b->b_next) { - for (int i = 0; i < b->b_iused; i++) { - cfg_instr *instr = &b->b_instr[i]; - if (instr->i_opcode == POP_BLOCK) { - INSTR_SET_OP0(instr, NOP); - } - } - } #ifdef Py_DEBUG for (basicblock *b = entryblock; b != NULL; b = b->b_next) { assert(b->b_exceptstack == NULL); From a5c4c8c71dd9283c389d718b05bc70bd67780aa9 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Thu, 25 Jan 2024 15:55:10 +0000 Subject: [PATCH 7/7] whitespace --- Python/flowgraph.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 2061819d859def..96610b3cb11a43 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -954,7 +954,6 @@ label_exception_targets(basicblock *entryblock) { PyMem_Free(except_stack); } } - #ifdef Py_DEBUG for (basicblock *b = entryblock; b != NULL; b = b->b_next) { assert(b->b_exceptstack == NULL);