Eliminate redundant refcounting from BINARY_SLICE

Author: cocolato

Include/internal/pycore_opcode_metadata.h

@@ -2098,6 +2098,23 @@ def testfunc(n):
         self.assertIn("_BINARY_OP_SUBSCR_DICT", uops)
         self.assertLessEqual(count_ops(ex, "_POP_TOP"), 2)
 
+    def test_binary_slice(self):
+        def testfunc(n):
+            x = 0
+            s = "hello world"
+            a, b = 1, 4
+            for _ in range(n):
+                v = s[a:b]
+                x += len(v)
+            return x
+
+        res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
+        self.assertEqual(res, TIER2_THRESHOLD * 3)
+        self.assertIsNotNone(ex)
+        uops = get_opnames(ex)
+        self.assertIn("_BINARY_SLICE", uops)
+        self.assertGreaterEqual(count_ops(ex, "_POP_TOP"), 3)
+
     def test_contains_op(self):
         def testfunc(n):
             x = 0

Include/internal/pycore_uop_ids.h

@@ -862,25 +862,29 @@ dummy_func(
             #endif  /* ENABLE_SPECIALIZATION */
         }
 
-        op(_BINARY_SLICE, (container, start, stop -- res)) {
-            PyObject *slice = _PyBuildSlice_ConsumeRefs(PyStackRef_AsPyObjectSteal(start),
-                                                        PyStackRef_AsPyObjectSteal(stop));
+        op(_BINARY_SLICE, (container, start, stop -- res, c, sta, sto)) {
+            PyObject *slice = PySlice_New(PyStackRef_AsPyObjectBorrow(start),
+                                         PyStackRef_AsPyObjectBorrow(stop),
+                                         NULL);
             PyObject *res_o;
-            // Can't use ERROR_IF() here, because we haven't
-            // DECREF'ed container yet, and we still own slice.
             if (slice == NULL) {
                 res_o = NULL;
             }
             else {
                 res_o = PyObject_GetItem(PyStackRef_AsPyObjectBorrow(container), slice);
                 Py_DECREF(slice);
             }
-            PyStackRef_CLOSE(container);
-            ERROR_IF(res_o == NULL);
+            if (res_o == NULL) {
+                ERROR_NO_POP();
+            }
             res = PyStackRef_FromPyObjectSteal(res_o);
+            c = container;
+            sta = start;
+            sto = stop;
+            INPUTS_DEAD();
         }
 
-        macro(BINARY_SLICE) = _SPECIALIZE_BINARY_SLICE + _BINARY_SLICE;
+        macro(BINARY_SLICE) = _SPECIALIZE_BINARY_SLICE + _BINARY_SLICE + POP_TOP + POP_TOP + POP_TOP;
 
         specializing op(_SPECIALIZE_STORE_SLICE, (v, container, start, stop -- v, container, start, stop)) {
             // Placeholder until we implement STORE_SLICE specialization

Include/internal/pycore_uop_metadata.h

@@ -1491,7 +1491,7 @@ dummy_func(void) {
         sym_set_const(callable, list_append);
     }
 
-    op(_BINARY_SLICE, (container, start, stop -- res)) {
+    op(_BINARY_SLICE, (container, start, stop -- res, c, sta, sto)) {
         // Slicing a string/list/tuple always returns the same type.
         PyTypeObject *type = sym_get_type(container);
         if (type == &PyUnicode_Type ||
@@ -1503,6 +1503,9 @@ dummy_func(void) {
         else {
             res = sym_new_not_null(ctx);
         }
+        c = container;
+        sta = start;
+        sto = stop;
     }
 
     op(_GUARD_GLOBALS_VERSION, (version/1 --)) {