Skip to content

gh-102859: Remove JUMP_IF_FALSE_OR_POP and JUMP_IF_TRUE_OR_POP #102870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iritkatriel merged 9 commits into from
Mar 22, 2023
Merged

gh-102859: Remove JUMP_IF_FALSE_OR_POP and JUMP_IF_TRUE_OR_POP #102870

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1e63ce0
gh-102859: remove opcodes JUMP_IF_FALSE_OR_POP and JUMP_IF_TRUE_OR_POP
iritkatriel Mar 21, 2023
96602a0
add news
iritkatriel Mar 21, 2023
22ed3a6
fix peepholer
iritkatriel Mar 21, 2023
08c0826
Merge remote-tracking branch 'upstream/main' into remove-JUMP_IF_FALS…
iritkatriel Mar 21, 2023
0097e54
fix optimizations
iritkatriel Mar 21, 2023
12595f6
init 'target'
iritkatriel Mar 21, 2023
b2bf829
Merge branch 'main' into remove-JUMP_IF_FALSE_OR_POP
iritkatriel Mar 22, 2023
ddb3003
make remove_redundant_nops_and_pairs safe
iritkatriel Mar 22, 2023
ab2dfe1
Merge remote-tracking branch 'upstream/main' into remove-JUMP_IF_FALS…
iritkatriel Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix peepholer
  • Loading branch information
@iritkatriel
iritkatriel committed Mar 21, 2023
commit 22ed3a66d63b0faf93f6e1616b2b1fb34f97ca6b
24 changes: 9 additions & 15 deletions Lib/test/test_peepholer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ def check_jump_targets(self, code):
tgt.opname == 'RETURN_VALUE'):
self.fail(f'{instr.opname} at {instr.offset} '
f'jumps to {tgt.opname} at {tgt.offset}')
# JUMP_IF_*_OR_POP jump to conditional jump
if '_OR_POP' in instr.opname and 'JUMP_IF_' in tgt.opname:
self.fail(f'{instr.opname} at {instr.offset} '
f'jumps to {tgt.opname} at {tgt.offset}')

def check_lnotab(self, code):
"Check that the lnotab byte offsets are sensible."
Expand Down Expand Up @@ -384,38 +380,36 @@ def f():

def test_elim_jump_to_uncond_jump3(self):
# Intentionally use two-line expressions to test issue37213.
# JUMP_IF_FALSE_OR_POP to JUMP_IF_FALSE_OR_POP --> JUMP_IF_FALSE_OR_POP to non-jump
# POP_JUMP_IF_FALSE to POP_JUMP_IF_FALSE --> POP_JUMP_IF_FALSE to non-jump
def f(a, b, c):
return ((a and b)
and c)
self.check_jump_targets(f)
self.check_lnotab(f)
self.assertEqual(count_instr_recursively(f, 'JUMP_IF_FALSE_OR_POP'), 2)
# JUMP_IF_TRUE_OR_POP to JUMP_IF_TRUE_OR_POP --> JUMP_IF_TRUE_OR_POP to non-jump
self.assertEqual(count_instr_recursively(f, 'POP_JUMP_IF_FALSE'), 2)
# POP_JUMP_IF_TRUE to POP_JUMP_IF_TRUE --> POP_JUMP_IF_TRUE to non-jump
def f(a, b, c):
return ((a or b)
or c)
self.check_jump_targets(f)
self.check_lnotab(f)
self.assertEqual(count_instr_recursively(f, 'JUMP_IF_TRUE_OR_POP'), 2)
self.assertEqual(count_instr_recursively(f, 'POP_JUMP_IF_TRUE'), 2)
# JUMP_IF_FALSE_OR_POP to JUMP_IF_TRUE_OR_POP --> POP_JUMP_IF_FALSE to non-jump
def f(a, b, c):
return ((a and b)
or c)
self.check_jump_targets(f)
self.check_lnotab(f)
self.assertNotInBytecode(f, 'JUMP_IF_FALSE_OR_POP')
self.assertInBytecode(f, 'JUMP_IF_TRUE_OR_POP')
self.assertInBytecode(f, 'POP_JUMP_IF_FALSE')
# JUMP_IF_TRUE_OR_POP to JUMP_IF_FALSE_OR_POP --> POP_JUMP_IF_TRUE to non-jump
self.assertEqual(count_instr_recursively(f, 'POP_JUMP_IF_FALSE'), 1)
self.assertEqual(count_instr_recursively(f, 'POP_JUMP_IF_TRUE'), 1)
# POP_JUMP_IF_TRUE to POP_JUMP_IF_FALSE --> POP_JUMP_IF_TRUE to non-jump
def f(a, b, c):
return ((a or b)
and c)
self.check_jump_targets(f)
self.check_lnotab(f)
self.assertNotInBytecode(f, 'JUMP_IF_TRUE_OR_POP')
self.assertInBytecode(f, 'JUMP_IF_FALSE_OR_POP')
self.assertInBytecode(f, 'POP_JUMP_IF_TRUE')
self.assertEqual(count_instr_recursively(f, 'POP_JUMP_IF_FALSE'), 1)
self.assertEqual(count_instr_recursively(f, 'POP_JUMP_IF_TRUE'), 1)

def test_elim_jump_to_uncond_jump4(self):
def f():
Expand Down
35 changes: 22 additions & 13 deletions Python/compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9092,20 +9092,29 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
struct cfg_instr nop;
INSTR_SET_OP0(&nop, NOP);
struct cfg_instr *target;
int opcode = 0;
int oparg = 0;
int nextop = 0;
for (int i = 0; i < bb->b_iused; i++) {
struct cfg_instr *inst = &bb->b_instr[i];
int oparg = inst->i_oparg;
int nextop = i+1 < bb->b_iused ? bb->b_instr[i+1].i_opcode : 0;
if (HAS_TARGET(inst->i_opcode)) {
assert(inst->i_target->b_iused > 0);
target = &inst->i_target->b_instr[0];
assert(!IS_ASSEMBLER_OPCODE(target->i_opcode));
}
else {
target = &nop;
bool is_copy_of_load_const = (opcode == LOAD_CONST &&
inst->i_opcode == COPY &&
inst->i_oparg == 1);
if (! is_copy_of_load_const) {
opcode = inst->i_opcode;
oparg = inst->i_oparg;
nextop = i+1 < bb->b_iused ? bb->b_instr[i+1].i_opcode : 0;
if (HAS_TARGET(opcode)) {
assert(inst->i_target->b_iused > 0);
target = &inst->i_target->b_instr[0];
assert(!IS_ASSEMBLER_OPCODE(target->i_opcode));
}
else {
target = &nop;
}
Comment thread
carljm marked this conversation as resolved.
}
assert(!IS_ASSEMBLER_OPCODE(inst->i_opcode));
switch (inst->i_opcode) {
assert(!IS_ASSEMBLER_OPCODE(opcode));
switch (opcode) {
/* Remove LOAD_CONST const; conditional jump */
case LOAD_CONST:
{
Expand All @@ -9115,7 +9124,7 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
switch(nextop) {
case POP_JUMP_IF_FALSE:
case POP_JUMP_IF_TRUE:
cnt = get_const_value(inst->i_opcode, oparg, consts);
cnt = get_const_value(opcode, oparg, consts);
if (cnt == NULL) {
goto error;
}
Expand All @@ -9134,7 +9143,7 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
}
break;
case IS_OP:
cnt = get_const_value(inst->i_opcode, oparg, consts);
cnt = get_const_value(opcode, oparg, consts);
if (cnt == NULL) {
goto error;
}
Expand Down