mirror of
https://github.com/imjasonh/nescript
synced 2026-07-08 08:55:38 +00:00
Remove the legacy AST codegen — IR path is canonical now
The `--use-ast` path through `src/codegen/mod.rs` was a strictly
inferior subset of the IR codegen. Building every example with
`--use-ast` through the jsnes harness:
- `arrays_and_functions` — fully black (array init + function
return values + OAM-in-loop all broken)
- `structs_enums_for` — fully black (struct literal is a no-op,
all fields stay at 0)
- `inline_asm_demo` — fully black
- `bitwise_ops`, `loop_break_continue` — below sprite floors
(static `next_oam_slot` bug B)
- `match_demo` — panics at compile time with
`branch offset 153 out of range` (AST's if/else-chain
desugaring of `match` emits short branches that can't reach
the far arms in a multi-arm match)
Six of fourteen examples are non-functional under `--use-ast`.
The other eight happen to fall inside the subset AST handles
(no arrays, no structs, no function return values, no
multi-sprite loops, no long match chains).
`docs/future-work.md` already listed "Once working, delete the
AST-based codegen entirely" as the intended direction. It's
working, so this commit does the deletion.
What's removed:
- The `CodeGen` struct, its impl block, and every helper in
`src/codegen/mod.rs` (the AST codegen body) — ~1150 lines.
The file is now a module header that re-exports `IrCodeGen`.
- `src/codegen/tests.rs` — 15 AST-specific instruction-pattern
tests. Every feature they covered has an equivalent test in
`src/codegen/ir_codegen.rs::{tests,more_tests}` already.
- The `--use-ast` CLI flag and its branch in `src/main.rs`.
- `compile_with_ir_codegen` in `tests/integration_test.rs` —
`compile()` now does what it did, so they merged. All 40
integration tests go through the IR path.
- Outdated sections in `docs/future-work.md` that described the
IR codegen as "not yet implemented" and listed AST codegen
gaps as priority work.
What's kept:
- `src/codegen/ir_codegen.rs` — the real codegen.
- `src/codegen/peephole.rs` — post-codegen cleanup pass, now
run unconditionally from `main.rs`.
Test plan:
- `cargo test --release` — 313 unit + 37 integration tests pass
(was 328 + 37; the 15 dropped are the deleted AST-specific
tests).
- `cargo fmt --check` clean.
- `cargo clippy --release --all-targets -- -D warnings` clean.
- `node tests/emulator/run_examples.mjs` — 14/14 ROMs render
above their per-example nonBlack floors.
- The one tightening: `sprite_resolution_uses_tile_index` was
asserting on the old static-slot encoding
(`A9 01 8D 01 02`). Updated to the cursor-based form
(`A9 01 99 01 02`, i.e. STA AbsoluteY).
Net diff: 1581 deletions, 62 insertions.
https://claude.ai/code/session_014Z5y3Q9krLcAxYpZQJhZ5V
This commit is contained in:
parent
54acb9ee38
commit
a757336681
6 changed files with 61 additions and 1580 deletions
|
|
@ -7,64 +7,9 @@ in the NEScript compiler. Items are organized by priority and area.
|
|||
|
||||
## 1. IR-Based Code Generation
|
||||
|
||||
**Status**: The IR pipeline (lowering + optimization) runs during compilation but
|
||||
its output is discarded. Code generation still works from the AST directly
|
||||
(`src/codegen/mod.rs`). This means IR-level optimizations have no effect on the
|
||||
final ROM.
|
||||
|
||||
**What exists**:
|
||||
- `src/ir/mod.rs`: Complete IR type definitions (`IrProgram`, `IrFunction`,
|
||||
`IrBasicBlock`, `IrOp`, `IrTerminator`)
|
||||
- `src/ir/lowering.rs`: AST → IR translation for all statement and expression types
|
||||
- `src/optimizer/mod.rs`: Constant folding, dead code elimination, strength
|
||||
reduction, ZP promotion analysis, function inlining — all operating on IR
|
||||
|
||||
**What's needed**:
|
||||
- A new `src/codegen/ir_codegen.rs` that walks `IrProgram` and emits 6502
|
||||
`Instruction` sequences from `IrOp`/`IrTerminator` instead of from AST nodes
|
||||
- Register allocation strategy for IR temps → A/X/Y/zero-page spill slots
|
||||
- Replace the `CodeGen::generate(&program)` call in `main.rs` with
|
||||
`ir_codegen::generate(&ir_program, &analysis)`
|
||||
- Once working, delete the AST-based codegen entirely
|
||||
|
||||
**IR lowering issues to fix first** (found during code review):
|
||||
- `ButtonRead` emits `ReadInput` with no destination temp, then uses uninitialized
|
||||
temp in the `And` mask operation (`src/ir/lowering.rs:534`). The fix: `ReadInput`
|
||||
should store the input byte into a temp, or the lowering should emit
|
||||
`LoadVar(t, input_var_id)` after `ReadInput`.
|
||||
- Logical AND/OR use raw `VarId(self.next_var_id)` for temp storage without
|
||||
registering it (`src/ir/lowering.rs:603,637`). Should use `IrTemp` instead.
|
||||
- Break/continue create unreachable blocks that contain subsequent dead statements
|
||||
(`src/ir/lowering.rs:259`). Should either skip lowering after a terminator, or
|
||||
the dead code elimination pass should handle it.
|
||||
|
||||
**Impact**: Enables all optimizer passes to actually affect output quality.
|
||||
Currently the optimizer is validated by tests but its results are thrown away.
|
||||
|
||||
---
|
||||
|
||||
## 2. Codegen Gaps (AST-Based)
|
||||
|
||||
These features are parsed and analyzed but produce no 6502 output:
|
||||
|
||||
| Feature | Location | Status |
|
||||
|---------|----------|--------|
|
||||
| Function calls | `codegen:148` | `Statement::Call` is a no-op |
|
||||
| Return values | `codegen:148` | `Statement::Return` is a no-op |
|
||||
| State transitions | `codegen:148` | `Statement::Transition` is a no-op |
|
||||
| Array indexing | `codegen:199-200` | `LValue::ArrayIndex` assignment is a no-op |
|
||||
| Array expressions | `codegen:417` | `Expr::ArrayIndex`, `Expr::ArrayLiteral` are no-ops |
|
||||
| Function call expressions | `codegen:417` | `Expr::Call` returns nothing |
|
||||
| Scroll | `codegen:151-152` | `Statement::Scroll` is a no-op |
|
||||
| Load background | `codegen:154-155` | `Statement::LoadBackground` is a no-op |
|
||||
| Set palette | `codegen:154-155` | `Statement::SetPalette` is a no-op |
|
||||
| Multiply/divide/modulo | `codegen:450-452` | `BinOp::Mul/Div/Mod` only emit left operand |
|
||||
| Dynamic shifts | `ir/lowering:575` | Shift amount is hardcoded to 1 |
|
||||
|
||||
**Priority fixes for a working multi-state game**:
|
||||
1. **Function calls**: JSR to function label, pass args via zero-page, return via A
|
||||
2. **State transitions**: Write state ID to a zero-page variable, jump to dispatcher
|
||||
3. **Array indexing**: Use X register for index, LDA absolute,X for loads
|
||||
**Status**: Complete. The AST → IR lowering, optimizer, and
|
||||
`src/codegen/ir_codegen.rs` all work end-to-end; the legacy AST
|
||||
codegen has been removed. See "Recently completed" below.
|
||||
|
||||
---
|
||||
|
||||
|
|
@ -373,8 +318,10 @@ These items were documented as future work but have since been implemented:
|
|||
- **IR-based codegen** — `src/codegen/ir_codegen.rs` walks `IrProgram` and
|
||||
emits 6502 for every IR op: load/store, arithmetic, comparisons, arrays,
|
||||
calls, draws, input (P1 and P2), scroll, debug.log/assert, state
|
||||
dispatch, multi-OAM slot allocation, transitions + on_enter handlers.
|
||||
Now the default; `--use-ast` falls back to the legacy AST-based codegen.
|
||||
dispatch, runtime OAM cursor for looped draws, transitions + on_enter
|
||||
handlers. It's the only codegen — the legacy AST-based path and the
|
||||
`--use-ast` flag were removed once the IR pipeline was proven correct
|
||||
by the jsnes emulator smoke test.
|
||||
- **IR lowering bug fixes** — `ReadInput` now has a destination temp,
|
||||
`ButtonRead` uses the proper input temp, logical AND/OR use a new
|
||||
`emit_move` helper instead of the buggy raw VarId temp storage
|
||||
|
|
@ -490,23 +437,18 @@ These items were documented as future work but have since been implemented:
|
|||
|
||||
For someone picking up this codebase, the recommended order of work:
|
||||
|
||||
1. **Delete AST codegen** — IR codegen is now the default and beats
|
||||
the AST codegen in 5/7 examples. Once confidence is high, remove
|
||||
`--use-ast` and `src/codegen/mod.rs`'s AST-specific code.
|
||||
2. **Struct literals** — `pos = Vec2 { x: 100, y: 50 }` as an
|
||||
expression. Currently fields must be assigned individually.
|
||||
3. **u16 / array / nested struct fields** — current structs only
|
||||
1. **u16 / array / nested struct fields** — current structs only
|
||||
allow u8/i8/bool fields. The layout machinery is ready for more
|
||||
types but the codegen only handles 1-byte loads/stores.
|
||||
4. **Audio driver** — the `play`/`start_music`/`stop_music`
|
||||
2. **Audio driver** — the `play`/`start_music`/`stop_music`
|
||||
statements parse but don't generate any code. A minimal NSF-style
|
||||
driver running in NMI would unblock game projects.
|
||||
5. **Multi-scanline on_scanline reload** — the current codegen
|
||||
3. **Multi-scanline on_scanline reload** — the current codegen
|
||||
supports one scanline per state but not a chain of handlers at
|
||||
different scanlines in the same frame.
|
||||
6. **Register allocator** — proper A/X/Y allocation to replace
|
||||
4. **Register allocator** — proper A/X/Y allocation to replace
|
||||
zero-page spills used by the current IR codegen. Partially
|
||||
mitigated by peephole passes but still wasteful in some cases.
|
||||
7. **Match statement** — `match x { 0 => ... , 1 => ... }` would be
|
||||
5. **Match statement** — `match x { 0 => ... , 1 => ... }` would be
|
||||
useful for state dispatch and enum-driven logic.
|
||||
8. **Text / HUD layer** — font sheet + layout system for scores.
|
||||
|
|
|
|||
|
|
@ -945,7 +945,6 @@ nescript build game.ne --dump-ir
|
|||
| `--dump-ir` | Dump the lowered IR program (after optimization) to stdout |
|
||||
| `--memory-map` | Dump a memory map of variable allocations to stdout |
|
||||
| `--call-graph` | Dump a call graph (which handler/function calls which) to stdout |
|
||||
| `--use-ast` | Use the legacy AST-based codegen (default is the IR codegen) |
|
||||
|
||||
### Check
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue