Skip to content

gh-104635: Eliminate redundant STORE_FAST instructions in the compiler #105320

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.

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
merged 2 commits into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions Lib/test/test_dis.py
Original file line number Diff line number Diff line change
Expand Up @@ -808,8 +808,9 @@ def extended_arg_quick():
%3d 2 LOAD_CONST 1 (Ellipsis)
4 EXTENDED_ARG 1
6 UNPACK_EX 256
8 STORE_FAST_STORE_FAST 0 (_, _)
10 RETURN_CONST 0 (None)
8 POP_TOP
10 STORE_FAST 0 (_)
12 RETURN_CONST 0 (None)
"""% (extended_arg_quick.__code__.co_firstlineno,
extended_arg_quick.__code__.co_firstlineno + 1,)

Expand Down
36 changes: 34 additions & 2 deletions Lib/test/test_peepholer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1075,15 +1075,47 @@ def test_no_unsafe_static_swap(self):
('RETURN_VALUE', 5)
]
expected_insts = [
('LOAD_CONST', 0, 1),
('LOAD_CONST', 1, 2),
('NOP', 0, 3),
('STORE_FAST', 1, 4),
('POP_TOP', 0, 4),
('RETURN_VALUE', 5)
]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 instructions -> 5 instructions

self.cfg_optimization_test(insts, expected_insts, consts=list(range(3)), nlocals=1)

def test_dead_store_elimination_in_same_lineno(self):
insts = [
('LOAD_CONST', 0, 1),
('LOAD_CONST', 1, 2),
('LOAD_CONST', 2, 3),
('SWAP', 3, 4),
('STORE_FAST_STORE_FAST', 17, 4),
('STORE_FAST', 1, 4),
('STORE_FAST', 1, 4),
('STORE_FAST', 1, 4),
('RETURN_VALUE', 5)
]
expected_insts = [
('LOAD_CONST', 0, 1),
('LOAD_CONST', 1, 2),
('NOP', 0, 3),
('POP_TOP', 0, 4),
('STORE_FAST', 1, 4),
('RETURN_VALUE', 5)
]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 instructions -> 5 instructions

self.cfg_optimization_test(insts, expected_insts, consts=list(range(3)), nlocals=1)

def test_no_dead_store_elimination_in_different_lineno(self):
insts = [
('LOAD_CONST', 0, 1),
('LOAD_CONST', 1, 2),
('LOAD_CONST', 2, 3),
('STORE_FAST', 1, 4),
('STORE_FAST', 1, 5),
('STORE_FAST', 1, 6),
('RETURN_VALUE', 5)
]
self.cfg_optimization_test(insts, insts, consts=list(range(3)), nlocals=1)


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Eliminate redundant :opcode:`STORE_FAST` instructions in the compiler. Patch
by Dong-hee Na and Carl Meyer.
23 changes: 18 additions & 5 deletions Python/flowgraph.c
Original file line number Diff line number Diff line change
Expand Up @@ -1515,15 +1515,18 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
*/
}
break;
case STORE_FAST:
if (opcode == nextop &&
oparg == bb->b_instr[i+1].i_oparg &&
bb->b_instr[i].i_loc.lineno == bb->b_instr[i+1].i_loc.lineno) {
bb->b_instr[i].i_opcode = POP_TOP;
bb->b_instr[i].i_oparg = 0;
}
break;
case SWAP:
if (oparg == 1) {
INSTR_SET_OP0(inst, NOP);
break;
}
if (swaptimize(bb, &i) < 0) {
goto error;
}
apply_static_swaps(bb, i);
break;
case KW_NAMES:
break;
Expand All @@ -1538,6 +1541,16 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
assert (!HAS_CONST(inst->i_opcode));
}
}

for (int i = 0; i < bb->b_iused; i++) {
cfg_instr *inst = &bb->b_instr[i];
if (inst->i_opcode == SWAP) {
if (swaptimize(bb, &i) < 0) {
goto error;
}
apply_static_swaps(bb, i);
}
}
return SUCCESS;
error:
return ERROR;
Expand Down