GH-104909: Split LOAD_ATTR_INSTANCE_VALUE into micro-ops (GH-106678)

Include/internal/pycore_opcode_metadata.h

@@ -39,10 +39,12 @@
 #define _SKIP_CACHE 317
 #define _GUARD_GLOBALS_VERSION 318
 #define _GUARD_BUILTINS_VERSION 319
-#define IS_NONE 320
+#define _GUARD_TYPE_VERSION 320
-#define _ITER_CHECK_RANGE 321
+#define _CHECK_MANAGED_OBJECT_HAS_VALUES 321
-#define _ITER_EXHAUSTED_RANGE 322
+#define IS_NONE 322
-#define _ITER_NEXT_RANGE 323
+#define _ITER_CHECK_RANGE 323
+#define _ITER_EXHAUSTED_RANGE 324
+#define _ITER_NEXT_RANGE 325
 
 #ifndef NEED_OPCODE_METADATA
 extern int _PyOpcode_num_popped(int opcode, int oparg, bool jump);
@@ -932,7 +934,7 @@ _PyOpcode_num_pushed(int opcode, int oparg, bool jump) {
 }
 #endif
 
-enum InstructionFormat { INSTR_FMT_IB, INSTR_FMT_IBC, INSTR_FMT_IBC00, INSTR_FMT_IBC000, INSTR_FMT_IBC00000000, INSTR_FMT_IX, INSTR_FMT_IXC, INSTR_FMT_IXC00, INSTR_FMT_IXC000 };
+enum InstructionFormat { INSTR_FMT_IB, INSTR_FMT_IBC, INSTR_FMT_IBC00, INSTR_FMT_IBC000, INSTR_FMT_IBC00000, INSTR_FMT_IBC00000000, INSTR_FMT_IX, INSTR_FMT_IXC, INSTR_FMT_IXC0, INSTR_FMT_IXC00, INSTR_FMT_IXC000 };
 #define HAS_ARG_FLAG (1)
 #define HAS_CONST_FLAG (2)
 #define HAS_NAME_FLAG (4)
@@ -1321,9 +1323,11 @@ const char * const _PyOpcode_uop_name[512] = {
     [317] = "_SKIP_CACHE",
     [318] = "_GUARD_GLOBALS_VERSION",
     [319] = "_GUARD_BUILTINS_VERSION",
-    [320] = "IS_NONE",
+    [320] = "_GUARD_TYPE_VERSION",
-    [321] = "_ITER_CHECK_RANGE",
+    [321] = "_CHECK_MANAGED_OBJECT_HAS_VALUES",
-    [322] = "_ITER_EXHAUSTED_RANGE",
+    [322] = "IS_NONE",
-    [323] = "_ITER_NEXT_RANGE",
+    [323] = "_ITER_CHECK_RANGE",
+    [324] = "_ITER_EXHAUSTED_RANGE",
+    [325] = "_ITER_NEXT_RANGE",
 };
 #endif // NEED_OPCODE_METADATA

Misc/NEWS.d/next/Core and Builtins/2023-07-12-11-18-55.gh-issue-104909.DRUsuh.rst

@@ -0,0 +1 @@
+Split :opcode:`LOAD_ATTR_INSTANCE_VALUE` into micro-ops.

Python/bytecodes.c

@@ -1816,14 +1816,21 @@ dummy_func(
             LOAD_ATTR,
         };
 
