pythongh-104635: Eliminate redundant STORE_FAST instructions in the c…

…ompiler
corona10 · Jun 5, 2023 · 1a8881b · 1a8881b
1 parent 058b960
commit 1a8881b
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 9 deletions.
diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py
@@ -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,)
 

diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py
@@ -1075,15 +1075,56 @@ 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)
+        ]
+        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)
+        ]
+        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)
+        ]
+        expected_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, expected_insts, consts=list(range(3)), nlocals=1)
 
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-06-05-23-38-43.gh-issue-104635.VYZhVh.rst b/Misc/NEWS.d/next/Core and Builtins/2023-06-05-23-38-43.gh-issue-104635.VYZhVh.rst
@@ -0,0 +1,2 @@
+Eliminate redundant :opcode:`STORE_FAST` instructions in the compiler. Patch
+by Dong-hee Na and Carl Meyer.
diff --git a/Python/flowgraph.c b/Python/flowgraph.c
@@ -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;
@@ -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;