docs: stack slots follow reference weight, not declaration order

AGENTS.md

@@ -92,6 +92,12 @@ Provides project-specific utilities including:
 
 Imports and synchronizes Ghidra-exported data.
 
+### Compiler Internals (`tools/compiler-internals`)
+
+Notes on the bundled MSVC backend itself - what `c2.dll` does, how its symbols and source layout were recovered,
+the decoded stack-slot rule, and a `cdb` recipe for observing one function's allocation from inside the compiler.
+Read it when a diff is down to register or stack-slot allocation and no source-level hypothesis is paying.
+
 ### Reimplementation Control (`tools/reimplementation-control`)
 
 Scripts for enabling implementations and maintaining project state.
@@ -169,7 +175,10 @@ restyling anything large:
 flags a `sub esp` frame-size mismatch (`FRAME`), byte-sized stack slots we have and the original does not (`BYTE`), a
 cold return block we inline where the original outlines it (`RET`), and the case where only register and stack-slot
 *naming* is left (`alloc-only`). Chase a `FRAME` flag with `diff_slots.py NAME`, which lists every `[esp + N]` slot per
-side: one extra slot is one local the original does not have, and removing it has been worth 10-30 points.
+side: one extra slot is often one local the original does not have, and removing it has been worth 10-30 points -
+but locals with disjoint live ranges share a slot, so a slot count is not a local count. `slot_weight.py NAME.cpp`
+reads MSVC's own `_name$ = N` table out of a `/FA` listing and says which local actually got which slot, instead
+of inferring it from the diff.
 `diff_jcc.py` finds comparisons whose operator is one strictness step off the original (`jl`/`jle`, `jg`/`jge`) - these
 are real off-by-one bugs where Ghidra decompiled the condition wrongly, so confirm a fix by re-running that tool rather
 than by the percentage. `diff_types.py` is its counterpart away from comparisons: `movsx` against `movzx` for one field
@@ -179,7 +188,11 @@ sign. It carries only those two rules on purpose: a third, "same instruction wit
 dropped because every hit it produced was a loop-induction stride rather than a constant the source chose.
 `reorder_search.py FILE NAME` hill-climbs the match % by reordering independent statements,
 which is the only lever left once a function is `alloc-only`; it pays about one move in ten, and mostly where the
-statements sit between two calls or on a loop back-edge.
+statements sit between two calls or on a loop back-edge. `alloc-only` is worth reading literally: a register
+mismatch is most likely an interference difference - a different set of values was live at that point - rather
+than a preference the source could state. That is a hypothesis, not a decoded rule;
+`tools/compiler-internals/msvc-backend.md` has what is actually established about the compiler's allocator and
+the debugger recipe that can settle it per function.
 
 ### Binary Comparison (`reccmp`)
 
@@ -205,6 +218,12 @@ nothing getting worse. It is also the missing precondition for `cmake/compiler-f
 does nothing while the callee is still proxied. `tools/reimplementation-control/Enable-Reimplemented-Functions.ps1`
 flips them; like the struct resolvers they are never committed. Only enable a callee that is itself finished.
 
+`structural_diff.py --all` splits the build list by *what kind* of difference is left - wrong instruction mix
+(`mix`), right mix in the wrong order (`order`), or register naming only (`allocation`). Check it before picking
+a target: the reccmp percentage is dominated by allocation, so it will rank a structurally wrong function above a
+structurally perfect one. On `AI::AICState` it puts `aiGiveRaidInstructions` (delta 21) at the top while marking
+`sellExcessGoods` - a 62.4% function - as having nothing structural left to fix.
+
 ## Working on Many Functions
 
 When restyling or improving a large set of functions, work in batches (e.g. per folder) and verify each batch:
@@ -326,7 +345,10 @@ Diff patterns that were reliable (more in the cheat sheet):
 - `diff_triage.py`'s `FRAME` column is worth checking but, unlike `BYTE`, is not mechanically actionable. Every
   cheap hypothesis for the frame mismatches in `Map::Units::UnitsState` came back an exact tie or worse: local
   declaration order, array element type, widening unsigned locals, and splitting a local the decompiler had reused
-  for two values. `diff_slots.py` says the lowest-access extra slot is usually the culprit, so a function whose
+  for two values. Declaration order was never going to work: the stack colorer sorts its elements by **reference
+  weight per byte**, not by declaration order, and leaves only the ties to the order the symbols happen to arrive
+  in - see `tools/compiler-internals/msvc-backend.md` for the comparator and the evidence. The lever on a slot layout is
+  how often, and at what loop depth, each local is read. `diff_slots.py` says the lowest-access extra slot is usually the culprit, so a function whose
   extra slots are all heavily used has genuine live values rather than a stray local, and needs structural
   understanding rather than a rule.
 - `jmp dword ptr [reg*4 + table]` on one side only is a dispatch-form mismatch: a `switch` over contiguous values

IMPLEMENTATION_CHEAT_SHEET.md

@@ -111,11 +111,33 @@ matched instruction for instruction except that the parameter was loaded into `e
 for the parameter load and took it from 95.7% to 100%. Uninitialised declarations do not have this effect - a
 sweep of `quick_reorder.py -s` over the declaration block of 27 functions found no win at all.
 
