GH-101369: Allow macros as family members (#101399)

Also check for instructions straddling families (this includes macro parts).

Author: gvanrossum

Tools/cases_generator/generate_cases.py

@@ -426,10 +426,12 @@ class Component:
 
     def write_body(self, out: Formatter, cache_adjust: int) -> None:
         with out.block(""):
+            input_names = {ieffect.name for _, ieffect in self.input_mapping}
             for var, ieffect in self.input_mapping:
                 out.declare(ieffect, var)
             for _, oeffect in self.output_mapping:
-                out.declare(oeffect, None)
+                if oeffect.name not in input_names:
+                    out.declare(oeffect, None)
 
             self.instr.write_body(out, dedent=-4, cache_adjust=cache_adjust)
 
@@ -565,10 +567,10 @@ def analyze(self) -> None:
         Raises SystemExit if there is an error.
         """
         self.find_predictions()
-        self.map_families()
-        self.check_families()
         self.analyze_register_instrs()
         self.analyze_supers_and_macros()
+        self.map_families()
+        self.check_families()
 
     def find_predictions(self) -> None:
         """Find the instructions that need PREDICTED() labels."""
@@ -587,11 +589,30 @@ def find_predictions(self) -> None:
                     )
 
     def map_families(self) -> None:
-        """Make instruction names back to their family, if they have one."""
+        """Link instruction names back to their family, if they have one."""
         for family in self.families.values():
             for member in family.members:
                 if member_instr := self.instrs.get(member):
-                    member_instr.family = family
+                    if member_instr.family not in (family, None):
+                        self.error(
+                            f"Instruction {member} is a member of multiple families "
+                            f"({member_instr.family.name}, {family.name}).",
+                            family
+                        )
+                    else:
+                        member_instr.family = family
+                elif member_macro := 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}",
@@ -608,32 +629,50 @@ def check_families(self) -> None:
         for family in self.families.values():
             if len(family.members) < 2:
                 self.error(f"Family {family.name!r} has insufficient members", family)
-            members = [member for member in family.members if member in self.instrs]
+            members = [member for member in family.members if member in self.instrs or member in self.macro_instrs]
             if members != family.members:
                 unknown = set(family.members) - set(members)
                 self.error(
                     f"Family {family.name!r} has unknown members: {unknown}", family
                 )
             if len(members) < 2:
                 continue
-            head = self.instrs[members[0]]
-            cache = head.cache_offset
-            input = len(head.input_effects)
-            output = len(head.output_effects)
+            expected_effects = self.effect_counts(members[0])
             for member in members[1:]:
-                instr = self.instrs[member]
-                c = instr.cache_offset
-                i = len(instr.input_effects)
-                o = len(instr.output_effects)
-                if (c, i, o) != (cache, input, output):
+                member_effects = self.effect_counts(member)
+                if member_effects != expected_effects:
                     self.error(
                         f"Family {family.name!r} has inconsistent "
-                        f"(cache, inputs, outputs) effects:\n"
-                        f"  {family.members[0]} = {(cache, input, output)}; "
-                        f"{member} = {(c, i, o)}",
+                        f"(cache, input, output) effects:\n"
+                        f"  {family.members[0]} = {expected_effects}; "
+                        f"{member} = {member_effects}",
                         family,
                     )
 
+    def effect_counts(self, name: str) -> tuple[int, int, int]:
+        if instr := self.instrs.get(name):
+            cache = instr.cache_offset
+            input = len(instr.input_effects)
+            output = len(instr.output_effects)
+        elif macro := self.macro_instrs.get(name):
+            cache, input, output = 0, 0, 0
+            for part in macro.parts:
+                if isinstance(part, Component):
+                    cache += part.instr.cache_offset
+                    # A component may pop what the previous component pushed,
+                    # so we offset the input/output counts by that.
+                    delta_i = len(part.instr.input_effects)
+                    delta_o = len(part.instr.output_effects)
+                    offset = min(delta_i, output)
+                    input += delta_i - offset
+                    output += delta_o - offset
+                else:
+                    assert isinstance(part, parser.CacheEffect), part
+                    cache += part.size
+        else:
+            assert False, f"Unknown instruction {name!r}"
+        return cache, input, output
+
     def analyze_register_instrs(self) -> None:
         for instr in self.instrs.values():
             if instr.register:

Tools/cases_generator/test_generator.py

@@ -339,46 +339,64 @@ def test_super_instruction():
 
 def test_macro_instruction():
     input = """
-        inst(OP1, (counter/1, arg1 -- interim)) {
-            interim = op1(arg1);
+        inst(OP1, (counter/1, left, right -- left, right)) {
+            op1(left, right);
         }
-        op(OP2, (extra/2, arg2, interim -- res)) {
-            res = op2(arg2, interim);
+        op(OP2, (extra/2, arg2, left, right -- res)) {
+            res = op2(arg2, left, right);
         }
         macro(OP) = OP1 + cache/2 + OP2;
+        inst(OP3, (unused/5, arg2, left, right -- res)) {
+            res = op3(arg2, left, right);
+        }
+        family(op, INLINE_CACHE_ENTRIES_OP) = { OP, OP3 };
     """
     output = """
         TARGET(OP1) {
-            PyObject *arg1 = PEEK(1);
-            PyObject *interim;
+            PyObject *right = PEEK(1);
+            PyObject *left = PEEK(2);
             uint16_t counter = read_u16(&next_instr[0].cache);
-            interim = op1(arg1);
-            POKE(1, interim);
+            op1(left, right);
             JUMPBY(1);
             DISPATCH();
         }
 
         TARGET(OP) {
             PyObject *_tmp_1 = PEEK(1);
             PyObject *_tmp_2 = PEEK(2);
+            PyObject *_tmp_3 = PEEK(3);
             {
-                PyObject *arg1 = _tmp_1;
-                PyObject *interim;
+                PyObject *right = _tmp_1;
+                PyObject *left = _tmp_2;
                 uint16_t counter = read_u16(&next_instr[0].cache);
-                interim = op1(arg1);
-                _tmp_1 = interim;
+                op1(left, right);
+                _tmp_2 = left;
+                _tmp_1 = right;
             }
             {
-                PyObject *interim = _tmp_1;
-                PyObject *arg2 = _tmp_2;
+                PyObject *right = _tmp_1;
+                PyObject *left = _tmp_2;
+                PyObject *arg2 = _tmp_3;
                 PyObject *res;
                 uint32_t extra = read_u32(&next_instr[3].cache);
-                res = op2(arg2, interim);
-                _tmp_2 = res;
+                res = op2(arg2, left, right);
+                _tmp_3 = res;
             }
             JUMPBY(5);
-            STACK_SHRINK(1);
-            POKE(1, _tmp_2);
+            STACK_SHRINK(2);
+            POKE(1, _tmp_3);
+            DISPATCH();
+        }
+
+        TARGET(OP3) {
+            PyObject *right = PEEK(1);
+            PyObject *left = PEEK(2);
+            PyObject *arg2 = PEEK(3);
+            PyObject *res;
+            res = op3(arg2, left, right);
+            STACK_SHRINK(2);
+            POKE(1, res);
+            JUMPBY(5);
             DISPATCH();
         }
     """