From a7b842faf543424a55bcf884fa7170dbdbac3ddf Mon Sep 17 00:00:00 2001 From: Christopher Williams Date: Thu, 24 Sep 2026 08:18:37 -0400 Subject: [PATCH] =?UTF-8?q?phase10:=20maspsx=20fix=20=E2=80=94=20honour=20?= =?UTF-8?q?cc1's=20explicit=20#nop=20marker=20(developer-authorised)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: uses_at('sw\t$2,D_801221C4') -> True (macro store, expands via $at) uses_at('sw\t$2,0($4)') -> False (register+offset, no macro) nop_at_expansion is False for ASPSX >= 2.30 so neither test in _handle_nop_before_next_instruction fired, nop_required stayed False, and an instruction cc1 had explicitly asked for was dropped (0x80107C5C at 108 vs 112; worker B's 0x8003A9C8). The fix honours the marker instead of overruling it; only that path changes. Worker B2 found the gap but mis-diagnosed it: its proposed fix was to extend the `line_loads_from_reg` predicate, which ALREADY returns True for a store source, so that patch would have been a no-op. The coordinator traced the actual call and found the real mechanism in the uses_at / nop_at_expansion interaction. B2 then appended a CORRECTION row to its own staged report superseding its paragraph — the right response, and it records the general lesson: a named mechanism is a hypothesis until it is traced, even when the observation is solid and reproducible. REGRESSION VERIFICATION (the whole point of gating this): make check exit 0 regions=489 disagreements=0 AGREE c_regions=489 differing_bytes=0 MATCH 237 tests OK All 489 previously-matched regions are byte-identical with the fix in place. Carried as a tracked patch (tools/maspsx/ is git-ignored, so an in-place edit would not survive a fresh clone); patch verified to reproduce both modified files exactly from the pristine pinned checkout. docs/SETUP.md records the fix and its provenance. 0x80107C5C NOW MATCHES (112 B, 0 differing, verified against worker B2's variant X3.c) — but that is a BARE variant with no header, and the project convention requires a documented source stating the observed instructions and limits. So the row is UNBLOCKED and one documented source away, not claimed. Recorded as a carry-forward. --- docs/SETUP.md | 8 ++++++++ tools/patches/maspsx-phase10-r1r2.patch | 20 ++++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/docs/SETUP.md b/docs/SETUP.md index f28f58e..bf16bf2 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -88,6 +88,14 @@ 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 + 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 + (`0x80107C5C`, `0x8003A9C8`). The fix honours the marker instead of overruling it. **Verified + regression-free: `make check` green at 489 regions / 237 tests with all 489 regions byte-identical.** + Worker B2 found the gap; the coordinator traced the mechanism after B2's first diagnosis (extend the + predicate) proved to be a no-op — the predicate already returned True. Both default **off**, and `make check` is green at 439 regions with them off, so the matched corpus is byte-identical. **Status: implemented, but neither mode has been shown to close a region.** See diff --git a/tools/patches/maspsx-phase10-r1r2.patch b/tools/patches/maspsx-phase10-r1r2.patch index 5301b56..4204a89 100644 --- a/tools/patches/maspsx-phase10-r1r2.patch +++ b/tools/patches/maspsx-phase10-r1r2.patch @@ -64,7 +64,7 @@ self.sltu_at = sltu_at self.addiu_at = addiu_at self.div_uses_tge = div_uses_tge -@@ -696,7 +739,14 @@ +@@ -696,9 +739,30 @@ ) -> List[str]: res: List[str] = [] @@ -79,8 +79,24 @@ + 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( ++ 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): -@@ -1102,7 +1152,7 @@ + reason = f"'{next_instruction}' does not use $at" + nop_required = True +@@ -1102,7 +1166,7 @@ elif op in branch_mnemonics or op in jump_mnemonics: res.append(line)