Reading builtin80's body (0x82268460) to name it: it is NOT a predicate. It allocates a 20-byte object, stamps vtable 0x820A8CB0, magic 0xAB0311BA and the unit's live object into it, pushes it onto a queue via the same helper push.i uses, and returns 1 -- or 0 when the unit is absent. A command. That made the conditions listing impossible: it showed a six-way switch `if builtin80(TCT206) == 0 … == 5` on a function returning 1 or 0. Disassembling the site shows two unconditional `jmp`s between the call and the compare, so 0x1B6C0 is reached ONLY by a branch and its special[0] has nothing to do with builtin80. op12 is unconditional -- the next instruction is never reached by fall-through -- and the tracker walked through it exactly as it had walked through end_coroutine. Last iteration I fixed the instance and not the class, leaving 22x more bad sites in place than the fix removed. A/B over all 28 stages, 7563 sites, resetting at jmp as well: sites whose operands change 889 (11.75%) LHS unresolved, before -> after 34 (0.45%) -> 756 (10.00%) So the previous commit's headline "0.0% unresolved" was a MISSING CHECK, not a strong result: the linear walk always had some value to report, and reporting it was the bug. 10% is the honest figure and the other 90% is trustworthy for a reason. Also corrected: isl-unit-args.md illustrated its diff with 0x1B6C0, which is one of the bogus sites. The UNIT_ARG result itself stands -- it came from reading implementations, not from this listing -- but the example was picked from bad output. Not done, and said so: recovering the 756 needs a dataflow join over each block's actual predecessors, a CFG fixpoint rather than a linear pass. The branch targets are all known so the CFG is available; the analysis is not written. calls and phase-ends regenerate byte-identical; conditions changes on 187 lines.
185 lines
7.6 KiB
Markdown
185 lines
7.6 KiB
Markdown
# ✅ Every condition site, with its comparand — the clear conditions are readable
|
||
|
||
This is what the whole ISL chain was for. `data/isl-stage02-conditions.txt` has
|
||
existed for a long time as a **stale artefact with no generator** —
|
||
`isl_report.py`'s own docstring says so. It has one now, and the conditions are
|
||
resolved rather than printed as `special[N]`.
|
||
|
||
## ✅ The deque ops are an expression stack
|
||
|
||
`isl-bytecode.md` names ops 21–24 `push.i`/`push.f`/`pop.i`/`pop.f` over deques
|
||
at `phase+44` / `phase+64`. What they are *used for* is the missing piece:
|
||
|
||
```
|
||
set.i special[0] = 1
|
||
set.i special[1] = special[0]
|
||
push.i ; save the comparand
|
||
set.i local[0] = 1
|
||
set.i local[4] = 0x49
|
||
call unit_state(ADT308) ; result -> special[0] (clobbers it)
|
||
pop.i ; restore comparand -> special[1]
|
||
cmp.i special[0], special[1]
|
||
beq -> 0xFEB4
|
||
```
|
||
|
||
A textbook stack-machine lowering: **push the left operand, evaluate the right,
|
||
pop, compare.** Tracking the stack through the decode is therefore enough to
|
||
recover what every site actually tests.
|
||
|
||
### The evidence it is a stack, not something else
|
||
|
||
| check | result |
|
||
|---|---|
|
||
| `push` vs `pop` across all 28 stages | **1877 vs 1877** |
|
||
| files where the deque underflows or ends unbalanced | **0 of 28** |
|
||
| Stage 02 `pop.i` sites immediately followed by `cmp.i` | **319 / 319** |
|
||
| ops immediately preceding a `pop.i` | `call` ×313, `cmp.a` ×6 |
|
||
|
||
Perfect balance with zero underflow across 28 independent files is not something
|
||
a wrong model produces, and `pop.i` → `cmp.i` at 319/319 makes `pop.i` a reliable
|
||
marker for a condition site.
|
||
|
||
## ✅ Result — disc-wide
|
||
|
||
`isl.conditions()` walks the linear stream tracking `special[]`, `local[]` and
|
||
the stack, then reports each compare with its branch:
|
||
|
||
| | all 28 stages |
|
||
|---|---|
|
||
| condition sites | **7 563** |
|
||
| **LHS honestly unresolved** | **756 — 10.0 %** |
|
||
| RHS is a plain number | 7 544 — 99.7 % |
|
||
|
||
🔴 **The first version of this table claimed 0.0 % unresolved. That was wrong** —
|
||
see *The correction* below. ~11.75 % of the operands it reported were derived
|
||
from state that leaked across an unconditional jump.
|
||
|
||
**Most-tested predicates disc-wide:** `hp_pct_test` 1955, `unit_state` 1257,
|
||
`unit_relation` 796, `dist_lt` 450, `request_script_message` 425,
|
||
`unit_alive` 413, `read_freg` 187.
|
||
|
||
And they read as conditions:
|
||
|
||
```
|
||
0x032F4 ph1 if unit_alive(TCN105) != 1 -> 0x3388
|
||
0x04D70 ph1 if unit_hp_pct(TCN001, Character_Player_Test) != 0 -> 0x51FC
|
||
0x2C4E0 ph3 if hp_pct_test(ADT308, 0) != 1 -> 0x2CF8C
|
||
0x30790 ph3 if dist_lt(ADT308, TCN000, 15000) != 1 -> 0x307B0
|
||
0x33FA0 ph3 if unit_state(ADT308) != 2 -> 0x34030
|
||
0x349E0 ph3 if unit_state(ADT308) == 1 -> 0x34A00
|
||
```
|
||
|
||
`dist_lt(ADT308, TCN000, 15000)` — with the world unit established as 1 metre,
|
||
that is a **15 km** proximity test.
|
||
|
||
## ✅ FIXED — and the cause was not what I expected
|
||
|
||
The first version of this listing showed **`end_coroutine`** as the left-hand
|
||
side of 34 comparisons disc-wide (15 in Stage 02). That is impossible —
|
||
`end_coroutine` returns no value a script can test — and an impossible output is
|
||
the bug reporting itself.
|
||
|
||
### 🔴 The obvious fix is REFUTED
|
||
|
||
The plan recorded here was *"set `special[0]` only for built-ins that write
|
||
`[phase+164]`"*. That would not have worked: `end_coroutine`'s handler
|
||
`0x82272624` is
|
||
|
||
```
|
||
addi r11, r0, 1
|
||
addi r3, r0, 3
|
||
stw r11, 164(r31) ; it DOES write [phase+164]
|
||
```
|
||
|
||
so the filter would have kept it. Checking the handler before writing the filter
|
||
is what caught this.
|
||
|
||
### ✅ The real cause: a coroutine boundary
|
||
|
||
`end_coroutine` returns **3**, which *destroys the thread*. Execution does not
|
||
continue past it — so the instructions that follow it in the **flat** stream
|
||
belong to a **different routine**, and every value the tracker was carrying is
|
||
stale. The linear walk that makes this decode possible at all is precisely what
|
||
walks across that boundary.
|
||
|
||
Resetting the tracker at `end_coroutine`:
|
||
|
||
| A/B over all 28 stages, 7563 sites | |
|
||
|---|---|
|
||
| sites whose operands change | **34 — 0.45 %** |
|
||
| LHS `end_coroutine` before → after | **34 → 0** |
|
||
| left as an explicit unknown afterwards | **34 — 0.45 %** |
|
||
|
||
**The two counts are equal, so the leak was confined to exactly the sites that
|
||
displayed the impossible value** — the other 7 529 conditions were never
|
||
affected. Those 34 now print
|
||
`<unknown: reached after a coroutine boundary>` rather than a wrong answer.
|
||
|
||
🟡 **They are recoverable but not recovered.** The right-hand side of each is
|
||
still exact; only the left is lost. Resolving them means seeding the tracker at
|
||
each coroutine **entry** rather than walking in from the previous routine, and
|
||
the entries are available — `start_coroutine`'s target is staged slot 0. Not
|
||
done.
|
||
|
||
## 🟡 Not settled
|
||
|
||
* The **35 unnamed built-ins** still print as `builtinN` — `builtin103` (115
|
||
sites), `builtin105` (117) and `builtin16` (132) are the highest-traffic
|
||
unknowns, and each is now a vtable-slot lookup away.
|
||
* **Which condition guards each `END_PHASE`.** Every site is readable, but
|
||
linking a condition to the phase exit it eventually reaches needs the control
|
||
flow between them, which this listing does not follow.
|
||
* Only Stage 02's artefact is committed; the other 27 generate from the same
|
||
command but are not in the tree.
|
||
|
||
|
||
## 🔴 The correction — I fixed the instance, not the class
|
||
|
||
The `end_coroutine` fix above was **too narrow**, and the giveaway was again an
|
||
impossible output: the listing showed
|
||
|
||
```
|
||
if builtin80(TCT206) == 0 … == 1 … == 2 … == 3 … == 4 … == 5
|
||
```
|
||
|
||
a six-way switch on a built-in that returns only **1 or 0**. Reading
|
||
`builtin80`'s body (`0x82268460`) settles that it is not a predicate at all — it
|
||
allocates a 20-byte object, stamps a vtable `0x820A8CB0`, a magic `0xAB0311BA`
|
||
and the unit's live object into it, pushes it onto a queue via the same helper
|
||
`push.i` uses, and returns 1, or 0 when the unit is absent. **It is a command.**
|
||
|
||
Disassembling the site explains it:
|
||
|
||
```
|
||
01B698 call builtin80(TCT206)
|
||
01B6A4 set.i global[76] = 0
|
||
01B6B0 jmp -> 0x1B738
|
||
01B6B8 jmp -> 0x1B738
|
||
01B6C0 cmp.i … <- reached ONLY by a branch from elsewhere
|
||
```
|
||
|
||
Two unconditional jumps sit between the call and the compare. **`op12` is
|
||
unconditional, so the next instruction is never reached by fall-through** — and
|
||
the tracker walked straight through it, exactly as it had walked through
|
||
`end_coroutine`. One is a thread boundary and the other a block boundary, but
|
||
they are the same defect: a linear walk cannot carry state across a point where
|
||
control does not flow.
|
||
|
||
| A/B over all 28 stages, 7563 sites | |
|
||
|---|---|
|
||
| sites whose operands change once `jmp` also resets | **889 — 11.75 %** |
|
||
| LHS unresolved, before → after | 34 (0.45 %) → **756 (10.00 %)** |
|
||
|
||
So the earlier "0.0 % unresolved" was not a strong result, it was a **missing
|
||
check**: the walk always had *some* value to report, and reporting it was the
|
||
bug. 10 % is the honest figure, and the remaining 90 % is now trustworthy for a
|
||
reason — the state reaching those sites really does flow there.
|
||
|
||
🟡 **Recovering the 756 needs real dataflow.** Each is a block entered only by a
|
||
branch, so its state is the *join* over its actual predecessors — a fixpoint over
|
||
the CFG, not a linear pass. The branch targets are all known (`[phase+232] +
|
||
word@+4`), so the CFG is available; the analysis is not written.
|
||
|
||
**Method note worth keeping:** when the same defect appears twice, fix the class.
|
||
Patching `end_coroutine` alone left 22× more bad sites in place than it removed,
|
||
and only another impossible-looking output exposed them. |