Skip to content

Commit 2ceec0b

Browse files
CopilotjonathanpeppersCopilot
authored
Switch statement coverage: state-machine tests, mini-sample, docs (#535)
* Initial plan * Add switch state-machine tests, sample, and update docs Agent-Logs-Url: https://github.com/jonathanpeppers/dotnes/sessions/75bd402b-4c42-406c-86de-f4a5e97f7589 Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com> * Tighten switch test assertions per review feedback - SwitchWithDefault: assert the CMP/BNE+JMP trampoline signature (C9 imm D0 03 4C) for cases 1..3, plus BNE+JMP (D0 03 4C) for case 0, instead of bare 'D0'/'4C' substrings that match unrelated control flow. - SwitchStateMachine: give the default arm a distinctive sentinel (lives = 0xAB) so the test can prove the default body is actually emitted, and assert the same trampoline signature for cases 1, 2 and the case 0 BEQ-style trampoline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Replace statemachine sample with SwitchStateMachineWithRendering test The samples/statemachine project wasn't pulling its weight as a standalone sample — it just demonstrated the switch dispatch pattern that's already covered by tests. Convert its content into a dedicated RoslynTests case that also covers the surrounding rendering setup (per-state pal_col, NTADR_A + vram_write, ppu_on_all). - Add SwitchStateMachineWithRendering to ControlFlowTests, asserting both the CMP/BNE+JMP trampoline signature and the distinctive per-case pal_col immediates (0x02 / 0x0A / 0x06). - Delete samples/statemachine entirely. - Update docs/8bitworkshop-samples.md to reference the new test name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add SwitchLarge test covering 32-case dense switch Verifies that a large dense integral switch (Roslyn lowers to the IL 'switch' opcode / jump table) transpiles end-to-end. dotnes expands the opcode into a linear chain of CMP/BNE+JMP trampolines; this test asserts the trampoline signature at several case indices (1, 15, 31) and that the default arm is reachable. Not an optimization — just a regression guard that the transpiler doesn't choke on big case counts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Make STATE_TITLE arm assertion distinctive The STATE_TITLE case body was setting pal_col(0, 0x02), which is also called in the setup block — so asserting A902 would pass even if the case 0 dispatch was broken. Change the arm to pal_col(0, 0x12) and assert A912 instead, matching the distinctiveness of the other arms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com> Co-authored-by: jonathanpeppers <jonathan.peppers@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 825b962 commit 2ceec0b

2 files changed

Lines changed: 203 additions & 2 deletions

File tree

docs/8bitworkshop-samples.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@
216216
- Extensive use of pointers, arrays of structs, bitfields
217217
- `static` variables, `const` arrays
218218
- Multiple user-defined functions with complex control flow
219-
- `switch/case` statements
220219
- Far exceeds dotnes's current single-top-level-statement model
221220

222221
### climber.c
@@ -340,7 +339,7 @@
340339
Prioritized TODO list of features needed to port climber.c, ordered by dependency and impact:
341340

342341
- [x] **User functions with return values** — byte return values work: the last computed value stays in A through `incsp` parameter cleanup and RTS. The caller's `HasReturnValue` check sets `_runtimeValueInA = true`. Verified with RoslynTests: constant return, parameterized return, return value stored to local.
343-
- [x] **switch/case** — small switches use branch chains (beq.s, already supported). Larger sequential switches use the IL `switch` opcode (0x45) which emits CMP/BNE+JMP trampolines. Tested with SwitchSmall (3 cases) and SwitchEnum (7 cases with enum).
342+
- [x] **switch/case** — small switches use branch chains (beq.s, already supported). Larger sequential switches use the IL `switch` opcode (0x45) which emits CMP/BNE+JMP trampolines. `default:` clauses fall through correctly. Tested with SwitchSmall (3 cases), SwitchEnum (7 cases with enum), SwitchWithDefault (5-way byte with default), SwitchStateMachine (title/playing/over game-state pattern), and SwitchStateMachineWithRendering (full pal_col + vram_write rendering loop).
344343
- [x] **BCD arithmetic**`bcd_add(ushort, ushort)` built-in NESLib function backed by 6502 subroutine. Software BCD since NES CPU disabled hardware BCD mode. Also fixed: 16-bit return values from built-in functions now correctly store both bytes (STA+STX) to word locals.
345344
- [x] **Global/static variables**`stsfld`/`ldsfld` for user-defined static class fields. Allocated at `$0325+` (same region as locals). Both `static class State { public static byte x; }` and local variables work for the same patterns. Tested with store, load, and loop increment.
346345
- [x] **sbyte (signed char)**`Ldc_i4_m1`, negative constants via two's complement, `conv.i1`/`conv.i2`/`conv.i4` as no-ops on 8-bit 6502. Signed comparisons work via `Blt_s` (BMI).

src/dotnes.tests/ControlFlowTests.cs

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,208 @@ enum ActorState : byte { Inactive, Standing, Walking, Climbing, Jumping, Falling
119119
Assert.Contains("C906", hex);
120120
}
121121

122+
[Fact]
123+
public void SwitchWithDefault()
124+
{
125+
// switch on byte with 4+ cases and a default clause.
126+
// The default must run when the discriminator does not match any case.
127+
var bytes = GetProgramBytes(
128+
"""
129+
byte op = 4;
130+
byte r = 0;
131+
switch (op) {
132+
case 0: r = 0x10; break;
133+
case 1: r = 0x20; break;
134+
case 2: r = 0x30; break;
135+
case 3: r = 0x16; break;
136+
default: r = 0x26; break;
137+
}
138+
pal_col(0, r);
139+
ppu_on_all();
140+
while (true) ;
141+
""");
142+
Assert.NotNull(bytes);
143+
Assert.NotEmpty(bytes);
144+
145+
var hex = Convert.ToHexString(bytes);
146+
// Each non-zero case must emit the BNE+JMP trampoline:
147+
// CMP #imm (C9 imm), BNE +3 (D0 03), JMP target (4C lo hi)
148+
// Assert the 5-byte prefix C9 imm D0 03 4C is present for cases 1..3.
149+
Assert.Contains("C901D0034C", hex);
150+
Assert.Contains("C902D0034C", hex);
151+
Assert.Contains("C903D0034C", hex);
152+
// Case 0 uses BEQ-style trampoline (no CMP): BNE +3, JMP target.
153+
Assert.Contains("D0034C", hex);
154+
// The default value 0x26 must appear (LDA #$26 = A926) — this is the
155+
// distinctive side effect of the default arm and proves fall-through dispatch.
156+
Assert.Contains("A926", hex);
157+
}
158+
159+
[Fact]
160+
public void SwitchStateMachine()
161+
{
162+
// The state-machine pattern from the issue: title / playing / over with a default.
163+
// Roslyn lowers a dense byte switch with default into IL `switch` + br to default.
164+
// The default arm sets `lives` to a distinctive sentinel value (0xAB) so the
165+
// assertions can prove the default arm's body is actually emitted, separate
166+
// from the normal state transitions.
167+
var bytes = GetProgramBytes(
168+
"""
169+
const byte STATE_TITLE = 0;
170+
const byte STATE_PLAYING = 1;
171+
const byte STATE_OVER = 2;
172+
173+
byte state = STATE_TITLE;
174+
byte lives = 3;
175+
176+
while (true)
177+
{
178+
pad_poll(0);
179+
switch (state)
180+
{
181+
case STATE_TITLE:
182+
if ((pad_trigger(0) & PAD.START) != 0)
183+
state = STATE_PLAYING;
184+
break;
185+
case STATE_PLAYING:
186+
if (lives == 0)
187+
state = STATE_OVER;
188+
break;
189+
case STATE_OVER:
190+
if ((pad_trigger(0) & PAD.START) != 0)
191+
state = STATE_TITLE;
192+
break;
193+
default:
194+
lives = 0xAB;
195+
state = STATE_TITLE;
196+
break;
197+
}
198+
ppu_wait_nmi();
199+
}
200+
""");
201+
Assert.NotNull(bytes);
202+
Assert.NotEmpty(bytes);
203+
204+
var hex = Convert.ToHexString(bytes);
205+
// Each non-zero case must emit the BNE+JMP trampoline signature:
206+
// CMP #imm (C9 imm), BNE +3 (D0 03), JMP target (4C lo hi).
207+
// Asserting the 5-byte prefix proves the dispatch is CMP/BNE+JMP, not
208+
// unrelated CMP/BNE/JMP from surrounding control flow.
209+
Assert.Contains("C901D0034C", hex);
210+
Assert.Contains("C902D0034C", hex);
211+
// Case 0 uses BEQ-style trampoline (value already in A): BNE +3, JMP target.
212+
Assert.Contains("D0034C", hex);
213+
// The default arm's distinctive sentinel value 0xAB must appear
214+
// (LDA #$AB = A9AB), proving the default body is reachable/emitted.
215+
Assert.Contains("A9AB", hex);
216+
}
217+
218+
[Fact]
219+
public void SwitchStateMachineWithRendering()
220+
{
221+
// Full game-style state machine: nametable text setup + per-state background
222+
// color via pal_col, driven by START on controller 1. Each case body has a
223+
// distinctive pal_col immediate so we can assert the dispatch lands in each arm.
224+
var bytes = GetProgramBytes(
225+
"""
226+
const byte STATE_TITLE = 0;
227+
const byte STATE_PLAYING = 1;
228+
const byte STATE_OVER = 2;
229+
230+
pal_col(0, 0x02);
231+
pal_col(1, 0x14);
232+
pal_col(2, 0x20);
233+
pal_col(3, 0x30);
234+
235+
vram_adr(NTADR_A(8, 14));
236+
vram_write("PRESS START");
237+
ppu_on_all();
238+
239+
byte state = STATE_TITLE;
240+
241+
while (true)
242+
{
243+
ppu_wait_nmi();
244+
pad_poll(0);
245+
246+
switch (state)
247+
{
248+
case STATE_TITLE:
249+
pal_col(0, 0x12);
250+
if ((pad_trigger(0) & PAD.START) != 0)
251+
state = STATE_PLAYING;
252+
break;
253+
case STATE_PLAYING:
254+
pal_col(0, 0x0A);
255+
if ((pad_trigger(0) & PAD.START) != 0)
256+
state = STATE_OVER;
257+
break;
258+
case STATE_OVER:
259+
pal_col(0, 0x06);
260+
if ((pad_trigger(0) & PAD.START) != 0)
261+
state = STATE_TITLE;
262+
break;
263+
default:
264+
state = STATE_TITLE;
265+
break;
266+
}
267+
}
268+
""");
269+
Assert.NotNull(bytes);
270+
Assert.NotEmpty(bytes);
271+
272+
var hex = Convert.ToHexString(bytes);
273+
// CMP/BNE+JMP trampolines for cases 1 and 2 (case 0 uses BEQ-style).
274+
Assert.Contains("C901D0034C", hex);
275+
Assert.Contains("C902D0034C", hex);
276+
Assert.Contains("D0034C", hex);
277+
// Distinctive per-case pal_col immediates prove each arm's body is emitted.
278+
// Each value is chosen to NOT appear in the setup pal_col calls above,
279+
// so the assertion can only be satisfied by the switch arm itself.
280+
// LDA #imm = A9 imm.
281+
Assert.Contains("A912", hex); // STATE_TITLE arm
282+
Assert.Contains("A90A", hex); // STATE_PLAYING arm
283+
Assert.Contains("A906", hex); // STATE_OVER arm
284+
}
285+
286+
[Fact]
287+
public void SwitchLarge()
288+
{
289+
// Build a dense 32-case switch programmatically. Roslyn lowers a dense
290+
// integral switch to the IL `switch` opcode (jump table), which dotnes
291+
// expands into a linear chain of CMP/BNE+JMP trampolines. This just
292+
// verifies the transpiler doesn't choke on a large case count and that
293+
// the dispatch chain is actually emitted.
294+
const int caseCount = 32;
295+
var sb = new System.Text.StringBuilder();
296+
sb.AppendLine("byte op = 17;");
297+
sb.AppendLine("byte r = 0;");
298+
sb.AppendLine("switch (op) {");
299+
for (int i = 0; i < caseCount; i++)
300+
{
301+
// Distinctive value per case: 0x40 + i (avoids collision with case index).
302+
sb.AppendLine($" case {i}: r = 0x{0x40 + i:X2}; break;");
303+
}
304+
sb.AppendLine(" default: r = 0xFF; break;");
305+
sb.AppendLine("}");
306+
sb.AppendLine("pal_col(0, r);");
307+
sb.AppendLine("ppu_on_all();");
308+
sb.AppendLine("while (true) ;");
309+
310+
var bytes = GetProgramBytes(sb.ToString());
311+
Assert.NotNull(bytes);
312+
Assert.NotEmpty(bytes);
313+
314+
var hex = Convert.ToHexString(bytes);
315+
// Spot-check the BNE+JMP trampoline signature at several case indices.
316+
// CMP #i, BNE +3, JMP target = C9 ii D0 03 4C.
317+
Assert.Contains("C901D0034C", hex); // case 1
318+
Assert.Contains("C90FD0034C", hex); // case 15
319+
Assert.Contains("C91FD0034C", hex); // case 31 (last)
320+
// Default sentinel 0xFF must appear (LDA #$FF = A9FF).
321+
Assert.Contains("A9FF", hex);
322+
}
323+
122324
[Fact]
123325
public void DoWhileLoop()
124326
{

0 commit comments

Comments
 (0)