gh-98831: Support conditional effects; use for LOAD_ATTR

Python/bytecodes.c

@@ -51,7 +51,7 @@
 
 // Dummy variables for stack effects.
 static PyObject *value, *value1, *value2, *left, *right, *res, *sum, *prod, *sub;
-static PyObject *container, *start, *stop, *v, *lhs, *rhs;
+static PyObject *container, *start, *stop, *v, *lhs, *rhs, *res2;
 static PyObject *list, *tuple, *dict, *owner, *set, *str, *tup, *map, *keys;
 static PyObject *exit_func, *lasti, *val, *retval, *obj, *iter;
 static PyObject *aiter, *awaitable, *iterable, *w, *exc_value, *bc;
@@ -1438,13 +1438,11 @@ dummy_func(
             PREDICT(JUMP_BACKWARD);
         }
 
-        // error: LOAD_ATTR has irregular stack effect
-        inst(LOAD_ATTR) {
+        inst(LOAD_ATTR, (unused/9, owner -- res, res2 if (oparg & 1))) {
             #if ENABLE_SPECIALIZATION
             _PyAttrCache *cache = (_PyAttrCache *)next_instr;
             if (ADAPTIVE_COUNTER_IS_ZERO(cache->counter)) {
                 assert(cframe.use_tracing == 0);
-                PyObject *owner = TOP();
                 PyObject *name = GETITEM(names, oparg>>1);
                 next_instr--;
                 _Py_Specialize_LoadAttr(owner, next_instr, name);
@@ -1454,26 +1452,18 @@ dummy_func(
             DECREMENT_ADAPTIVE_COUNTER(cache->counter);
             #endif  /* ENABLE_SPECIALIZATION */
             PyObject *name = GETITEM(names, oparg >> 1);
-            PyObject *owner = TOP();
             if (oparg & 1) {
-                /* Designed to work in tandem with CALL. */
+                /* Designed to work in tandem with CALL, pushes two values. */
                 PyObject* meth = NULL;
-
-                int meth_found = _PyObject_GetMethod(owner, name, &meth);
-
-                if (meth == NULL) {
-                    /* Most likely attribute wasn't found. */
-                    goto error;
-                }
-
-                if (meth_found) {
+                if (_PyObject_GetMethod(owner, name, &meth)) {
                     /* We can bypass temporary bound method object.
                        meth is unbound method and obj is self.
 
                        meth | self | arg1 | ... | argN
                      */
-                    SET_TOP(meth);
-                    PUSH(owner);  // self
+                    assert(meth != NULL);  // No errors on this branch
+                    res = meth;
+                    res2 = owner;  // Transfer ownership
                 }
                 else {
                     /* meth is not an unbound method (but a regular attr, or
@@ -1483,20 +1473,18 @@ dummy_func(
 
                        NULL | meth | arg1 | ... | argN
                     */
-                    SET_TOP(NULL);
                     Py_DECREF(owner);
-                    PUSH(meth);
+                    ERROR_IF(meth == NULL, error);
+                    res = NULL;
+                    res2 = meth;
                 }
             }
             else {
-                PyObject *res = PyObject_GetAttr(owner, name);
-                if (res == NULL) {
-                    goto error;
-                }
+                /* Classic, pushes one value. */
+                res = PyObject_GetAttr(owner, name);
                 Py_DECREF(owner);
-                SET_TOP(res);
+                ERROR_IF(res == NULL, error);
             }
-            JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR);
         }
 
         // error: LOAD_ATTR has irregular stack effect

Python/opcode_metadata.h

@@ -185,7 +185,7 @@ _PyOpcode_num_popped(int opcode, int oparg) {
         case MAP_ADD:
             return 2;
         case LOAD_ATTR:
-            return -1;
+            return 1;
         case LOAD_ATTR_INSTANCE_VALUE:
             return -1;
         case LOAD_ATTR_MODULE:
@@ -531,7 +531,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) {
         case MAP_ADD:
             return 0;
         case LOAD_ATTR:
-            return -1;
+            return ((oparg & 1) != 0) + 1;
         case LOAD_ATTR_INSTANCE_VALUE:
             return -1;
         case LOAD_ATTR_MODULE:
@@ -694,7 +694,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) {
 }
 #endif
 enum Direction { DIR_NONE, DIR_READ, DIR_WRITE };
-enum InstructionFormat { INSTR_FMT_IB, INSTR_FMT_IBC, INSTR_FMT_IBC0, INSTR_FMT_IBC000, INSTR_FMT_IBIB, INSTR_FMT_IX, INSTR_FMT_IXC, INSTR_FMT_IXC000 };
+enum InstructionFormat { INSTR_FMT_IB, INSTR_FMT_IBC, INSTR_FMT_IBC0, INSTR_FMT_IBC000, INSTR_FMT_IBC00000000, INSTR_FMT_IBIB, INSTR_FMT_IX, INSTR_FMT_IXC, INSTR_FMT_IXC000 };
 struct opcode_metadata {
     enum Direction dir_op1;
     enum Direction dir_op2;
@@ -791,7 +791,7 @@ struct opcode_metadata {
     [DICT_UPDATE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
     [DICT_MERGE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
     [MAP_ADD] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
-    [LOAD_ATTR] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
+    [LOAD_ATTR] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IBC00000000 },
     [LOAD_ATTR_INSTANCE_VALUE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
     [LOAD_ATTR_MODULE] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },
     [LOAD_ATTR_WITH_HINT] = { DIR_NONE, DIR_NONE, DIR_NONE, true, INSTR_FMT_IB },

Tools/cases_generator/generate_cases.py

@@ -59,7 +59,10 @@ def effect_size(effect: StackEffect) -> tuple[int, str]:
     At most one of these will be non-zero / non-empty.
     """
     if effect.size:
+        assert not effect.cond, "Manual effects should be conditional"
         return 0, effect.size
+    elif effect.cond:
+        return 0, f"{maybe_parenthesize(effect.cond)} != 0"
     else:
         return 1, ""
 
@@ -163,6 +166,11 @@ def assign(self, dst: StackEffect, src: StackEffect):
         cast = self.cast(dst, src)
         if m := re.match(r"^PEEK\((.*)\)$", dst.name):
             self.emit(f"POKE({m.group(1)}, {cast}{src.name});")
+        elif m := re.match(r"^POP\(\)$", dst.name):
+            if src.cond:
+                self.emit(f"if ({src.cond}) {{ PUSH({cast}{src.name}); }}")
+            else:
+                self.emit(f"PUSH({cast}{src.name});")
         elif m := re.match(r"^&PEEK\(.*\)$", dst.name):
             # NOTE: MOVE_ITEMS() does not actually exist.
             # The only supported output array forms are:
@@ -220,10 +228,23 @@ def __init__(self, inst: parser.InstDef):
             effect for effect in inst.inputs if isinstance(effect, parser.CacheEffect)
         ]
         self.cache_offset = sum(c.size for c in self.cache_effects)
+
         self.input_effects = [
             effect for effect in inst.inputs if isinstance(effect, StackEffect)
         ]
+        manual = False
+        for effect in self.input_effects:
+            if effect.cond:
+                manual = True
+            effect.manual = manual
+
         self.output_effects = inst.outputs  # For consistency/completeness
+        manual = False
+        for effect in self.output_effects:
+            if effect.cond:
+                manual = True
+            effect.manual = manual
+
         unmoved_names: set[str] = set()
         for ieffect, oeffect in zip(self.input_effects, self.output_effects):
             if ieffect.name == oeffect.name:
@@ -276,15 +297,26 @@ def write(self, out: Formatter) -> None:
             # Write input stack effect variable declarations and initializations
             ieffects = list(reversed(self.input_effects))
             for i, ieffect in enumerate(ieffects):
-                isize = string_effect_size(list_effect_size(ieffects[:i+1]))
+                isize = string_effect_size(list_effect_size([ieff for ieff in ieffects[:i+1] if not ieff.manual]))
                 if ieffect.size:
+                    assert not ieffect.manual, "Manual array effects not yet supported"
                     src = StackEffect(f"&PEEK({isize})", "PyObject **")
                 else:
-                    src = StackEffect(f"PEEK({isize})", "")
+                    if ieffect.manual:
+                        if ieffect.cond:
+                            src = StackEffect(f"({ieffect.cond}) ? POP() : NULL", "")            
+                        else:
+                            src = StackEffect(f"POP()", "")            
+                    else:
+                        src = StackEffect(f"PEEK({isize})", "")            
                 out.declare(ieffect, src)
+                # if ieffect.cond:
+                #     assert ieffect.manual, "Conditional effects must be manual"
+                #     out.emit(f"if ({ieffect.cond}) {{ {ieffect.name} = POP(); }}")
         else:
             # Write input register variable declarations and initializations
             for ieffect, reg in zip(self.input_effects, self.input_registers):
+                assert not ieffect.manual, "Manual register effects not yet supported"
                 src = StackEffect(reg, "")
                 out.declare(ieffect, src)
 
@@ -304,22 +336,33 @@ def write(self, out: Formatter) -> None:
 
         if not self.register:
             # Write net stack growth/shrinkage
-            out.stack_adjust(0, self.input_effects, self.output_effects)
+            out.stack_adjust(0,
+                [ieff for ieff in self.input_effects if not ieff.manual],
+                [oeff for oeff in self.output_effects if not oeff.manual])
 
             # Write output stack effect assignments
             oeffects = list(reversed(self.output_effects))
+            real_oeffects: list[tuple[StackEffect, StackEffect]] = []
             for i, oeffect in enumerate(oeffects):
                 if oeffect.name in self.unmoved_names:
                     continue
-                osize = string_effect_size(list_effect_size(oeffects[:i+1]))
+                osize = string_effect_size(list_effect_size([oeff for oeff in oeffects[:i+1] if not oeff.manual]))
                 if oeffect.size:
+                    assert not oeffect.manual, "Manual array effects not yet supported"
                     dst = StackEffect(f"&PEEK({osize})", "PyObject **")
                 else:
-                    dst = StackEffect(f"PEEK({osize})", "")
+                    if oeffect.manual:
+                        dst = StackEffect(f"POP()", "")
+                    else:
+                        dst = StackEffect(f"PEEK({osize})", "")
+                real_oeffects.append((dst, oeffect))
+            for dst, oeffect in reversed(real_oeffects):
                 out.assign(dst, oeffect)
         else:
             # Write output register assignments
             for oeffect, reg in zip(self.output_effects, self.output_registers):
+                assert not oeffect.manual, "Manual register effects not yet supported"
+                src = StackEffect(reg, "")
                 dst = StackEffect(reg, "")
                 out.assign(dst, oeffect)
 
@@ -738,10 +781,10 @@ def get_stack_effect_info(
         self, thing: parser.InstDef | parser.Super | parser.Macro
     ) -> tuple[Instruction, str, str]:
 
-        def effect_str(effect: list[StackEffect]) -> str:
+        def effect_str(effects: list[StackEffect]) -> str:
             if getattr(thing, 'kind', None) == 'legacy':
                 return str(-1)
-            n_effect, sym_effect = list_effect_size(effect)
+            n_effect, sym_effect = list_effect_size(effects)
             if sym_effect:
                 return f"{sym_effect} + {n_effect}" if n_effect else sym_effect
             return str(n_effect)