-        inst(LOAD_ATTR_INSTANCE_VALUE, (unused/1, type_version/2, index/1, unused/5, owner -- res2 if (oparg & 1), res)) {
+        op(_GUARD_TYPE_VERSION, (type_version/2, owner -- owner)) {
             PyTypeObject *tp = Py_TYPE(owner);
             assert(type_version != 0);
             DEOPT_IF(tp->tp_version_tag != type_version, LOAD_ATTR);
-            assert(tp->tp_dictoffset < 0);
+        }
-            assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT);
+
+        op(_CHECK_MANAGED_OBJECT_HAS_VALUES, (owner -- owner)) {
+            assert(Py_TYPE(owner)->tp_dictoffset < 0);
+            assert(Py_TYPE(owner)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
             PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(owner);
             DEOPT_IF(!_PyDictOrValues_IsValues(dorv), LOAD_ATTR);
+        }
+
+        op(_LOAD_ATTR_INSTANCE_VALUE, (index/1, unused/5, owner -- res2 if (oparg & 1), res)) {
+            PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(owner);
             res = _PyDictOrValues_GetValues(dorv)->values[index];
             DEOPT_IF(res == NULL, LOAD_ATTR);
             STAT_INC(LOAD_ATTR, hit);
@@ -1832,6 +1839,12 @@ dummy_func(
             DECREF_INPUTS();
         }
 
+        macro(LOAD_ATTR_INSTANCE_VALUE) =
+            _SKIP_CACHE + // Skip over the counter
+            _GUARD_TYPE_VERSION +
+            _CHECK_MANAGED_OBJECT_HAS_VALUES +
+            _LOAD_ATTR_INSTANCE_VALUE;
+
         inst(LOAD_ATTR_MODULE, (unused/1, type_version/2, index/1, unused/5, owner -- res2 if (oparg & 1), res)) {
             DEOPT_IF(!PyModule_CheckExact(owner), LOAD_ATTR);
             PyDictObject *dict = (PyDictObject *)((PyModuleObject *)owner)->md_dict;

Python/executor_cases.c.h

@@ -1425,6 +1425,24 @@
             break;
         }
 
+        case _GUARD_TYPE_VERSION: {
+            PyObject *owner = stack_pointer[-1];
+            uint32_t type_version = (uint32_t)operand;
+            PyTypeObject *tp = Py_TYPE(owner);
+            assert(type_version != 0);
+            DEOPT_IF(tp->tp_version_tag != type_version, LOAD_ATTR);
+            break;
+        }
+
+        case _CHECK_MANAGED_OBJECT_HAS_VALUES: {
+            PyObject *owner = stack_pointer[-1];
+            assert(Py_TYPE(owner)->tp_dictoffset < 0);
+            assert(Py_TYPE(owner)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
+            PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(owner);
+            DEOPT_IF(!_PyDictOrValues_IsValues(dorv), LOAD_ATTR);
+            break;
+        }
+
         case COMPARE_OP: {
             static_assert(INLINE_CACHE_ENTRIES_COMPARE_OP == 1, "incorrect cache size");
             PyObject *right = stack_pointer[-1];

Python/generated_cases.c.h

@@ -2240,28 +2240,45 @@
         }
 
         TARGET(LOAD_ATTR_INSTANCE_VALUE) {
-            PyObject *owner = stack_pointer[-1];
+            PyObject *_tmp_1;
-            PyObject *res2 = NULL;
+            PyObject *_tmp_2 = stack_pointer[-1];
-            PyObject *res;
+            {
-            uint32_t type_version = read_u32(&next_instr[1].cache);
+            }
-            uint16_t index = read_u16(&next_instr[3].cache);
+            {
-            PyTypeObject *tp = Py_TYPE(owner);
+                PyObject *owner = _tmp_2;
-            assert(type_version != 0);
+                uint32_t type_version = read_u32(&next_instr[1].cache);
-            DEOPT_IF(tp->tp_version_tag != type_version, LOAD_ATTR);
+                PyTypeObject *tp = Py_TYPE(owner);
-            assert(tp->tp_dictoffset < 0);
+                assert(type_version != 0);
-            assert(tp->tp_flags & Py_TPFLAGS_MANAGED_DICT);
+                DEOPT_IF(tp->tp_version_tag != type_version, LOAD_ATTR);
-            PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(owner);
+                _tmp_2 = owner;
-            DEOPT_IF(!_PyDictOrValues_IsValues(dorv), LOAD_ATTR);
+            }
-            res = _PyDictOrValues_GetValues(dorv)->values[index];
+            {
-            DEOPT_IF(res == NULL, LOAD_ATTR);
+                PyObject *owner = _tmp_2;
-            STAT_INC(LOAD_ATTR, hit);
+                assert(Py_TYPE(owner)->tp_dictoffset < 0);
-            Py_INCREF(res);
+                assert(Py_TYPE(owner)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
-            res2 = NULL;
+                PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(owner);
-            Py_DECREF(owner);
+                DEOPT_IF(!_PyDictOrValues_IsValues(dorv), LOAD_ATTR);
-            STACK_GROW(((oparg & 1) ? 1 : 0));
+                _tmp_2 = owner;
-            stack_pointer[-1] = res;
+            }
-            if (oparg & 1) { stack_pointer[-(1 + ((oparg & 1) ? 1 : 0))] = res2; }
+            {
+                PyObject *owner = _tmp_2;
+                PyObject *res2 = NULL;
+                PyObject *res;
+                uint16_t index = read_u16(&next_instr[3].cache);
+                PyDictOrValues dorv = *_PyObject_DictOrValuesPointer(owner);
+                res = _PyDictOrValues_GetValues(dorv)->values[index];
+                DEOPT_IF(res == NULL, LOAD_ATTR);
+                STAT_INC(LOAD_ATTR, hit);
+                Py_INCREF(res);
+                res2 = NULL;
+                Py_DECREF(owner);
+                if (oparg & 1) { _tmp_2 = res2; }
+                _tmp_1 = res;
+            }
             next_instr += 9;
+            STACK_GROW(((oparg & 1) ? 1 : 0));
+            stack_pointer[-1] = _tmp_1;
+            if (oparg & 1) { stack_pointer[-2] = _tmp_2; }
             DISPATCH();
         }
 

Tools/cases_generator/generate_cases.py

@@ -839,19 +839,7 @@ class Analyzer:
                         )
                     else:
                         member_instr.family = family
-                elif member_macro := self.macro_instrs.get(member):
+                elif not self.macro_instrs.get(member):
-                    for part in member_macro.parts:
-                        if isinstance(part, Component):
-                            if part.instr.family not in (family, None):
-                                self.error(
-                                    f"Component {part.instr.name} of macro {member} "
-                                    f"is a member of multiple families "
-                                    f"({part.instr.family.name}, {family.name}).",
-                                    family,
-                                )
-                            else:
-                                part.instr.family = family
-                else:
                     self.error(
                         f"Unknown instruction {member!r} referenced in family {family.name!r}",
                         family,