[Python-checkins] gh-98831: Finish the UNPACK_SEQUENCE family (#101666)

gvanrossum webhook-mailer at python.org
Tue Feb 7 18:44:43 EST 2023


https://github.com/python/cpython/commit/aacbdb0c650492756738b044e6ddf8b72f89a1a2
commit: aacbdb0c650492756738b044e6ddf8b72f89a1a2
branch: main
author: Guido van Rossum <guido at python.org>
committer: gvanrossum <gvanrossum at gmail.com>
date: 2023-02-07T15:44:37-08:00
summary:

gh-98831: Finish the UNPACK_SEQUENCE family (#101666)

New generator feature: Generate useful glue for output arrays, so you can just write values to the output array (no bounds checking). Rewrote UNPACK_SEQUENCE_TWO_TUPLE to use this, and also UNPACK_SEQUENCE_{TUPLE,LIST}.

files:
M Python/bytecodes.c
M Python/generated_cases.c.h
M Python/opcode_metadata.h
M Tools/cases_generator/generate_cases.py
M Tools/cases_generator/test_generator.py

diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index 0d7d922816ce..c6c00a7ab9b0 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -876,6 +876,13 @@ dummy_func(
             }
         }
 
+        family(unpack_sequence, INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE) = {
+            UNPACK_SEQUENCE,
+            UNPACK_SEQUENCE_TWO_TUPLE,
+            UNPACK_SEQUENCE_TUPLE,
+            UNPACK_SEQUENCE_LIST,
+        };
+
         inst(UNPACK_SEQUENCE, (unused/1, seq -- unused[oparg])) {
             #if ENABLE_SPECIALIZATION
             _PyUnpackSequenceCache *cache = (_PyUnpackSequenceCache *)next_instr;
@@ -894,43 +901,36 @@ dummy_func(
             ERROR_IF(res == 0, error);
         }
 
-        inst(UNPACK_SEQUENCE_TWO_TUPLE, (unused/1, seq -- v1, v0)) {
+        inst(UNPACK_SEQUENCE_TWO_TUPLE, (unused/1, seq -- values[oparg])) {
             DEOPT_IF(!PyTuple_CheckExact(seq), UNPACK_SEQUENCE);
             DEOPT_IF(PyTuple_GET_SIZE(seq) != 2, UNPACK_SEQUENCE);
+            assert(oparg == 2);
             STAT_INC(UNPACK_SEQUENCE, hit);
-            v1 = Py_NewRef(PyTuple_GET_ITEM(seq, 1));
-            v0 = Py_NewRef(PyTuple_GET_ITEM(seq, 0));
+            values[0] = Py_NewRef(PyTuple_GET_ITEM(seq, 1));
+            values[1] = Py_NewRef(PyTuple_GET_ITEM(seq, 0));
             Py_DECREF(seq);
         }
 
-        // stack effect: (__0 -- __array[oparg])
-        inst(UNPACK_SEQUENCE_TUPLE) {
-            PyObject *seq = TOP();
+        inst(UNPACK_SEQUENCE_TUPLE, (unused/1, seq -- values[oparg])) {
             DEOPT_IF(!PyTuple_CheckExact(seq), UNPACK_SEQUENCE);
             DEOPT_IF(PyTuple_GET_SIZE(seq) != oparg, UNPACK_SEQUENCE);
             STAT_INC(UNPACK_SEQUENCE, hit);
-            STACK_SHRINK(1);
             PyObject **items = _PyTuple_ITEMS(seq);
-            while (oparg--) {
-                PUSH(Py_NewRef(items[oparg]));
+            for (int i = oparg; --i >= 0; ) {
+                *values++ = Py_NewRef(items[i]);
             }
             Py_DECREF(seq);
-            JUMPBY(INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE);
         }
 
-        // stack effect: (__0 -- __array[oparg])
-        inst(UNPACK_SEQUENCE_LIST) {
-            PyObject *seq = TOP();
+        inst(UNPACK_SEQUENCE_LIST, (unused/1, seq -- values[oparg])) {
             DEOPT_IF(!PyList_CheckExact(seq), UNPACK_SEQUENCE);
             DEOPT_IF(PyList_GET_SIZE(seq) != oparg, UNPACK_SEQUENCE);
             STAT_INC(UNPACK_SEQUENCE, hit);
-            STACK_SHRINK(1);
             PyObject **items = _PyList_ITEMS(seq);
-            while (oparg--) {
-                PUSH(Py_NewRef(items[oparg]));
+            for (int i = oparg; --i >= 0; ) {
+                *values++ = Py_NewRef(items[i]);
             }
             Py_DECREF(seq);
-            JUMPBY(INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE);
         }
 
         inst(UNPACK_EX, (seq -- unused[oparg & 0xFF], unused, unused[oparg >> 8])) {
@@ -3185,6 +3185,3 @@ family(call, INLINE_CACHE_ENTRIES_CALL) = {
     CALL_NO_KW_METHOD_DESCRIPTOR_O, CALL_NO_KW_STR_1, CALL_NO_KW_TUPLE_1,
     CALL_NO_KW_TYPE_1 };
 family(store_fast) = { STORE_FAST, STORE_FAST__LOAD_FAST, STORE_FAST__STORE_FAST };
-family(unpack_sequence, INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE) = {
-    UNPACK_SEQUENCE, UNPACK_SEQUENCE_LIST,
-    UNPACK_SEQUENCE_TUPLE, UNPACK_SEQUENCE_TWO_TUPLE };
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index de98b1a4f2ed..ded68d011c6b 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -1125,6 +1125,7 @@
 
         TARGET(UNPACK_SEQUENCE) {
             PREDICTED(UNPACK_SEQUENCE);
+            static_assert(INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE == 1, "incorrect cache size");
             PyObject *seq = PEEK(1);
             #if ENABLE_SPECIALIZATION
             _PyUnpackSequenceCache *cache = (_PyUnpackSequenceCache *)next_instr;
@@ -1149,48 +1150,51 @@
 
         TARGET(UNPACK_SEQUENCE_TWO_TUPLE) {
             PyObject *seq = PEEK(1);
-            PyObject *v1;
-            PyObject *v0;
+            PyObject **values = stack_pointer - (1);
             DEOPT_IF(!PyTuple_CheckExact(seq), UNPACK_SEQUENCE);
             DEOPT_IF(PyTuple_GET_SIZE(seq) != 2, UNPACK_SEQUENCE);
+            assert(oparg == 2);
             STAT_INC(UNPACK_SEQUENCE, hit);
-            v1 = Py_NewRef(PyTuple_GET_ITEM(seq, 1));
-            v0 = Py_NewRef(PyTuple_GET_ITEM(seq, 0));
+            values[0] = Py_NewRef(PyTuple_GET_ITEM(seq, 1));
+            values[1] = Py_NewRef(PyTuple_GET_ITEM(seq, 0));
             Py_DECREF(seq);
-            STACK_GROW(1);
-            POKE(1, v0);
-            POKE(2, v1);
+            STACK_SHRINK(1);
+            STACK_GROW(oparg);
             JUMPBY(1);
             DISPATCH();
         }
 
         TARGET(UNPACK_SEQUENCE_TUPLE) {
-            PyObject *seq = TOP();
+            PyObject *seq = PEEK(1);
+            PyObject **values = stack_pointer - (1);
             DEOPT_IF(!PyTuple_CheckExact(seq), UNPACK_SEQUENCE);
             DEOPT_IF(PyTuple_GET_SIZE(seq) != oparg, UNPACK_SEQUENCE);
             STAT_INC(UNPACK_SEQUENCE, hit);
-            STACK_SHRINK(1);
             PyObject **items = _PyTuple_ITEMS(seq);
-            while (oparg--) {
-                PUSH(Py_NewRef(items[oparg]));
+            for (int i = oparg; --i >= 0; ) {
+                *values++ = Py_NewRef(items[i]);
             }
             Py_DECREF(seq);
-            JUMPBY(INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE);
+            STACK_SHRINK(1);
+            STACK_GROW(oparg);
+            JUMPBY(1);
             DISPATCH();
         }
 
         TARGET(UNPACK_SEQUENCE_LIST) {
-            PyObject *seq = TOP();
+            PyObject *seq = PEEK(1);
+            PyObject **values = stack_pointer - (1);
             DEOPT_IF(!PyList_CheckExact(seq), UNPACK_SEQUENCE);
             DEOPT_IF(PyList_GET_SIZE(seq) != oparg, UNPACK_SEQUENCE);
             STAT_INC(UNPACK_SEQUENCE, hit);
-            STACK_SHRINK(1);
             PyObject **items = _PyList_ITEMS(seq);
-            while (oparg--) {
-                PUSH(Py_NewRef(items[oparg]));
+            for (int i = oparg; --i >= 0; ) {
+                *values++ = Py_NewRef(items[i]);
             }
             Py_DECREF(seq);
-            JUMPBY(INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE);
+            STACK_SHRINK(1);
+            STACK_GROW(oparg);
+            JUMPBY(1);
             DISPATCH();
         }
 
diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h
index bae5492c0496..c1e12a4bbede 100644
--- a/Python/opcode_metadata.h
+++ b/Python/opcode_metadata.h
@@ -127,9 +127,9 @@ _PyOpcode_num_popped(int opcode, int oparg, bool jump) {
         case UNPACK_SEQUENCE_TWO_TUPLE:
             return 1;
         case UNPACK_SEQUENCE_TUPLE:
-            return -1;
+            return 1;
         case UNPACK_SEQUENCE_LIST:
-            return -1;
+            return 1;
         case UNPACK_EX:
             return 1;
         case STORE_ATTR:
@@ -473,11 +473,11 @@ _PyOpcode_num_pushed(int opcode, int oparg, bool jump) {
         case UNPACK_SEQUENCE:
             return oparg;
         case UNPACK_SEQUENCE_TWO_TUPLE:
-            return 2;
+            return oparg;
         case UNPACK_SEQUENCE_TUPLE:
-            return -1;
+            return oparg;
         case UNPACK_SEQUENCE_LIST:
-            return -1;
+            return oparg;
         case UNPACK_EX:
             return (oparg & 0xFF) + (oparg >> 8) + 1;
         case STORE_ATTR:
@@ -765,9 +765,9 @@ struct opcode_metadata {
     [STORE_NAME] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
     [DELETE_NAME] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
     [UNPACK_SEQUENCE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IBC },
-    [UNPACK_SEQUENCE_TWO_TUPLE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IXC },
-    [UNPACK_SEQUENCE_TUPLE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
-    [UNPACK_SEQUENCE_LIST] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
+    [UNPACK_SEQUENCE_TWO_TUPLE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IBC },
+    [UNPACK_SEQUENCE_TUPLE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IBC },
+    [UNPACK_SEQUENCE_LIST] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IBC },
     [UNPACK_EX] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
     [STORE_ATTR] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IBC000 },
     [DELETE_ATTR] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py
index 3925583b40e7..4f94b48d114d 100644
--- a/Tools/cases_generator/generate_cases.py
+++ b/Tools/cases_generator/generate_cases.py
@@ -180,11 +180,8 @@ def assign(self, dst: StackEffect, src: StackEffect):
                 stmt = f"if ({src.cond}) {{ {stmt} }}"
             self.emit(stmt)
         elif m := re.match(r"^&PEEK\(.*\)$", dst.name):
-            # NOTE: MOVE_ITEMS() does not actually exist.
-            # The only supported output array forms are:
-            # - unused[...]
-            # - X[...] where X[...] matches an input array exactly
-            self.emit(f"MOVE_ITEMS({dst.name}, {src.name}, {src.size});")
+            # The user code is responsible for writing to the output array.
+            pass
         elif m := re.match(r"^REG\(oparg(\d+)\)$", dst.name):
             self.emit(f"Py_XSETREF({dst.name}, {cast}{src.name});")
         else:
@@ -309,10 +306,24 @@ def write(self, out: Formatter) -> None:
                 out.declare(ieffect, src)
 
         # Write output stack effect variable declarations
+        isize = string_effect_size(list_effect_size(self.input_effects))
         input_names = {ieffect.name for ieffect in self.input_effects}
-        for oeffect in self.output_effects:
+        for i, oeffect in enumerate(self.output_effects):
             if oeffect.name not in input_names:
-                out.declare(oeffect, None)
+                if oeffect.size:
+                    osize = string_effect_size(
+                        list_effect_size([oeff for oeff in self.output_effects[:i]])
+                    )
+                    offset = "stack_pointer"
+                    if isize != osize:
+                        if isize != "0":
+                            offset += f" - ({isize})"
+                        if osize != "0":
+                            offset += f" + {osize}"
+                    src = StackEffect(offset, "PyObject **")
+                    out.declare(oeffect, src)
+                else:
+                    out.declare(oeffect, None)
 
         # out.emit(f"JUMPBY(OPSIZE({self.inst.name}) - 1);")
 
diff --git a/Tools/cases_generator/test_generator.py b/Tools/cases_generator/test_generator.py
index 9df97d24ab6f..0c3d04b45dd9 100644
--- a/Tools/cases_generator/test_generator.py
+++ b/Tools/cases_generator/test_generator.py
@@ -424,20 +424,18 @@ def test_array_input():
 
 def test_array_output():
     input = """
-        inst(OP, (-- below, values[oparg*3], above)) {
-            spam();
+        inst(OP, (unused, unused -- below, values[oparg*3], above)) {
+            spam(values, oparg);
         }
     """
     output = """
         TARGET(OP) {
             PyObject *below;
-            PyObject **values;
+            PyObject **values = stack_pointer - (2) + 1;
             PyObject *above;
-            spam();
-            STACK_GROW(2);
+            spam(values, oparg);
             STACK_GROW(oparg*3);
             POKE(1, above);
-            MOVE_ITEMS(&PEEK(1 + oparg*3), values, oparg*3);
             POKE(2 + oparg*3, below);
             DISPATCH();
         }
@@ -446,18 +444,17 @@ def test_array_output():
 
 def test_array_input_output():
     input = """
-        inst(OP, (below, values[oparg] -- values[oparg], above)) {
-            spam();
+        inst(OP, (values[oparg] -- values[oparg], above)) {
+            spam(values, oparg);
         }
     """
     output = """
         TARGET(OP) {
             PyObject **values = &PEEK(oparg);
-            PyObject *below = PEEK(1 + oparg);
             PyObject *above;
-            spam();
+            spam(values, oparg);
+            STACK_GROW(1);
             POKE(1, above);
-            MOVE_ITEMS(&PEEK(1 + oparg), values, oparg);
             DISPATCH();
         }
     """



More information about the Python-checkins mailing list