diff --git a/docs/SETUP.md b/docs/SETUP.md index bf16bf2..de27469 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -88,7 +88,15 @@ patch, and diffing the result against the working tree (byte-identical). It adds after a branch/jump so GNU `as` can fill the slot. - `--nop-on-reg-read` (region token `maspsx=regread`) — extend the load-delay predicate (cookbook finding 27) so a following `jr`/`jalr` that *uses* the loaded register also gets the delay `nop`. -- **A default-on fix (no region token): maspsx now honours cc1's explicit `#nop` marker.** cc1 emits the +- **`nopmarker` (region token `maspsx=nopmarker`) — OPT-IN as of Phase 11.** cc1 emits an explicit + `#nop` marker when it thinks a load-delay nop is needed. For a bare-symbol store consumer + (`sw $2,D_801221C4`) the store's own `lui $at` expansion fills the delay slot, so the marker is + **spurious** and honouring it costs an instruction (worker C's `0x800AFDBC`: the original is + `lhu` / `lui at` / `sh` with NO nop). But `0x80107C5C` genuinely wants the nop (112 vs 108). + Phase 10 made the honouring unconditional and that was **too broad**; it is now per-region and + default OFF. Verified: `make check` green at 510 regions with it off, and `0x80107C5C` matches + 112 B with the token and comes out 108 B LENGTH-MISMATCH without it. +- **A default-on fix (superseded by the opt-in above): maspsx now honours cc1's explicit `#nop` marker.** cc1 emits the marker when it wants a load-delay nop; maspsx used to re-derive the need and could overrule cc1 for a **bare-symbol store** consumer (`sw $2,D_801221C4`), because `uses_at` is True for a macro store while `nop_at_expansion` is False for ASPSX >= 2.30, so neither test fired and the nop was dropped diff --git a/tools/patches/maspsx-phase10-r1r2.patch b/tools/patches/maspsx-phase10-r1r2.patch index 4204a89..cbbcc08 100644 --- a/tools/patches/maspsx-phase10-r1r2.patch +++ b/tools/patches/maspsx-phase10-r1r2.patch @@ -39,16 +39,15 @@ def line_loads_from_reg(line: str, r_source: str, loads_to_reg=False) -> bool: """ NOTE: Returns True even if line might use $at expansion -@@ -448,6 +480,8 @@ - gp_allow_la=False, - use_comm_section=False, +@@ -450,6 +482,7 @@ use_comm_for_lcomm=False, -+ no_jump_slot_nop=False, -+ nop_on_reg_read=False, + no_jump_slot_nop=False, + nop_on_reg_read=False, ++ honour_nop_marker=False, ): self.lines = [x.strip() for x in lines] -@@ -460,6 +494,15 @@ +@@ -462,6 +495,18 @@ self.nop_mflo_mfhi = nop_mflo_mfhi self.nop_lw_lw = nop_lw_lw @@ -60,11 +59,14 @@ + # the loaded register as needing the delay nop (worker C's R2). + self.no_jump_slot_nop = no_jump_slot_nop + self.nop_on_reg_read = nop_on_reg_read ++ # Opt-in (Phase 11 correction): honour cc1's explicit `#nop` marker even when ++ # the following instruction's own macro expansion would fill the delay slot. ++ self.honour_nop_marker = honour_nop_marker + self.sltu_at = sltu_at self.addiu_at = addiu_at self.div_uses_tge = div_uses_tge -@@ -696,9 +739,30 @@ +@@ -698,9 +743,34 @@ ) -> List[str]: res: List[str] = [] @@ -79,24 +81,28 @@ + if reuse: nop_required = False -+ # Phase 10 (developer-authorised): cc1 emits an explicit `#nop` marker -+ # when it wants a load-delay nop. maspsx used to re-derive the need and -+ # could overrule cc1: for a BARE-SYMBOL store consumer (`sw $2,D_801221C4`) -+ # `uses_at` is True (the store expands via $at) while `nop_at_expansion` -+ # is False for ASPSX >= 2.30, so neither test below fired and the nop was -+ # dropped -- losing an instruction cc1 had asked for (0x80107C5C, -+ # 0x8003A9C8). Honour the explicit marker instead of overruling it. -+ # Only the marker changes behaviour; every other path is untouched. -+ if self.get_next_instruction( ++ # Phase 10/11: cc1 emits an explicit `#nop` marker when it thinks a ++ # load-delay nop is needed. For a BARE-SYMBOL store consumer ++ # (`sw $2,D_801221C4`) `uses_at` is True (the store expands via $at) while ++ # `nop_at_expansion` is False for ASPSX >= 2.30, so neither test below ++ # fires and the marker is ignored -- which is the correct default, because ++ # the store's own `lui $at` expansion fills the delay slot (worker C's ++ # 0x800AFDBC: the original is `lhu` / `lui at` / `sh` with NO nop, and ++ # honouring the marker costs 2 instructions there). ++ # ++ # Phase 10 made this unconditional and it was TOO BROAD: 0x80107C5C DOES ++ # want the nop (108 vs 112). So it is opt-in per region now ++ # (`maspsx=nopmarker`). Default off; no registered region depends on ++ # either behaviour (the gate is green at 510 regions both ways). ++ if self.honour_nop_marker and self.get_next_instruction( + skip=0, ignore_nop=False, ignore_set=True, ignore_label=True + ) == "#nop": + reason = "cc1 emitted an explicit '#nop' marker" + nop_required = True -+ + if not uses_at(next_instruction): reason = f"'{next_instruction}' does not use $at" - nop_required = True -@@ -1102,7 +1166,7 @@ +@@ -1105,7 +1175,7 @@ elif op in branch_mnemonics or op in jump_mnemonics: res.append(line) @@ -107,23 +113,25 @@ elif op == "move": --- a/maspsx.py +++ b/maspsx.py -@@ -62,6 +62,10 @@ +@@ -62,8 +62,11 @@ parser.add_argument("--passthrough", action="store_true") parser.add_argument("--use-comm-section", action="store_true") parser.add_argument("--use-comm-for-lcomm", action="store_true") + # Phase 10 local additions (developer-authorised, opt-in; see + # tools/patches/maspsx-phase10-r1r2.patch and docs/SETUP.md). -+ parser.add_argument("--no-jump-slot-nop", action="store_true") -+ parser.add_argument("--nop-on-reg-read", action="store_true") + parser.add_argument("--no-jump-slot-nop", action="store_true") + parser.add_argument("--nop-on-reg-read", action="store_true") ++ parser.add_argument("--honour-nop-marker", action="store_true") # decomp.me debugging parser.add_argument("--print-output", action="store_true") parser.add_argument("--print-input", action="store_true") -@@ -152,6 +156,8 @@ +@@ -154,6 +157,9 @@ gp_allow_la=version_config.gp_allow_la, use_comm_section=args.use_comm_section, use_comm_for_lcomm=args.use_comm_for_lcomm, + no_jump_slot_nop=args.no_jump_slot_nop, + nop_on_reg_read=args.nop_on_reg_read, ++ honour_nop_marker=args.honour_nop_marker, ) try: diff --git a/tools/sf3_match b/tools/sf3_match index 4ef59b2..509426e 100755 --- a/tools/sf3_match +++ b/tools/sf3_match @@ -216,7 +216,8 @@ _REGION_OPTION_KEYS = ("cc1", "as", "gp", "maspsx") # `move`->`addu` rewrite and NO maspsx stage, letting `as` fill exactly the slots cc1 # left empty while leaving cc1's own `.set noreorder` windows alone. _MASPSX_MODES = {"off": "--off", "noreordernop": "--no-jump-slot-nop", - "regread": "--nop-on-reg-read", "moves": "moves"} + "regread": "--nop-on-reg-read", "moves": "moves", + "nopmarker": "--honour-nop-marker"} def parse_region_options(text: str, line_number: int) -> tuple[tuple[str, ...], tuple[str, ...], tuple[str, ...], bool, tuple[str, ...], bool]: @@ -852,6 +853,10 @@ def add_toolchain_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument("--no-jump-slot-nop", action="store_true", help="maspsx mode: suppress the unconditional reorder nop after a " "branch/jump so GNU as can fill the slot (region: maspsx=noreordernop)") + parser.add_argument("--honour-nop-marker", action="store_true", + help="maspsx mode: honour cc1's explicit `#nop` marker even when the " + "following macro expansion would fill the delay slot " + "(region token: maspsx=nopmarker)") parser.add_argument("--maspsx-moves", action="store_true", help="run the `move`->`addu` rewrite and NO maspsx stage, so GNU as " "reorder mode fills the slots (region token: maspsx=moves)") @@ -963,7 +968,8 @@ def resolve_toolchain(args: argparse.Namespace) -> Toolchain: maspsx_flags=tuple( flag for flag, enabled in ( ("--no-jump-slot-nop", getattr(args, "no_jump_slot_nop", False)), - ("--nop-on-reg-read", getattr(args, "nop_on_reg_read", False))) if enabled), + ("--nop-on-reg-read", getattr(args, "nop_on_reg_read", False)), + ("--honour-nop-marker", getattr(args, "honour_nop_marker", False))) if enabled), aspsx_version=args.aspsx_version, )