-Stack *slot* assignment for spilled locals is a different matter and is not reachable from the source. Where the
-original lays its locals out in Ghidra order (`local_1c` at the lowest `esp` offset through `local_4` at the
-highest) and our build permutes them, neither declaration order, initialisation order, nor renaming changes
-anything; this was tried on `setupSkirmishLobby`, `initMultiplayerLobbyState`, `initializeFinalResultsForActivePlayers`
-and `createCompoundAddressBasedOnSelectedProvider`. Spend the effort elsewhere.
+Stack *slot* assignment for spilled locals is a different matter: it follows the **reference weight** of each
+local, not the order they are declared in. Neither declaration order, initialisation order, nor renaming moves a
+slot - that was observed on `setupSkirmishLobby`, `initMultiplayerLobbyState`,
+`initializeFinalResultsForActivePlayers` and `createCompoundAddressBasedOnSelectedProvider`, and the compiler
+itself says why: its stack colorer sorts the elements by weight per byte and hands out offsets in that order. The
+mechanism, the evidence and the debugger recipe are in
+[tools/compiler-internals/msvc-backend.md](tools/compiler-internals/msvc-backend.md); what matters here is how to
+read a diff:
+
+* A slot-order mismatch means our locals carry **different reference counts** from the original's, so look for a
+  read the original has and we do not. It is not a declaration-order problem, which is why every declaration-order
+  hypothesis for the `Map::Units::UnitsState` frame mismatches came back a tie.
+* Weight **ties** are left to the order the symbols arrive in, permuted by an unstable quicksort. That is the
+  whole reason `perm_decls.py` took `assignSelectionToKey` from 95.9% to 100% once and does nothing in most
+  functions: permuting declarations can only decide ties.
+* Locals with disjoint live ranges **share** one slot, and a local can be placed on the slot of a parameter that
+  is already dead (`writeMapOrSaveFile` puts eight locals on the first parameter's slot). A slot count is
+  therefore not a local count, and Ghidra's "repurposed parameter" rendering - `playerID = 0` used as a loop
+  counter in `aiRecruitUnits` - is this reuse, not something the original source wrote. Transcribing it back cost
+  7 points.
+* Renaming locals is inert; the comparator never looks at a name.
+
+`slot_weight.py` prints the slot table for one function with each local's weight beside it, read out of MSVC's own
+`/FA` symbol table rather than inferred from the diff.
+
+For **registers** there is no decoded rule yet. Treat a register-only mismatch as an interference difference - a
+different set of values was live at that point - which is a working hypothesis, not an established mechanism.
 
 ### Statement Order
 

tools/compiler-internals/asm_scan.py

@@ -0,0 +1,95 @@
+"""Find functions whose ORIGINAL contains handwritten inline assembly.
+
+usage: asm_scan.py [SNAPSHOT.json] [SRCDIR ...]
+
+Without arguments it scans every .cpp in cmake/openshc-sources.txt.local; with a reccmp_report.py
+snapshot it scans only the functions below normalized 100 in it, and prints their percentages.
+
+Why this exists: tracePathPlanToDestinationViaUnoccupiedTiles sat at 12.5% with a frame nine
+locals short, and no C++ change could touch it, because the original's loop is an `__asm` block.
+MSVC keeps an ebp frame in any function containing inline assembly, and every local the block
+names has to live in memory - so the "missing locals" were the asm's operands, and the pragma
+that gets the frame pointer (`optimize("y", off)`) gets nothing else. LinkageNeighbourAsm.hpp
+documents the same discovery for three siblings. Such a function has to be written as asm,
+instruction for instruction; tuning the C++ around it is wasted.
+
+The fingerprints, each a thing MSVC's own codegen never emits:
+
+  ebp   an ebp frame (push ebp / mov ebp, esp) WITHOUT the `and esp, -8` of stack alignment
+  zx    `mov eax, 0` immediately followed by `mov ax, word ptr ...` (MSVC writes movzx)
+  c16   16-bit register compares, `cmp ax, cx` and the like
+  rld   layer bases reloaded from their own stack slot, `mov esi/edi, dword ptr [ebp - N]`
+  al0   `cmp al/cl/dl, 0` (MSVC writes test)
+
+The score is 3 for the frame plus 2 per zero-extension plus the others; anything scoring 3 or
+more is listed. An ebp frame alone is also what `__asm`-free functions with a `double` or an
+alloca get, so read the other columns before believing it. Over the 97 sub-100 PathFindingState
+functions this listed exactly the four that are asm; the other 93 are ordinary C++ problems.
+"""
+
+import io
+import json
+import os
+import re
+import subprocess
+import sys
+
+ROOT = os.path.abspath(os.path.join(os.path.dirname(os.path.abspath(__file__)), '..', '..'))
+MARK = re.compile(r'FUNCTION:\s*STRONGHOLDCRUSADER\s+(0x[0-9A-Fa-f]+)')
+
+
+def original(addr):
+    out = subprocess.run([sys.executable, 'tools/reimplementation-helper/batch/orig_body.py', addr],
+                         cwd=ROOT, capture_output=True, text=True).stdout
+    return [re.sub(r'^[0-9a-f]{8}\s+', '', l) for l in out.splitlines() if re.match(r'^[0-9a-f]{8}\s', l)]
+
+
+def fingerprints(ins):
+    n = len(ins)
+    ebp = n > 2 and ins[0].startswith('push    ebp') and ins[1].startswith('mov     ebp, esp') \
+        and not any('and     esp' in x for x in ins[:5])
+    zx = sum(1 for i in range(n - 1) if re.match(r'mov\s+eax, 0$', ins[i]) and re.match(r'mov\s+ax,', ins[i + 1]))
+    c16 = sum(1 for x in ins if re.match(r'cmp\s+[a-d]x, [a-d]x$', x))
+    rld = sum(1 for x in ins if re.match(r'mov\s+(esi|edi), dword ptr \[ebp - 0x[0-9a-f]+\]$', x))
+    al0 = sum(1 for x in ins if re.match(r'cmp\s+(al|cl|dl), 0$', x))
+    score = (3 if ebp else 0) + zx * 2 + c16 + rld // 3 + al0
+    return score, n, ebp, zx, c16, rld, al0
+
+
+def main():
+    args = sys.argv[1:]
+    snap = None
+    if args and args[0].endswith('.json'):
+        snap = json.load(io.open(args[0], encoding='utf-8'))
+        args = args[1:]
+    files = []
+    if args:
+        for d in args:
+            files += [os.path.join(d, f) for f in sorted(os.listdir(os.path.join(ROOT, d))) if f.endswith('.cpp')]
+    else:
+        files = [l.strip() for l in io.open(os.path.join(ROOT, 'cmake/openshc-sources.txt.local')) if l.strip()]
+    pct = {}
+    if snap is not None:
+        for k, (raw, norm) in snap.items():
+            pct[k.split('::')[-1]] = (100 * raw, 100 * norm)
+    rows = []
+    for p in files:
+        short = os.path.basename(p)[:-4]
+        if snap is not None and (short not in pct or pct[short][1] >= 99.95):
+            continue
+        m = MARK.search(io.open(os.path.join(ROOT, p), encoding='utf-8', errors='replace').read(4000))
+        if not m:
+            continue
+        score, n, ebp, zx, c16, rld, al0 = fingerprints(original(m.group(1)))
+        rows.append((score, pct.get(short, (0, 0)), short, n, ebp, zx, c16, rld, al0))
+    rows.sort(reverse=True)
+    print('%5s %5s %5s  %-52s %4s ebp zx c16 rld al0' % ('score', 'raw', 'norm', 'function', 'insn'))
+    for score, (raw, norm), short, n, ebp, zx, c16, rld, al0 in rows:
+        if score >= 3:
+            print('%5d %5.1f %5.1f  %-52s %4d  %s  %2d %3d %3d %3d'
+                  % (score, raw, norm, short[:52], n, 'Y' if ebp else '-', zx, c16, rld, al0))
+    print('(%d function(s) scanned; listed: score >= 3)' % len(rows))
+
+
+if __name__ == '__main__':
+    main()