Files
Sylpheed/docs/re/structures/isl-conditions.md
Sylpheed RE agent 29eda81ede re: recover ISL conditions by CFG dataflow instead of a linear walk
The linear walk's 10% unknown was a floor imposed by the method: a block entered only
by a branch has a well-defined state, just not one a straight-line pass can see.
tools/re-capture/isl_cfg.py replaces it with a worklist fixpoint that joins each
block's state over its ACTUAL predecessors -- a value survives only if every
predecessor agrees.

Over all 28 stages:
  instructions reached by the CFG          85.0%
  condition sites, unknown LHS             756 (10.00%) -> 402 (5.32%)
  of those, never reached at all           389
  joined away (predecessors disagree)       13
  both resolve but DISAGREE                161   <- linear walk was wrong here

Those 161 are on top of the 889 the previous jmp fix caught.

Two zero-results on the way, both my own bug, both caught because the number looked
wrong rather than because a test failed:

  * The first CFG run reached only 36% of instructions and made things WORSE (35%
    unknown).  Cause: the phase bases reach almost nothing.  Most routines are
    COROUTINES the engine starts from its trigger queue, with no static predecessor,
    so every start_coroutine target has to be seeded as an entry.
  * That seeding then found ZERO entries in a file with 216 start_coroutine calls,
    because the target is staged in TWO steps -- special[0] = imm, then
    local[0] = special[0] -- and I matched only the direct-immediate form.

Reachability went 36% -> 64% -> 85% as each was fixed.

The 389 still unreached are an honest limit rather than a gap: nothing in the bytecode
starts them; they are entered from the trigger queue at phase+272, by data rather than
code, so no purely static analysis reaches them.

isl_report.py conditions now uses isl_cfg; calls and phase-ends regenerate
byte-identical.  Stage 02 unknowns drop from 71 to 25.
2026-08-27 06:02:37 +00:00

229 lines
9.8 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# ✅ 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 2124 `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** | **402 — 5.3 %** (CFG dataflow; the linear walk left 756) |
| 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.
## ✅ Replaced by a CFG dataflow fixpoint — `isl_cfg.py`
The linear walk's honest 10 % unknown was a floor imposed by the method, not by
the data: a block entered only by a branch has *a* well-defined state, just not
one a straight-line pass can see. So the walk is gone, replaced by a worklist
fixpoint that joins each block's state over its **actual predecessors** — a value
survives only if every predecessor agrees.
| over all 28 stages | linear walk | CFG dataflow |
|---|---|---|
| instructions reached | (all, but with stale state) | **85.0 %** |
| condition sites with an unknown LHS | 756 — 10.00 % | **402 — 5.32 %** |
| of those, never reached at all | — | 389 |
| joined away (predecessors genuinely disagree) | — | **13** |
| **sites where both resolve but DISAGREE** | — | **161** |
Those 161 are 161 more places the linear walk reported a confident wrong answer,
on top of the 889 the `jmp` fix caught.
### ⚠️ Two zero-results that were both my bug
The first CFG run reached only **36 %** of instructions and made things *worse*
— 35 % unknown against the walk's 10 %. Two causes, and each surfaced as a
suspiciously round zero:
1. **The phase bases reach almost nothing.** Most routines are **coroutines** the
engine starts from its trigger queue, so they have no static predecessor at
all. They must be seeded from every `start_coroutine` target.
2. **The seeding found ZERO entries in a file with 216 `start_coroutine` calls.**
The target is staged in *two* steps — `special[0] = imm`, then
`local[0] = special[0]` — and I matched only the direct-immediate form.
Reachability went 36 % → 64 % → **85 %** as each was fixed. The lesson is the one
this corpus keeps re-teaching: *a count that does not move when it should is the
bug reporting itself.* Both times I checked because the number looked wrong, not
because a test failed.
### 🟡 The 389 that remain are a real limit, not a gap
Nothing in the bytecode starts them. They are entered from the **trigger queue at
`phase+272`** — by data, not by code — so no purely static analysis reaches them.
Resolving those needs the trigger table's contents, which is a separate question.