From 43ff4da2086eb3c6a83176acd94334f1e19a8cff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 24 May 2026 23:08:17 +0000 Subject: [PATCH 1/4] Initial plan From 6482e8d891cbe2566e2081655018f752ad258dde Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 24 May 2026 23:29:27 +0000 Subject: [PATCH 2/4] Add transpiler support for fixed-buffer and [InlineArray] struct fields Agent-Logs-Url: https://github.com/jonathanpeppers/dotnes/sessions/6da1081e-c84d-4e00-b827-95f5b6947d92 Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com> --- docs/8bitworkshop-samples.md | 7 +- .../Utilities/IL2NESWriter.ILDispatch.cs | 16 + .../Utilities/IL2NESWriter.StructFields.cs | 350 ++++++++++++++++++ src/dotnes.tasks/Utilities/IL2NESWriter.cs | 13 + .../Utilities/Transpiler.ILParsing.cs | 10 + .../Utilities/Transpiler.StructAnalysis.cs | 137 +++++++ src/dotnes.tasks/Utilities/Transpiler.cs | 10 + src/dotnes.tests/StructsTests.cs | 138 +++++++ 8 files changed, 678 insertions(+), 3 deletions(-) diff --git a/docs/8bitworkshop-samples.md b/docs/8bitworkshop-samples.md index 8273a0c7..e651fa66 100644 --- a/docs/8bitworkshop-samples.md +++ b/docs/8bitworkshop-samples.md @@ -171,9 +171,9 @@ - **Missing Features:** - Direct APU register macros (`APU_PULSE_DECAY`, `APU_PULSE_SWEEP`, `APU_TRIANGLE_LENGTH`, `APU_NOISE_DECAY`, `APU_ENABLE`) - `APU.status` — direct hardware register reading - - `typedef struct` — no struct support - `sprintf()` — no string formatting - Global arrays of structs, `const` struct arrays +- **Implemented:** `typedef struct` now maps to C# `struct` with `byte`/`ushort` fields, fixed-size arrays via `fixed byte buf[N]` (Option A, requires `unsafe`) or `[InlineArray(N)]` (Option B). See `StructsTests` for examples. ### ppuhello.c - **Description:** Directly programs PPU registers (`PPU.control`, `PPU.mask`, `PPU.vram.address`, `PPU.vram.data`) to display text — no neslib used. @@ -212,7 +212,7 @@ - **Status:** 🔴 Complex - **Missing Features:** - Extremely large codebase (100+ KB) with dozens of functions - - `typedef struct` and struct instances — no struct support + - `typedef struct` and struct instances — basic struct field access works (including `fixed`-buffer and `[InlineArray(N)]` fields), but pointers to structs are not yet supported - Extensive use of pointers, arrays of structs, bitfields - `static` variables, `const` arrays - Multiple user-defined functions with complex control flow @@ -294,7 +294,7 @@ |-----------------|-----------------| | User-defined functions with parameters | 25+ samples (byte params and return values now supported; ushort/string params pending) | | Global/static arrays | 15+ samples | -| `typedef struct` / struct support | 8 samples (basic field access now supported; arrays of structs, pointers to structs pending) | +| `typedef struct` / struct support | 8 samples (basic field access, `fixed`-buffer and `[InlineArray(N)]` fields supported; pointers to structs pending) | | Direct APU/PPU register access | 5 samples (apu.c already covered by built-in `apu_init()`) | | Mapper support (MMC3, UxROM) | 3 samples | | CC65-specific libraries (conio, joystick) | 2 samples | @@ -329,6 +329,7 @@ | Global/static variables (`stsfld`/`ldsfld`) — user-defined static fields at $0325+ | climber, shoot2, siegegame (game state) | | `sbyte` (signed char) — negative constants, `conv.i1`/`conv.i2` as no-ops | climber, shoot2, siegegame | | Arrays of structs — `newarr` struct, `ldelema`, `stfld`/`ldfld` with AbsoluteX | climber, shoot2, siegegame | +| `fixed`-buffer (`fixed byte buf[N]`) and `[InlineArray(N)]` fields in structs — buffer-indexed reads/writes via `ldflda` + `stind.i1`/`ldind.u1` | StructsTests (`FixedBufferConstantIndex`, `FixedBufferRuntimeIndex`, `InlineArrayConstantIndex`, `InlineArrayRuntimeIndex`) | | `Array.Fill` / `Array.Copy` — inline 6502 fill/copy loops | climber, siegegame | | ca65 assembler — full expression evaluator (&&, \|\|, !), conditional assembly, all addressing modes | fami (FamiTone2 linking) | | Extern assembly linking — `static extern` → `JSR _func`, `.s` files assembled and linked | fami (FamiTone2, music data, SFX data) | diff --git a/src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs b/src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs index 32dab35b..b83ce61a 100644 --- a/src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs +++ b/src/dotnes.tasks/Utilities/IL2NESWriter.ILDispatch.cs @@ -1731,6 +1731,12 @@ public void Write(ILInstruction instruction, int operand) // native integer sizeof values and will always push 1 here. WriteLdc(1); break; + case ILOpCode.Initobj: + // initobj on a struct local: pop the ldloca.s address. The transpiler + // does not zero-initialise the underlying zero-page bytes because user + // code is expected to write all fields before reading them. + _pendingStructLocal = null; + break; default: throw new TranspileException(GetUnsupportedOpcodeMessage(instruction.OpCode), MethodName); } @@ -3409,6 +3415,16 @@ or nameof(NESLib.set_vram_update)) case ILOpCode.Ldfld: HandleLdfld(operand); break; + case ILOpCode.Ldflda: + HandleLdflda(operand); + break; + case ILOpCode.Initobj: + // initobj on a struct local: pop the ldloca.s address. The transpiler + // does not zero-initialise the underlying zero-page bytes because user + // code is expected to write all fields before reading them (matching the + // existing behaviour for `Point p;` without an explicit `= default`). + _pendingStructLocal = null; + break; case ILOpCode.Newobj: if (operand == "Array2D.Byte.Ctor") { diff --git a/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs b/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs index 9dc02bd0..4d30fe87 100644 --- a/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs +++ b/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs @@ -394,4 +394,354 @@ void HandleLdfld(string fieldName) Stack.Push(0); } } + + /// + /// Handles ldflda on a buffer field (fixed byte buf[N] or + /// [InlineArray(N)]-typed field). Pattern-matches the full IL pattern through + /// the matching stind.i1 or ldind.u1 and emits a single LDA/STA + /// (Absolute or AbsoluteX) at base+offset+index. + /// Supported indices: constant, ldarg, ldloc, ldsfld. + /// + void HandleLdflda(string fieldName) + { + // We only handle the OUTER ldflda on a buffer field. The inner ldflda + // (FixedElementField) is reached later via the skip mechanism and would not + // hit this handler. If we somehow reach an inner ldflda directly, the absence + // of _pendingStructLocal causes us to fall through with a clearer error below. + if (_pendingStructLocal is null) + throw new TranspileException( + $"ldflda '{fieldName}' without a preceding ldloca.s is not supported.", MethodName); + + if (!BufferFieldSizes.TryGetValue(fieldName, out int bufferSize) || bufferSize <= 0) + throw new TranspileException( + $"ldflda '{fieldName}' is only supported for fixed-size buffer or [InlineArray] fields. " + + "Passing a struct field by reference is not yet supported.", MethodName); + + int localIndex = _pendingStructLocal.Value; + _pendingStructLocal = null; + + string structType = ResolveStructType(localIndex, fieldName); + ushort baseAddr = GetOrAllocateStructLocal(localIndex, structType); + int fieldOffset = GetFieldOffset(structType, fieldName); + ushort bufferBase = (ushort)(baseAddr + fieldOffset); + + if (Instructions is null) + throw new InvalidOperationException("HandleLdflda requires Instructions to be set"); + + // Walk forward and classify each instruction until we reach stind.i1 / ldind.u1. + // Recognize: + // - second ldflda (FixedElementField) — ignore (fixed buffer) + // - call to InlineArrayElementRef / InlineArrayFirstElementRef — ignore (inline array) + // - call to InlineArrayAsSpan + stloc + ldloca.s + ... + call get_Item — Span path + // - ldc.i4.* / ldarg.* / ldloc.* / ldsfld — index expression + // - add — pointer + index (fixed buffer); ignore + // - ldc.i4.* immediately before stind.i1 — the VALUE being stored + int? indexConst = null; + int? indexLocalIdx = null; + int? indexArgIdx = null; + string? indexStaticField = null; + int? storeValueConst = null; + bool storeValueRuntime = false; // true when value comes from arg/local/etc., not yet supported here + bool isStore = false; + bool isLoad = false; + bool spanPath = false; + int endIndex = -1; + + for (int j = Index + 1; j < Instructions.Length; j++) + { + var op = Instructions[j].OpCode; + switch (op) + { + case ILOpCode.Ldflda: + // Inner ldflda (FixedElementField): ignore. + continue; + case ILOpCode.Call: + { + string? mname = Instructions[j].String; + if (mname is "InlineArrayElementRef" or "InlineArrayFirstElementRef") + continue; + if (mname is "InlineArrayAsSpan") + { + spanPath = true; + continue; + } + if (mname is "get_Item" && spanPath) + continue; + // Any other call inside this sequence is unexpected + throw new TranspileException( + $"Unexpected call '{mname}' while lowering buffer field '{fieldName}' access.", MethodName); + } + case ILOpCode.Stloc_0: case ILOpCode.Stloc_1: case ILOpCode.Stloc_2: case ILOpCode.Stloc_3: + case ILOpCode.Stloc_s: case ILOpCode.Stloc: + // Inside the Span path, Roslyn stores the Span in a temp local. + if (spanPath) continue; + throw new TranspileException($"Unexpected stloc while lowering buffer field '{fieldName}' access.", MethodName); + case ILOpCode.Ldloca_s: case ILOpCode.Ldloca: + if (spanPath) continue; + throw new TranspileException($"Unexpected ldloca while lowering buffer field '{fieldName}' access.", MethodName); + case ILOpCode.Add: + // Fixed-buffer pointer + index. Just ignore — we already track base separately. + continue; + case ILOpCode.Ldc_i4_0: indexConst ??= 0; break; + case ILOpCode.Ldc_i4_1: indexConst ??= 1; break; + case ILOpCode.Ldc_i4_2: indexConst ??= 2; break; + case ILOpCode.Ldc_i4_3: indexConst ??= 3; break; + case ILOpCode.Ldc_i4_4: indexConst ??= 4; break; + case ILOpCode.Ldc_i4_5: indexConst ??= 5; break; + case ILOpCode.Ldc_i4_6: indexConst ??= 6; break; + case ILOpCode.Ldc_i4_7: indexConst ??= 7; break; + case ILOpCode.Ldc_i4_8: indexConst ??= 8; break; + case ILOpCode.Ldc_i4_s: + case ILOpCode.Ldc_i4: + indexConst ??= Instructions[j].Integer ?? 0; + break; + case ILOpCode.Ldarg_0: indexArgIdx ??= 0; break; + case ILOpCode.Ldarg_1: indexArgIdx ??= 1; break; + case ILOpCode.Ldarg_2: indexArgIdx ??= 2; break; + case ILOpCode.Ldarg_3: indexArgIdx ??= 3; break; + case ILOpCode.Ldarg_s: case ILOpCode.Ldarg: + indexArgIdx ??= Instructions[j].Integer ?? 0; + break; + case ILOpCode.Ldloc_0: indexLocalIdx ??= 0; break; + case ILOpCode.Ldloc_1: indexLocalIdx ??= 1; break; + case ILOpCode.Ldloc_2: indexLocalIdx ??= 2; break; + case ILOpCode.Ldloc_3: indexLocalIdx ??= 3; break; + case ILOpCode.Ldloc_s: case ILOpCode.Ldloc: + indexLocalIdx ??= Instructions[j].Integer ?? 0; + break; + case ILOpCode.Ldsfld: + indexStaticField ??= Instructions[j].String; + break; + case ILOpCode.Conv_u: case ILOpCode.Conv_i: case ILOpCode.Conv_u1: + case ILOpCode.Conv_i1: case ILOpCode.Conv_u2: case ILOpCode.Conv_i2: + case ILOpCode.Conv_u4: case ILOpCode.Conv_i4: + case ILOpCode.Nop: + continue; + case ILOpCode.Stind_i1: + // The instruction immediately before stind.i1 is the VALUE. + isStore = true; + endIndex = j; + // The value should already be the most recently seen ldc.i4 (and we + // tracked it in indexConst because of the ??=). But we need to disambiguate: + // for stind.i1 with constant index, the order is: + // [add] stind.i1 + // So the LAST ldc.i4 seen is actually the value, and the FIRST ldc.i4 was + // the index. We re-scan to extract these correctly. + break; + case ILOpCode.Ldind_u1: + isLoad = true; + endIndex = j; + break; + default: + throw new TranspileException( + $"Unsupported opcode '{op}' while lowering buffer field '{fieldName}' access.", MethodName); + } + if (endIndex >= 0) break; + } + + if (endIndex < 0) + throw new TranspileException( + $"Could not find matching stind.i1/ldind.u1 for buffer field '{fieldName}'.", MethodName); + + // Re-scan precisely to separate index from value for the store case. + // Pattern shapes we accept: + // Fixed buffer store: [ldflda inner] [add] stind.i1 + // InlineArray store: call InlineArrayElementRef stind.i1 + // InlineArray store 0: call InlineArrayFirstElementRef stind.i1 + // Fixed buffer load: [ldflda inner] [add] ldind.u1 + // InlineArray load: call InlineArrayElementRef ldind.u1 + // InlineArray load 0: call InlineArrayFirstElementRef ldind.u1 + // InlineArray runtime: ldc.i4 N call InlineArrayAsSpan stloc.M + // ldloca.s M call get_Item stind.i1/ldind.u1 + indexConst = null; indexLocalIdx = null; indexArgIdx = null; indexStaticField = null; + storeValueConst = null; storeValueRuntime = false; + + // For the Span path, the index is between ldloca.s (temp span local) and the call to get_Item. + // For other paths, the index is before "add"/call. + int valueScanStart = isStore ? endIndex - 1 : -1; + int indexBoundaryEnd; // exclusive index for the index expression scan + if (spanPath) + { + // Find the call to get_Item; index is between the previous ldloca.s and that call. + int callItemIdx = -1, ldlocaIdx = -1; + for (int k = Index + 1; k < endIndex; k++) + { + if (Instructions[k].OpCode == ILOpCode.Call && Instructions[k].String == "get_Item") + { callItemIdx = k; break; } + if (Instructions[k].OpCode is ILOpCode.Ldloca_s or ILOpCode.Ldloca) + ldlocaIdx = k; + } + if (callItemIdx < 0 || ldlocaIdx < 0) + throw new TranspileException( + $"InlineArray runtime-index pattern for '{fieldName}' was not recognised.", MethodName); + indexBoundaryEnd = callItemIdx; + // Scan range for index: [ldlocaIdx+1, callItemIdx) + ScanForIndexAndValue(ldlocaIdx + 1, indexBoundaryEnd, isStore ? valueScanStart : -1, + out indexConst, out indexLocalIdx, out indexArgIdx, out indexStaticField, + out storeValueConst, out storeValueRuntime); + } + else + { + indexBoundaryEnd = endIndex; // up to but not including stind/ldind + ScanForIndexAndValue(Index + 1, indexBoundaryEnd, isStore ? valueScanStart : -1, + out indexConst, out indexLocalIdx, out indexArgIdx, out indexStaticField, + out storeValueConst, out storeValueRuntime); + } + + // Determine addressing + bool runtimeIdx = indexLocalIdx != null || indexArgIdx != null || indexStaticField != null; + + // Pop IL evaluation stack entries that the consumed instructions pushed. + // For store: the stind.i1 pops
and . The address was conceptually + // pushed by the outer ldflda. Our existing dispatch hasn't actually pushed anything + // for ldflda (we're handling it right now), and we haven't dispatched any of the + // skipped instructions, so we don't need to touch the IL stack — neither the index + // nor the value was visibly pushed. + + if (isStore) + { + // Value to store + if (storeValueConst is null && !storeValueRuntime) + throw new TranspileException( + $"Could not determine value being stored to buffer field '{fieldName}'.", MethodName); + + if (!runtimeIdx) + { + ushort target = (ushort)(bufferBase + (indexConst ?? 0)); + Emit(Opcode.LDA, AddressMode.Immediate, (byte)(storeValueConst!.Value & 0xFF)); + Emit(Opcode.STA, AddressMode.Absolute, target); + } + else + { + EmitLoadIndexIntoX(indexLocalIdx, indexArgIdx, indexStaticField); + Emit(Opcode.LDA, AddressMode.Immediate, (byte)(storeValueConst!.Value & 0xFF)); + Emit(Opcode.STA, AddressMode.AbsoluteX, bufferBase); + } + _runtimeValueInA = false; + _immediateInA = null; + } + else if (isLoad) + { + if (!runtimeIdx) + { + ushort target = (ushort)(bufferBase + (indexConst ?? 0)); + Emit(Opcode.LDA, AddressMode.Absolute, target); + } + else + { + EmitLoadIndexIntoX(indexLocalIdx, indexArgIdx, indexStaticField); + Emit(Opcode.LDA, AddressMode.AbsoluteX, bufferBase); + } + _runtimeValueInA = true; + _immediateInA = null; + // ldind.u1 leaves a byte on the IL stack + Stack.Push(0); + } + + // Skip every instruction we just consumed (including the stind.i1/ldind.u1). + SkipUntilIndex = endIndex + 1; + previous = isStore ? ILOpCode.Stind_i1 : ILOpCode.Ldind_u1; + } + + void ScanForIndexAndValue(int scanStart, int scanEndExclusive, int valueScanFrom, + out int? indexConst, out int? indexLocalIdx, out int? indexArgIdx, out string? indexStaticField, + out int? storeValueConst, out bool storeValueRuntime) + { + indexConst = null; indexLocalIdx = null; indexArgIdx = null; indexStaticField = null; + storeValueConst = null; storeValueRuntime = false; + + if (Instructions is null) return; + + // Scan for the index: first integer-producing instruction in [scanStart, scanEndExclusive). + for (int k = scanStart; k < scanEndExclusive; k++) + { + var op = Instructions[k].OpCode; + int? c = op switch + { + ILOpCode.Ldc_i4_0 => 0, ILOpCode.Ldc_i4_1 => 1, ILOpCode.Ldc_i4_2 => 2, + ILOpCode.Ldc_i4_3 => 3, ILOpCode.Ldc_i4_4 => 4, ILOpCode.Ldc_i4_5 => 5, + ILOpCode.Ldc_i4_6 => 6, ILOpCode.Ldc_i4_7 => 7, ILOpCode.Ldc_i4_8 => 8, + ILOpCode.Ldc_i4 or ILOpCode.Ldc_i4_s => Instructions[k].Integer ?? 0, + _ => (int?)null, + }; + if (c != null) { indexConst = c; break; } + switch (op) + { + case ILOpCode.Ldarg_0: indexArgIdx = 0; break; + case ILOpCode.Ldarg_1: indexArgIdx = 1; break; + case ILOpCode.Ldarg_2: indexArgIdx = 2; break; + case ILOpCode.Ldarg_3: indexArgIdx = 3; break; + case ILOpCode.Ldarg_s: case ILOpCode.Ldarg: + indexArgIdx = Instructions[k].Integer ?? 0; break; + case ILOpCode.Ldloc_0: indexLocalIdx = 0; break; + case ILOpCode.Ldloc_1: indexLocalIdx = 1; break; + case ILOpCode.Ldloc_2: indexLocalIdx = 2; break; + case ILOpCode.Ldloc_3: indexLocalIdx = 3; break; + case ILOpCode.Ldloc_s: case ILOpCode.Ldloc: + indexLocalIdx = Instructions[k].Integer ?? 0; break; + case ILOpCode.Ldsfld: + indexStaticField = Instructions[k].String; break; + default: continue; + } + break; + } + + // Scan for the value (last constant immediately before stind.i1). + if (valueScanFrom >= 0) + { + for (int k = valueScanFrom; k > 0; k--) + { + var op = Instructions[k].OpCode; + int? c = op switch + { + ILOpCode.Ldc_i4_0 => 0, ILOpCode.Ldc_i4_1 => 1, ILOpCode.Ldc_i4_2 => 2, + ILOpCode.Ldc_i4_3 => 3, ILOpCode.Ldc_i4_4 => 4, ILOpCode.Ldc_i4_5 => 5, + ILOpCode.Ldc_i4_6 => 6, ILOpCode.Ldc_i4_7 => 7, ILOpCode.Ldc_i4_8 => 8, + ILOpCode.Ldc_i4 or ILOpCode.Ldc_i4_s => Instructions[k].Integer ?? 0, + _ => (int?)null, + }; + if (c != null) { storeValueConst = c; return; } + // Allow runtime expressions for the value in the future + if (op is ILOpCode.Conv_u1 or ILOpCode.Conv_i1 or ILOpCode.Nop) continue; + // Stop search at first non-skip instruction + break; + } + } + } + + void EmitLoadIndexIntoX(int? localIdx, int? argIdx, string? staticField) + { + if (localIdx != null && Locals.TryGetValue(localIdx.Value, out var lcl) && lcl.Address is int la) + { + Emit(Opcode.LDX, AddressMode.Absolute, (ushort)la); + return; + } + if (argIdx != null) + { + // Arguments for the main entry method are not addressable; only user methods + // have ldarg with usable addressing. For main, ldarg is rare. We support it for + // user methods by reading from the cc65 stack/parameter slot if available — but + // the existing transpiler models args as zero-page slots in some scenarios. + // For now, emit a clear error to surface unsupported edge cases early. + // The simplest supported case is a user method with one byte arg → arg is in A + // on entry, but by the time we reach this code A has been clobbered. So we use + // the parameter zero-page slot if the writer tracks one. + // Fall back to LDX from the same zero-page address allocated for arg-0. + // In dotnes, user-method args are pushed by the caller and accessed by the callee + // via specific patterns. For now, try local-address lookup; if missing, throw. + if (Locals.TryGetValue(argIdx.Value, out var argLocal) && argLocal.Address is int aa) + { + Emit(Opcode.LDX, AddressMode.Absolute, (ushort)aa); + return; + } + throw new TranspileException( + $"Runtime index from arg {argIdx} into a buffer field is not supported at this position.", MethodName); + } + if (staticField != null && Variables.StaticFieldAddresses.TryGetValue(staticField, out var sfa)) + { + Emit(Opcode.LDX, AddressMode.Absolute, sfa); + return; + } + throw new TranspileException("Could not resolve buffer-index expression to an addressable value.", MethodName); + } } diff --git a/src/dotnes.tasks/Utilities/IL2NESWriter.cs b/src/dotnes.tasks/Utilities/IL2NESWriter.cs index d1f5fc75..742e5ee4 100644 --- a/src/dotnes.tasks/Utilities/IL2NESWriter.cs +++ b/src/dotnes.tasks/Utilities/IL2NESWriter.cs @@ -404,6 +404,12 @@ bool _dupPendingSave /// public Dictionary> StructLayouts { get => Variables.StructLayouts; init => Variables.StructLayouts = value; } + /// + /// Maps struct field names that hold a fixed-size buffer (C# fixed byte buf[N]) + /// or an [InlineArray(N)] element field to their total byte count. + /// + public Dictionary BufferFieldSizes { get; init; } = new(StringComparer.Ordinal); + // ── Pending struct state ───────────────────────────────────────── // _pendingStructLocal is set by ldloca.s for simple struct locals. // _pendingStructElement consolidates the former _pendingStructElementType, @@ -416,6 +422,13 @@ bool _dupPendingSave /// int? _pendingStructLocal; + /// + /// When set, the dispatch loop should skip all IL instructions whose + /// is < this value. Used by handlers that consume a multi-instruction IL pattern + /// (e.g. buffer-field access via ldflda + stind.i1) in a single step. + /// + internal int? SkipUntilIndex { get; set; } + /// /// The IL offset where execution should resume after the current finally handler. /// Set by Leave/Leave_s, consumed by Endfinally. diff --git a/src/dotnes.tasks/Utilities/Transpiler.ILParsing.cs b/src/dotnes.tasks/Utilities/Transpiler.ILParsing.cs index 1d86681c..3a9ebd7f 100644 --- a/src/dotnes.tasks/Utilities/Transpiler.ILParsing.cs +++ b/src/dotnes.tasks/Utilities/Transpiler.ILParsing.cs @@ -27,6 +27,14 @@ public IEnumerable ReadStaticVoidMain() var methodName = _reader.GetString(methodDef.Name); + // Skip compiler-emitted helper methods in and similar + // synthesized types (e.g. C# 12 inline-array helpers: InlineArrayAsSpan, + // InlineArrayElementRef, InlineArrayFirstElementRef). + var declType = _reader.GetTypeDefinition(methodDef.GetDeclaringType()); + var declTypeName = _reader.GetString(declType.Name); + if (declTypeName.StartsWith("<")) + continue; + if (methodName == "Main" || methodName == "
$") { // Parse exception regions (try/finally) before yielding instructions @@ -208,6 +216,8 @@ IEnumerable ReadMethodBody(MethodDefinition methodDef, Dictionary var methodSpec = _reader.GetMethodSpecification((MethodSpecificationHandle)entity); if (methodSpec.Method.Kind == HandleKind.MemberReference) stringValue = GetQualifiedMemberName(_reader.GetMemberReference((MemberReferenceHandle)methodSpec.Method)); + else if (methodSpec.Method.Kind == HandleKind.MethodDefinition) + stringValue = _reader.GetString(_reader.GetMethodDefinition((MethodDefinitionHandle)methodSpec.Method).Name); break; case HandleKind.FieldDefinition: var field = _reader.GetFieldDefinition((FieldDefinitionHandle)entity); diff --git a/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs b/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs index 9428210b..d5991309 100644 --- a/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs +++ b/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs @@ -72,12 +72,31 @@ partial class Transpiler if (typeName.StartsWith("<") || typeName.Contains("__")) continue; + // Detect [InlineArray(N)] on the struct itself + int? inlineArrayLength = GetInlineArrayLength(type); + var fields = new List<(string Name, int Size)>(); foreach (var f in type.GetFields()) { var field = _reader.GetFieldDefinition(f); var fieldName = _reader.GetString(field.Name); int fieldSize = DecodeFieldSize(field); + // If the containing type is itself an [InlineArray(N)], its single instance + // field represents N copies of the element type. + if (inlineArrayLength is int nWrap && fieldSize >= 1) + { + fieldSize = nWrap * fieldSize; + _bufferFieldSizes[fieldName] = fieldSize; + } + else + { + int? bufLen = TryGetBufferFieldSize(field); + if (bufLen is int n) + { + fieldSize = n; + _bufferFieldSizes[fieldName] = n; + } + } fields.Add((fieldName, fieldSize)); } @@ -88,6 +107,124 @@ partial class Transpiler return result; } + /// + /// Maps struct field names that hold a fixed-size buffer (C# fixed byte buf[N]) + /// or an [InlineArray(N)] element field to their total byte count. Used by + /// to lower buffer-indexed reads/writes. + /// + internal Dictionary BufferFieldSizes => _bufferFieldSizes; + readonly Dictionary _bufferFieldSizes = new(StringComparer.Ordinal); + + /// + /// If the field is a fixed-size buffer (C# fixed byte buf[N]) or its value type + /// is decorated with [InlineArray(N)], returns the total byte size. Otherwise null. + /// + int? TryGetBufferFieldSize(FieldDefinition field) + { + // 1. Check for [FixedBuffer(typeof(T), N)] on the field itself. + foreach (var attrHandle in field.GetCustomAttributes()) + { + var attr = _reader.GetCustomAttribute(attrHandle); + if (GetAttributeTypeName(attr) != "FixedBufferAttribute") + continue; + // Constructor signature: (Type, Int32). Value blob: 01 00 [type-name encoded] [int32 length] 00 00 + try + { + var blob = _reader.GetBlobReader(attr.Value); + blob.ReadUInt16(); // prolog 0x0001 + // First arg: SerString — the type-name as a length-prefixed UTF-8 string. + blob.ReadSerializedString(); + int length = blob.ReadInt32(); + return length; + } + catch { /* fall through */ } + } + + // 2. Inspect the field's signature: if it is a VALUETYPE referencing a TypeDef + // that has [InlineArray(N)] applied, or that has explicit ClassLayout (fixed buffer + // helper struct), treat as a buffer. + var sig = _reader.GetBlobReader(field.Signature); + sig.ReadByte(); // calling convention (0x06) + byte et = sig.ReadByte(); + if (et != 0x11) // ELEMENT_TYPE_VALUETYPE + return null; + + int token = sig.ReadCompressedInteger(); + int tableIndex = token & 0x3; + int rowId = token >> 2; + if (tableIndex != 0 /* TypeDef */) + return null; + var typeDefHandle = MetadataTokens.TypeDefinitionHandle(rowId); + var valueTypeDef = _reader.GetTypeDefinition(typeDefHandle); + + // 2a. [InlineArray(N)] on the type. + int? inlineLen = GetInlineArrayLength(valueTypeDef); + if (inlineLen is int n && n > 0) + { + // Element size is the size of the type's single instance field + int elementSize = 1; + foreach (var ef in valueTypeDef.GetFields()) + { + elementSize = DecodeFieldSize(_reader.GetFieldDefinition(ef)); + break; + } + if (elementSize < 1) elementSize = 1; + return n * elementSize; + } + + // 2b. Compiler-generated fixed-buffer helper struct (name pattern e__FixedBuffer) + // has explicit ClassLayout.Size set to the buffer's total byte count. + var layout = valueTypeDef.GetLayout(); + if (!layout.IsDefault && layout.Size > 0) + return layout.Size; + return null; + } + + /// + /// Returns the length argument of an [InlineArray(N)] attribute on the type, + /// or null if not present. + /// + int? GetInlineArrayLength(TypeDefinition typeDef) + { + foreach (var attrHandle in typeDef.GetCustomAttributes()) + { + var attr = _reader.GetCustomAttribute(attrHandle); + if (GetAttributeTypeName(attr) != "InlineArrayAttribute") + continue; + try + { + var blob = _reader.GetBlobReader(attr.Value); + blob.ReadUInt16(); // prolog 0x0001 + int length = blob.ReadInt32(); + return length; + } + catch { /* fall through */ } + } + return null; + } + + /// + /// Returns the simple name of an attribute's type (e.g. "InlineArrayAttribute"). + /// + string GetAttributeTypeName(CustomAttribute attr) + { + switch (attr.Constructor.Kind) + { + case HandleKind.MemberReference: + var memberRef = _reader.GetMemberReference((MemberReferenceHandle)attr.Constructor); + if (memberRef.Parent.Kind == HandleKind.TypeReference) + return _reader.GetString(_reader.GetTypeReference((TypeReferenceHandle)memberRef.Parent).Name); + if (memberRef.Parent.Kind == HandleKind.TypeDefinition) + return _reader.GetString(_reader.GetTypeDefinition((TypeDefinitionHandle)memberRef.Parent).Name); + break; + case HandleKind.MethodDefinition: + var methodDef = _reader.GetMethodDefinition((MethodDefinitionHandle)attr.Constructor); + var declaringType = _reader.GetTypeDefinition(methodDef.GetDeclaringType()); + return _reader.GetString(declaringType.Name); + } + return string.Empty; + } + /// /// Decodes the size of a field from its signature blob. /// Returns -1 for array types (ELEMENT_TYPE_SZARRAY). diff --git a/src/dotnes.tasks/Utilities/Transpiler.cs b/src/dotnes.tasks/Utilities/Transpiler.cs index a5cda219..1441413e 100644 --- a/src/dotnes.tasks/Utilities/Transpiler.cs +++ b/src/dotnes.tasks/Utilities/Transpiler.cs @@ -282,6 +282,7 @@ public Program6502 BuildProgram6502(out ushort sizeOfMain, out ushort locals) ExternMethodNames = externNames, WordLocals = DetectWordLocals(instructions, reflectionCache), StructLayouts = structLayouts, + BufferFieldSizes = _bufferFieldSizes, StaticFieldAddresses = staticFields, WordStaticFields = wordStaticFields, LocalCount = staticFieldBytes, @@ -299,6 +300,10 @@ public Program6502 BuildProgram6502(out ushort sizeOfMain, out ushort locals) for (int i = 0; i < writer.Instructions.Length; i++) { writer.Index = i; + if (writer.SkipUntilIndex is int sk && i < sk) + continue; + if (writer.SkipUntilIndex is int sk2 && i >= sk2) + writer.SkipUntilIndex = null; var instruction = writer.Instructions[i]; // Record IL instruction labels for branch targets @@ -369,6 +374,7 @@ public Program6502 BuildProgram6502(out ushort sizeOfMain, out ushort locals) MethodName = methodName, WordLocals = DetectWordLocals(methodIL, reflectionCache), StructLayouts = structLayouts, + BufferFieldSizes = _bufferFieldSizes, ByteArrayLabelStartIndex = writer.ByteArrays.Count, StringLabelStartIndex = writer.StringTable.Count, LocalCount = methodFrameOffsets[methodName], @@ -395,6 +401,10 @@ public Program6502 BuildProgram6502(out ushort sizeOfMain, out ushort locals) for (int i = 0; i < methodWriter.Instructions.Length; i++) { methodWriter.Index = i; + if (methodWriter.SkipUntilIndex is int sk && i < sk) + continue; + if (methodWriter.SkipUntilIndex is int sk2 && i >= sk2) + methodWriter.SkipUntilIndex = null; var instruction = methodWriter.Instructions[i]; var labelName = $"{methodName}_instruction_{instruction.Offset:X2}"; if (methodWriter.CurrentBlock != null) diff --git a/src/dotnes.tests/StructsTests.cs b/src/dotnes.tests/StructsTests.cs index e3757016..152f7eb3 100644 --- a/src/dotnes.tests/StructsTests.cs +++ b/src/dotnes.tests/StructsTests.cs @@ -140,4 +140,142 @@ struct Pos { public byte x; public byte y; public byte flags; } Assert.Contains("A903", hex); // LDA #3 Assert.Contains("A928", hex); // LDA #40 } + + [Fact] + public void FixedBufferConstantIndex() + { + // Option A: C# fixed buffer with constant indexes. + var bytes = GetProgramBytes( + """ + unsafe { + Cluster cur; + cur.sprite = 0x10; + cur.id = 1; + cur.layout[0] = 5; + cur.layout[1] = 6; + cur.layout[2] = 7; + cur.layout[3] = 8; + pal_col(0, cur.layout[2]); + } + ppu_on_all(); + while (true) ; + + unsafe struct Cluster { + public byte sprite; + public byte id; + public fixed byte layout[4]; + } + """, additionalAssemblyFiles: null, allowUnsafe: true); + Assert.NotNull(bytes); + Assert.NotEmpty(bytes); + + var hex = Convert.ToHexString(bytes); + // Stores of the four constant values into successive zero-page absolute addresses. + Assert.Contains("A910", hex); // LDA #$10 (sprite) + Assert.Contains("A905", hex); // LDA #$05 + Assert.Contains("A906", hex); // LDA #$06 + Assert.Contains("A907", hex); // LDA #$07 + Assert.Contains("A908", hex); // LDA #$08 + // Absolute store byte (8D = STA abs) + Assert.Contains("8D", hex); + // Absolute load byte (AD = LDA abs) for the read + Assert.Contains("AD", hex); + } + + [Fact] + public void FixedBufferRuntimeIndex() + { + // Option A: runtime byte index uses LDX + AbsoluteX addressing. + var bytes = GetProgramBytes( + """ + byte i = 2; + unsafe { + Cluster cur; + cur.layout[i] = 99; + } + ppu_on_all(); + while (true) ; + + unsafe struct Cluster { + public byte sprite; + public byte id; + public fixed byte layout[4]; + } + """, additionalAssemblyFiles: null, allowUnsafe: true); + Assert.NotNull(bytes); + Assert.NotEmpty(bytes); + var hex = Convert.ToHexString(bytes); + Assert.Contains("A963", hex); // LDA #$63 (99) + // STA absolute,X (9D) for AbsoluteX store + Assert.Contains("9D", hex); + // LDX absolute (AE) to load the runtime index + Assert.Contains("AE", hex); + } + + [Fact] + public void InlineArrayConstantIndex() + { + // Option B: [InlineArray(N)] field, constant indexes. + var bytes = GetProgramBytes( + """ + Cluster cur = default; + cur.sprite = 0x10; + cur.id = 1; + cur.layout[0] = 5; + cur.layout[1] = 6; + cur.layout[2] = 7; + cur.layout[3] = 8; + pal_col(0, cur.layout[2]); + ppu_on_all(); + while (true) ; + + [System.Runtime.CompilerServices.InlineArray(4)] + struct Bytes4 { private byte _element0; } + + struct Cluster { + public byte sprite; + public byte id; + public Bytes4 layout; + } + """); + Assert.NotNull(bytes); + Assert.NotEmpty(bytes); + var hex = Convert.ToHexString(bytes); + Assert.Contains("A910", hex); + Assert.Contains("A905", hex); + Assert.Contains("A906", hex); + Assert.Contains("A907", hex); + Assert.Contains("A908", hex); + Assert.Contains("8D", hex); + Assert.Contains("AD", hex); + } + + [Fact] + public void InlineArrayRuntimeIndex() + { + // Option B: runtime index into [InlineArray]. + var bytes = GetProgramBytes( + """ + byte i = 1; + Cluster cur = default; + cur.layout[i] = 99; + ppu_on_all(); + while (true) ; + + [System.Runtime.CompilerServices.InlineArray(4)] + struct Bytes4 { private byte _element0; } + + struct Cluster { + public byte sprite; + public byte id; + public Bytes4 layout; + } + """); + Assert.NotNull(bytes); + Assert.NotEmpty(bytes); + var hex = Convert.ToHexString(bytes); + Assert.Contains("A963", hex); // LDA #$63 (99) + Assert.Contains("9D", hex); // STA abs,X + Assert.Contains("AE", hex); // LDX abs (runtime index) + } } From cc306877b2e989d61ae5c01856e09f827ea1846c Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 27 May 2026 16:18:36 -0500 Subject: [PATCH 3/4] Address PR review comments - Key BufferFieldSizes by (structType, fieldName) tuple so same-named buffer fields in distinct structs no longer collide. - Validate FixedBuffer element type is System.Byte; reject other element types explicitly instead of silently producing wrong layouts. - Reject arithmetic in buffer index expressions (e.g. buf[i + 1]) by counting Add opcodes during the buffer-access scan. - Throw when the index expression cannot be recognised instead of falling back to index 0. - Remove dead storeValueRuntime flag/out param; storeValueConst already handles the only supported case. - EmitLoadIndexIntoX: reject ldarg-sourced runtime indices outright instead of silently aliasing them to a same-numbered local. - Tests: clarify wording (absolute RAM addresses, not zero-page). - Transpiler.cs: split two statements that were stuck on one line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Utilities/IL2NESWriter.StructFields.cs | 71 ++++++++++--------- src/dotnes.tasks/Utilities/IL2NESWriter.cs | 8 ++- .../Utilities/Transpiler.StructAnalysis.cs | 30 ++++++-- src/dotnes.tasks/Utilities/Transpiler.cs | 3 +- src/dotnes.tests/StructsTests.cs | 5 +- 5 files changed, 69 insertions(+), 48 deletions(-) diff --git a/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs b/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs index 4d30fe87..8eaa9295 100644 --- a/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs +++ b/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs @@ -412,15 +412,15 @@ void HandleLdflda(string fieldName) throw new TranspileException( $"ldflda '{fieldName}' without a preceding ldloca.s is not supported.", MethodName); - if (!BufferFieldSizes.TryGetValue(fieldName, out int bufferSize) || bufferSize <= 0) - throw new TranspileException( - $"ldflda '{fieldName}' is only supported for fixed-size buffer or [InlineArray] fields. " + - "Passing a struct field by reference is not yet supported.", MethodName); - int localIndex = _pendingStructLocal.Value; _pendingStructLocal = null; string structType = ResolveStructType(localIndex, fieldName); + if (!BufferFieldSizes.TryGetValue((structType, fieldName), out int bufferSize) || bufferSize <= 0) + throw new TranspileException( + $"ldflda '{structType}.{fieldName}' is only supported for fixed-size buffer or [InlineArray] fields. " + + "Passing a struct field by reference is not yet supported.", MethodName); + ushort baseAddr = GetOrAllocateStructLocal(localIndex, structType); int fieldOffset = GetFieldOffset(structType, fieldName); ushort bufferBase = (ushort)(baseAddr + fieldOffset); @@ -441,10 +441,10 @@ void HandleLdflda(string fieldName) int? indexArgIdx = null; string? indexStaticField = null; int? storeValueConst = null; - bool storeValueRuntime = false; // true when value comes from arg/local/etc., not yet supported here bool isStore = false; bool isLoad = false; bool spanPath = false; + int addCount = 0; int endIndex = -1; for (int j = Index + 1; j < Instructions.Length; j++) @@ -480,7 +480,14 @@ void HandleLdflda(string fieldName) if (spanPath) continue; throw new TranspileException($"Unexpected ldloca while lowering buffer field '{fieldName}' access.", MethodName); case ILOpCode.Add: - // Fixed-buffer pointer + index. Just ignore — we already track base separately. + // Fixed-buffer lowering produces exactly one `add` for `base + index`. + // Inline-array helpers produce zero. Anything beyond one would imply + // additional arithmetic in the index expression (e.g. `buf[i + 1]`), + // which we don't yet recognise — reject explicitly to avoid silent + // miscompilation. + if (++addCount > 1) + throw new TranspileException( + $"Arithmetic in the index expression for buffer field '{fieldName}' is not supported.", MethodName); continue; case ILOpCode.Ldc_i4_0: indexConst ??= 0; break; case ILOpCode.Ldc_i4_1: indexConst ??= 1; break; @@ -554,7 +561,7 @@ void HandleLdflda(string fieldName) // InlineArray runtime: ldc.i4 N call InlineArrayAsSpan stloc.M // ldloca.s M call get_Item stind.i1/ldind.u1 indexConst = null; indexLocalIdx = null; indexArgIdx = null; indexStaticField = null; - storeValueConst = null; storeValueRuntime = false; + storeValueConst = null; // For the Span path, the index is between ldloca.s (temp span local) and the call to get_Item. // For other paths, the index is before "add"/call. @@ -578,19 +585,25 @@ void HandleLdflda(string fieldName) // Scan range for index: [ldlocaIdx+1, callItemIdx) ScanForIndexAndValue(ldlocaIdx + 1, indexBoundaryEnd, isStore ? valueScanStart : -1, out indexConst, out indexLocalIdx, out indexArgIdx, out indexStaticField, - out storeValueConst, out storeValueRuntime); + out storeValueConst); } else { indexBoundaryEnd = endIndex; // up to but not including stind/ldind ScanForIndexAndValue(Index + 1, indexBoundaryEnd, isStore ? valueScanStart : -1, out indexConst, out indexLocalIdx, out indexArgIdx, out indexStaticField, - out storeValueConst, out storeValueRuntime); + out storeValueConst); } // Determine addressing bool runtimeIdx = indexLocalIdx != null || indexArgIdx != null || indexStaticField != null; + // Validate that we actually recognised the index expression. Falling back to "index 0" + // would silently miscompile if the IL pattern changes or an unsupported form appears. + if (!runtimeIdx && indexConst is null) + throw new TranspileException( + $"Could not determine index expression for buffer field '{fieldName}'.", MethodName); + // Pop IL evaluation stack entries that the consumed instructions pushed. // For store: the stind.i1 pops
and . The address was conceptually // pushed by the outer ldflda. Our existing dispatch hasn't actually pushed anything @@ -601,20 +614,20 @@ void HandleLdflda(string fieldName) if (isStore) { // Value to store - if (storeValueConst is null && !storeValueRuntime) + if (storeValueConst is null) throw new TranspileException( $"Could not determine value being stored to buffer field '{fieldName}'.", MethodName); if (!runtimeIdx) { - ushort target = (ushort)(bufferBase + (indexConst ?? 0)); - Emit(Opcode.LDA, AddressMode.Immediate, (byte)(storeValueConst!.Value & 0xFF)); + ushort target = (ushort)(bufferBase + indexConst!.Value); + Emit(Opcode.LDA, AddressMode.Immediate, (byte)(storeValueConst.Value & 0xFF)); Emit(Opcode.STA, AddressMode.Absolute, target); } else { EmitLoadIndexIntoX(indexLocalIdx, indexArgIdx, indexStaticField); - Emit(Opcode.LDA, AddressMode.Immediate, (byte)(storeValueConst!.Value & 0xFF)); + Emit(Opcode.LDA, AddressMode.Immediate, (byte)(storeValueConst.Value & 0xFF)); Emit(Opcode.STA, AddressMode.AbsoluteX, bufferBase); } _runtimeValueInA = false; @@ -624,7 +637,7 @@ void HandleLdflda(string fieldName) { if (!runtimeIdx) { - ushort target = (ushort)(bufferBase + (indexConst ?? 0)); + ushort target = (ushort)(bufferBase + indexConst!.Value); Emit(Opcode.LDA, AddressMode.Absolute, target); } else @@ -645,10 +658,10 @@ void HandleLdflda(string fieldName) void ScanForIndexAndValue(int scanStart, int scanEndExclusive, int valueScanFrom, out int? indexConst, out int? indexLocalIdx, out int? indexArgIdx, out string? indexStaticField, - out int? storeValueConst, out bool storeValueRuntime) + out int? storeValueConst) { indexConst = null; indexLocalIdx = null; indexArgIdx = null; indexStaticField = null; - storeValueConst = null; storeValueRuntime = false; + storeValueConst = null; if (Instructions is null) return; @@ -701,7 +714,7 @@ void ScanForIndexAndValue(int scanStart, int scanEndExclusive, int valueScanFrom _ => (int?)null, }; if (c != null) { storeValueConst = c; return; } - // Allow runtime expressions for the value in the future + // Skip pass-through conversions that don't change the value if (op is ILOpCode.Conv_u1 or ILOpCode.Conv_i1 or ILOpCode.Nop) continue; // Stop search at first non-skip instruction break; @@ -718,24 +731,12 @@ void EmitLoadIndexIntoX(int? localIdx, int? argIdx, string? staticField) } if (argIdx != null) { - // Arguments for the main entry method are not addressable; only user methods - // have ldarg with usable addressing. For main, ldarg is rare. We support it for - // user methods by reading from the cc65 stack/parameter slot if available — but - // the existing transpiler models args as zero-page slots in some scenarios. - // For now, emit a clear error to surface unsupported edge cases early. - // The simplest supported case is a user method with one byte arg → arg is in A - // on entry, but by the time we reach this code A has been clobbered. So we use - // the parameter zero-page slot if the writer tracks one. - // Fall back to LDX from the same zero-page address allocated for arg-0. - // In dotnes, user-method args are pushed by the caller and accessed by the callee - // via specific patterns. For now, try local-address lookup; if missing, throw. - if (Locals.TryGetValue(argIdx.Value, out var argLocal) && argLocal.Address is int aa) - { - Emit(Opcode.LDX, AddressMode.Absolute, (ushort)aa); - return; - } + // Method arguments are loaded from the cc65 stack by `WriteLdarg` and are not + // tracked in `Locals`. Falling back to `Locals[argIdx]` here would silently + // load an unrelated same-numbered local. Until proper argument-to-X loading is + // implemented (mirroring WriteLdarg into X), reject this case explicitly. throw new TranspileException( - $"Runtime index from arg {argIdx} into a buffer field is not supported at this position.", MethodName); + $"Runtime index from method argument {argIdx} into a buffer field is not yet supported.", MethodName); } if (staticField != null && Variables.StaticFieldAddresses.TryGetValue(staticField, out var sfa)) { diff --git a/src/dotnes.tasks/Utilities/IL2NESWriter.cs b/src/dotnes.tasks/Utilities/IL2NESWriter.cs index 742e5ee4..f35450b6 100644 --- a/src/dotnes.tasks/Utilities/IL2NESWriter.cs +++ b/src/dotnes.tasks/Utilities/IL2NESWriter.cs @@ -405,10 +405,12 @@ bool _dupPendingSave public Dictionary> StructLayouts { get => Variables.StructLayouts; init => Variables.StructLayouts = value; } /// - /// Maps struct field names that hold a fixed-size buffer (C# fixed byte buf[N]) - /// or an [InlineArray(N)] element field to their total byte count. + /// Maps (structType, fieldName) pairs that hold a fixed-size buffer + /// (C# fixed byte buf[N]) or an [InlineArray(N)] element field to their + /// total byte count. Keyed by struct type so distinct structs with same-named buffer + /// fields don't collide. /// - public Dictionary BufferFieldSizes { get; init; } = new(StringComparer.Ordinal); + public Dictionary<(string StructType, string FieldName), int> BufferFieldSizes { get; init; } = new(); // ── Pending struct state ───────────────────────────────────────── // _pendingStructLocal is set by ldloca.s for simple struct locals. diff --git a/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs b/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs index d5991309..60460836 100644 --- a/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs +++ b/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs @@ -86,7 +86,7 @@ partial class Transpiler if (inlineArrayLength is int nWrap && fieldSize >= 1) { fieldSize = nWrap * fieldSize; - _bufferFieldSizes[fieldName] = fieldSize; + _bufferFieldSizes[(typeName, fieldName)] = fieldSize; } else { @@ -94,7 +94,7 @@ partial class Transpiler if (bufLen is int n) { fieldSize = n; - _bufferFieldSizes[fieldName] = n; + _bufferFieldSizes[(typeName, fieldName)] = n; } } fields.Add((fieldName, fieldSize)); @@ -108,12 +108,13 @@ partial class Transpiler } /// - /// Maps struct field names that hold a fixed-size buffer (C# fixed byte buf[N]) + /// Maps (structType, fieldName) pairs that hold a fixed-size buffer (C# fixed byte buf[N]) /// or an [InlineArray(N)] element field to their total byte count. Used by - /// to lower buffer-indexed reads/writes. + /// to lower buffer-indexed reads/writes. Keyed by struct type + /// so that distinct structs with same-named buffer fields don't collide. /// - internal Dictionary BufferFieldSizes => _bufferFieldSizes; - readonly Dictionary _bufferFieldSizes = new(StringComparer.Ordinal); + internal Dictionary<(string StructType, string FieldName), int> BufferFieldSizes => _bufferFieldSizes; + readonly Dictionary<(string StructType, string FieldName), int> _bufferFieldSizes = new(); /// /// If the field is a fixed-size buffer (C# fixed byte buf[N]) or its value type @@ -133,10 +134,25 @@ partial class Transpiler var blob = _reader.GetBlobReader(attr.Value); blob.ReadUInt16(); // prolog 0x0001 // First arg: SerString — the type-name as a length-prefixed UTF-8 string. - blob.ReadSerializedString(); + string? elementTypeName = blob.ReadSerializedString(); int length = blob.ReadInt32(); + // We only support `fixed byte buf[N]`. Other element types (e.g. + // `fixed ushort buf[N]`) would need the length multiplied by element size and + // additional addressing work in IL2NESWriter — reject explicitly to avoid + // silently producing wrong layouts. The serialized type name is + // assembly-qualified, e.g. "System.Byte, System.Private.CoreLib, ...". + bool isByte = elementTypeName != null && + (elementTypeName == "System.Byte" || + elementTypeName.StartsWith("System.Byte,", StringComparison.Ordinal)); + if (!isByte) + { + throw new TranspileException( + $"Fixed buffer element type '{elementTypeName}' is not supported. " + + "Only `fixed byte` buffers are currently supported."); + } return length; } + catch (TranspileException) { throw; } catch { /* fall through */ } } diff --git a/src/dotnes.tasks/Utilities/Transpiler.cs b/src/dotnes.tasks/Utilities/Transpiler.cs index 1441413e..8f07ea52 100644 --- a/src/dotnes.tasks/Utilities/Transpiler.cs +++ b/src/dotnes.tasks/Utilities/Transpiler.cs @@ -407,7 +407,8 @@ public Program6502 BuildProgram6502(out ushort sizeOfMain, out ushort locals) methodWriter.SkipUntilIndex = null; var instruction = methodWriter.Instructions[i]; - var labelName = $"{methodName}_instruction_{instruction.Offset:X2}"; if (methodWriter.CurrentBlock != null) + var labelName = $"{methodName}_instruction_{instruction.Offset:X2}"; + if (methodWriter.CurrentBlock != null) methodWriter.CurrentBlock.SetNextLabel(labelName); methodWriter.RecordBlockCount(instruction.Offset); diff --git a/src/dotnes.tests/StructsTests.cs b/src/dotnes.tests/StructsTests.cs index 152f7eb3..87172a7c 100644 --- a/src/dotnes.tests/StructsTests.cs +++ b/src/dotnes.tests/StructsTests.cs @@ -170,13 +170,14 @@ unsafe struct Cluster { Assert.NotEmpty(bytes); var hex = Convert.ToHexString(bytes); - // Stores of the four constant values into successive zero-page absolute addresses. + // Stores of the four constant values, written to successive absolute RAM addresses + // (struct buffer base + 0..3). These are absolute addressing in RAM, not zero-page. Assert.Contains("A910", hex); // LDA #$10 (sprite) Assert.Contains("A905", hex); // LDA #$05 Assert.Contains("A906", hex); // LDA #$06 Assert.Contains("A907", hex); // LDA #$07 Assert.Contains("A908", hex); // LDA #$08 - // Absolute store byte (8D = STA abs) + // Absolute store byte (8D = STA abs) writing into the struct's allocated RAM (e.g. $0325+). Assert.Contains("8D", hex); // Absolute load byte (AD = LDA abs) for the read Assert.Contains("AD", hex); From e837b5c7b3b86d3fa7b33777600526c5ac0bf4fa Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 28 May 2026 11:35:26 -0500 Subject: [PATCH 4/4] Address second-round PR review comments - Rewrite HandleLdflda in IL2NESWriter.StructFields.cs to properly classify the various IL patterns (fixed buffer with/without add, inline array first-element ref, inline array element ref, Span path for runtime indexes). This also fixes a pre-existing bug where `cur.layout[0] = const` wrote the value to the wrong address because the lone ldc.i4 was being interpreted as both index and value (Roslyn elides the add when the index is 0 on a fixed buffer). - Support runtime store values: `cur.layout[idx] = local` / `ldsfld` now lower through a value scan + EmitLoadValueIntoA helper. - Narrow bare `catch { }` in Transpiler.StructAnalysis.cs to `catch (BadImageFormatException)` so genuine bugs aren't swallowed. - Extract IL2NESWriter.ConsumeSkip helper and use it from both dispatch loops so the skip-marker logic can't drift apart. - Strengthen StructsTests assertions with multi-byte hex patterns that pin down both the value and the absolute address. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Utilities/IL2NESWriter.StructFields.cs | 333 +++++++++--------- src/dotnes.tasks/Utilities/IL2NESWriter.cs | 16 + .../Utilities/Transpiler.StructAnalysis.cs | 4 +- src/dotnes.tasks/Utilities/Transpiler.cs | 8 +- src/dotnes.tests/StructsTests.cs | 52 +-- 5 files changed, 215 insertions(+), 198 deletions(-) diff --git a/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs b/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs index 8eaa9295..ae963eda 100644 --- a/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs +++ b/src/dotnes.tasks/Utilities/IL2NESWriter.StructFields.cs @@ -401,6 +401,7 @@ void HandleLdfld(string fieldName) /// the matching stind.i1 or ldind.u1 and emits a single LDA/STA /// (Absolute or AbsoluteX) at base+offset+index. /// Supported indices: constant, ldarg, ldloc, ldsfld. + /// Supported store values: constant, ldloc, ldsfld. /// void HandleLdflda(string fieldName) { @@ -428,23 +429,17 @@ void HandleLdflda(string fieldName) if (Instructions is null) throw new InvalidOperationException("HandleLdflda requires Instructions to be set"); - // Walk forward and classify each instruction until we reach stind.i1 / ldind.u1. - // Recognize: - // - second ldflda (FixedElementField) — ignore (fixed buffer) - // - call to InlineArrayElementRef / InlineArrayFirstElementRef — ignore (inline array) - // - call to InlineArrayAsSpan + stloc + ldloca.s + ... + call get_Item — Span path - // - ldc.i4.* / ldarg.* / ldloc.* / ldsfld — index expression - // - add — pointer + index (fixed buffer); ignore - // - ldc.i4.* immediately before stind.i1 — the VALUE being stored - int? indexConst = null; - int? indexLocalIdx = null; - int? indexArgIdx = null; - string? indexStaticField = null; - int? storeValueConst = null; + // First pass: locate boundary opcodes (add / inline-array call / span call) and the + // terminal stind.i1 / ldind.u1. This classifies which lowering pattern we're in and + // where the index vs value live in the IL stream. + int? addIdx = null; // `add` for fixed-buffer (base + index) + int? inlineCallIdx = null; // InlineArrayElementRef / InlineArrayFirstElementRef + bool inlineArrayHasIndex = false; // false = FirstElementRef (no explicit index) + int? spanLdlocaIdx = null; // ldloca.s for the Span temp local + int? spanGetItemIdx = null; // call Span<>.get_Item + bool spanPath = false; bool isStore = false; bool isLoad = false; - bool spanPath = false; - int addCount = 0; int endIndex = -1; for (int j = Index + 1; j < Instructions.Length; j++) @@ -458,16 +453,14 @@ void HandleLdflda(string fieldName) case ILOpCode.Call: { string? mname = Instructions[j].String; - if (mname is "InlineArrayElementRef" or "InlineArrayFirstElementRef") - continue; + if (mname is "InlineArrayElementRef") + { inlineCallIdx = j; inlineArrayHasIndex = true; continue; } + if (mname is "InlineArrayFirstElementRef") + { inlineCallIdx = j; inlineArrayHasIndex = false; continue; } if (mname is "InlineArrayAsSpan") - { - spanPath = true; - continue; - } + { spanPath = true; continue; } if (mname is "get_Item" && spanPath) - continue; - // Any other call inside this sequence is unexpected + { spanGetItemIdx = j; continue; } throw new TranspileException( $"Unexpected call '{mname}' while lowering buffer field '{fieldName}' access.", MethodName); } @@ -477,71 +470,30 @@ void HandleLdflda(string fieldName) if (spanPath) continue; throw new TranspileException($"Unexpected stloc while lowering buffer field '{fieldName}' access.", MethodName); case ILOpCode.Ldloca_s: case ILOpCode.Ldloca: - if (spanPath) continue; + if (spanPath) { spanLdlocaIdx = j; continue; } throw new TranspileException($"Unexpected ldloca while lowering buffer field '{fieldName}' access.", MethodName); case ILOpCode.Add: // Fixed-buffer lowering produces exactly one `add` for `base + index`. - // Inline-array helpers produce zero. Anything beyond one would imply - // additional arithmetic in the index expression (e.g. `buf[i + 1]`), - // which we don't yet recognise — reject explicitly to avoid silent - // miscompilation. - if (++addCount > 1) + // Anything beyond one would imply additional arithmetic in the index + // expression (e.g. `buf[i + 1]`), which we don't yet recognise — reject + // explicitly to avoid silent miscompilation. + if (addIdx != null) throw new TranspileException( $"Arithmetic in the index expression for buffer field '{fieldName}' is not supported.", MethodName); - continue; - case ILOpCode.Ldc_i4_0: indexConst ??= 0; break; - case ILOpCode.Ldc_i4_1: indexConst ??= 1; break; - case ILOpCode.Ldc_i4_2: indexConst ??= 2; break; - case ILOpCode.Ldc_i4_3: indexConst ??= 3; break; - case ILOpCode.Ldc_i4_4: indexConst ??= 4; break; - case ILOpCode.Ldc_i4_5: indexConst ??= 5; break; - case ILOpCode.Ldc_i4_6: indexConst ??= 6; break; - case ILOpCode.Ldc_i4_7: indexConst ??= 7; break; - case ILOpCode.Ldc_i4_8: indexConst ??= 8; break; - case ILOpCode.Ldc_i4_s: - case ILOpCode.Ldc_i4: - indexConst ??= Instructions[j].Integer ?? 0; - break; - case ILOpCode.Ldarg_0: indexArgIdx ??= 0; break; - case ILOpCode.Ldarg_1: indexArgIdx ??= 1; break; - case ILOpCode.Ldarg_2: indexArgIdx ??= 2; break; - case ILOpCode.Ldarg_3: indexArgIdx ??= 3; break; - case ILOpCode.Ldarg_s: case ILOpCode.Ldarg: - indexArgIdx ??= Instructions[j].Integer ?? 0; - break; - case ILOpCode.Ldloc_0: indexLocalIdx ??= 0; break; - case ILOpCode.Ldloc_1: indexLocalIdx ??= 1; break; - case ILOpCode.Ldloc_2: indexLocalIdx ??= 2; break; - case ILOpCode.Ldloc_3: indexLocalIdx ??= 3; break; - case ILOpCode.Ldloc_s: case ILOpCode.Ldloc: - indexLocalIdx ??= Instructions[j].Integer ?? 0; - break; - case ILOpCode.Ldsfld: - indexStaticField ??= Instructions[j].String; - break; - case ILOpCode.Conv_u: case ILOpCode.Conv_i: case ILOpCode.Conv_u1: - case ILOpCode.Conv_i1: case ILOpCode.Conv_u2: case ILOpCode.Conv_i2: - case ILOpCode.Conv_u4: case ILOpCode.Conv_i4: - case ILOpCode.Nop: + addIdx = j; continue; case ILOpCode.Stind_i1: - // The instruction immediately before stind.i1 is the VALUE. isStore = true; endIndex = j; - // The value should already be the most recently seen ldc.i4 (and we - // tracked it in indexConst because of the ??=). But we need to disambiguate: - // for stind.i1 with constant index, the order is: - // [add] stind.i1 - // So the LAST ldc.i4 seen is actually the value, and the FIRST ldc.i4 was - // the index. We re-scan to extract these correctly. break; case ILOpCode.Ldind_u1: isLoad = true; endIndex = j; break; + // Everything else (ldc, ldarg, ldloc, ldsfld, conv, nop) is value/index data — + // we extract it precisely in the second pass below. default: - throw new TranspileException( - $"Unsupported opcode '{op}' while lowering buffer field '{fieldName}' access.", MethodName); + continue; } if (endIndex >= 0) break; } @@ -550,84 +502,98 @@ void HandleLdflda(string fieldName) throw new TranspileException( $"Could not find matching stind.i1/ldind.u1 for buffer field '{fieldName}'.", MethodName); - // Re-scan precisely to separate index from value for the store case. - // Pattern shapes we accept: - // Fixed buffer store: [ldflda inner] [add] stind.i1 - // InlineArray store: call InlineArrayElementRef stind.i1 - // InlineArray store 0: call InlineArrayFirstElementRef stind.i1 - // Fixed buffer load: [ldflda inner] [add] ldind.u1 - // InlineArray load: call InlineArrayElementRef ldind.u1 - // InlineArray load 0: call InlineArrayFirstElementRef ldind.u1 - // InlineArray runtime: ldc.i4 N call InlineArrayAsSpan stloc.M - // ldloca.s M call get_Item stind.i1/ldind.u1 - indexConst = null; indexLocalIdx = null; indexArgIdx = null; indexStaticField = null; - storeValueConst = null; - - // For the Span path, the index is between ldloca.s (temp span local) and the call to get_Item. - // For other paths, the index is before "add"/call. - int valueScanStart = isStore ? endIndex - 1 : -1; - int indexBoundaryEnd; // exclusive index for the index expression scan + // Determine the index-expression range and whether an explicit index exists. + // Pattern shapes: + // Fixed buffer + index: [ldflda inner] add stind.i1 + // Fixed buffer no index: [ldflda inner] stind.i1 (implicit 0) + // InlineArray + index: call InlineArrayElementRef stind.i1 + // InlineArray no index: call InlineArrayFirstElementRef stind.i1 (implicit 0) + // Span path (runtime): ldc.i4 N call InlineArrayAsSpan stloc M + // ldloca.s M call get_Item stind/ldind + bool hasExplicitIndex; + int indexScanStart = -1, indexScanEnd = -1; + int valueScanFrom; if (spanPath) { - // Find the call to get_Item; index is between the previous ldloca.s and that call. - int callItemIdx = -1, ldlocaIdx = -1; - for (int k = Index + 1; k < endIndex; k++) + if (spanGetItemIdx is null || spanLdlocaIdx is null) + throw new TranspileException( + $"InlineArray runtime-index pattern for '{fieldName}' was not recognised.", MethodName); + hasExplicitIndex = true; + indexScanStart = spanLdlocaIdx.Value + 1; + indexScanEnd = spanGetItemIdx.Value; + valueScanFrom = endIndex - 1; + } + else if (inlineCallIdx != null) + { + hasExplicitIndex = inlineArrayHasIndex; + if (hasExplicitIndex) { - if (Instructions[k].OpCode == ILOpCode.Call && Instructions[k].String == "get_Item") - { callItemIdx = k; break; } - if (Instructions[k].OpCode is ILOpCode.Ldloca_s or ILOpCode.Ldloca) - ldlocaIdx = k; + indexScanStart = Index + 1; + indexScanEnd = inlineCallIdx.Value; } - if (callItemIdx < 0 || ldlocaIdx < 0) + valueScanFrom = endIndex - 1; + } + else + { + // Fixed-buffer path + hasExplicitIndex = addIdx != null; + if (hasExplicitIndex) + { + indexScanStart = Index + 1; + indexScanEnd = addIdx!.Value; + } + valueScanFrom = endIndex - 1; + } + + // Resolve the index expression. + int? indexConst = null; + int? indexLocalIdx = null; + int? indexArgIdx = null; + string? indexStaticField = null; + if (hasExplicitIndex) + { + ScanForIndexExpr(indexScanStart, indexScanEnd, + out indexConst, out indexLocalIdx, out indexArgIdx, out indexStaticField); + bool foundIdx = indexConst != null || indexLocalIdx != null || indexArgIdx != null || indexStaticField != null; + if (!foundIdx) throw new TranspileException( - $"InlineArray runtime-index pattern for '{fieldName}' was not recognised.", MethodName); - indexBoundaryEnd = callItemIdx; - // Scan range for index: [ldlocaIdx+1, callItemIdx) - ScanForIndexAndValue(ldlocaIdx + 1, indexBoundaryEnd, isStore ? valueScanStart : -1, - out indexConst, out indexLocalIdx, out indexArgIdx, out indexStaticField, - out storeValueConst); + $"Could not determine index expression for buffer field '{fieldName}'.", MethodName); } else { - indexBoundaryEnd = endIndex; // up to but not including stind/ldind - ScanForIndexAndValue(Index + 1, indexBoundaryEnd, isStore ? valueScanStart : -1, - out indexConst, out indexLocalIdx, out indexArgIdx, out indexStaticField, - out storeValueConst); + indexConst = 0; } - // Determine addressing bool runtimeIdx = indexLocalIdx != null || indexArgIdx != null || indexStaticField != null; - // Validate that we actually recognised the index expression. Falling back to "index 0" - // would silently miscompile if the IL pattern changes or an unsupported form appears. - if (!runtimeIdx && indexConst is null) - throw new TranspileException( - $"Could not determine index expression for buffer field '{fieldName}'.", MethodName); - - // Pop IL evaluation stack entries that the consumed instructions pushed. - // For store: the stind.i1 pops
and . The address was conceptually - // pushed by the outer ldflda. Our existing dispatch hasn't actually pushed anything - // for ldflda (we're handling it right now), and we haven't dispatched any of the - // skipped instructions, so we don't need to touch the IL stack — neither the index - // nor the value was visibly pushed. - + // Resolve the value (for stores). + int? storeValueConst = null; + int? storeValueLocalIdx = null; + string? storeValueStaticField = null; if (isStore) { - // Value to store - if (storeValueConst is null) + ScanForValueExpr(valueScanFrom, + out storeValueConst, out storeValueLocalIdx, out storeValueStaticField); + if (storeValueConst is null && storeValueLocalIdx is null && storeValueStaticField is null) throw new TranspileException( - $"Could not determine value being stored to buffer field '{fieldName}'.", MethodName); + $"Could not determine value being stored to buffer field '{fieldName}'. " + + "Only constants, locals, and static fields are supported as buffer store values; " + + "method arguments and expression results are not yet implemented.", MethodName); + } + if (isStore) + { if (!runtimeIdx) { ushort target = (ushort)(bufferBase + indexConst!.Value); - Emit(Opcode.LDA, AddressMode.Immediate, (byte)(storeValueConst.Value & 0xFF)); + EmitLoadValueIntoA(storeValueConst, storeValueLocalIdx, storeValueStaticField); Emit(Opcode.STA, AddressMode.Absolute, target); } else { + // Order matters: load X (index) first, then A (value), so the LDA doesn't clobber X. EmitLoadIndexIntoX(indexLocalIdx, indexArgIdx, indexStaticField); - Emit(Opcode.LDA, AddressMode.Immediate, (byte)(storeValueConst.Value & 0xFF)); + EmitLoadValueIntoA(storeValueConst, storeValueLocalIdx, storeValueStaticField); Emit(Opcode.STA, AddressMode.AbsoluteX, bufferBase); } _runtimeValueInA = false; @@ -656,16 +622,16 @@ void HandleLdflda(string fieldName) previous = isStore ? ILOpCode.Stind_i1 : ILOpCode.Ldind_u1; } - void ScanForIndexAndValue(int scanStart, int scanEndExclusive, int valueScanFrom, - out int? indexConst, out int? indexLocalIdx, out int? indexArgIdx, out string? indexStaticField, - out int? storeValueConst) + /// + /// Scans an IL window for the first instruction that produces an index value + /// (constant, local, argument, or static field). + /// + void ScanForIndexExpr(int scanStart, int scanEndExclusive, + out int? indexConst, out int? indexLocalIdx, out int? indexArgIdx, out string? indexStaticField) { indexConst = null; indexLocalIdx = null; indexArgIdx = null; indexStaticField = null; - storeValueConst = null; - if (Instructions is null) return; - // Scan for the index: first integer-producing instruction in [scanStart, scanEndExclusive). for (int k = scanStart; k < scanEndExclusive; k++) { var op = Instructions[k].OpCode; @@ -677,51 +643,90 @@ void ScanForIndexAndValue(int scanStart, int scanEndExclusive, int valueScanFrom ILOpCode.Ldc_i4 or ILOpCode.Ldc_i4_s => Instructions[k].Integer ?? 0, _ => (int?)null, }; - if (c != null) { indexConst = c; break; } + if (c != null) { indexConst = c; return; } switch (op) { - case ILOpCode.Ldarg_0: indexArgIdx = 0; break; - case ILOpCode.Ldarg_1: indexArgIdx = 1; break; - case ILOpCode.Ldarg_2: indexArgIdx = 2; break; - case ILOpCode.Ldarg_3: indexArgIdx = 3; break; + case ILOpCode.Ldarg_0: indexArgIdx = 0; return; + case ILOpCode.Ldarg_1: indexArgIdx = 1; return; + case ILOpCode.Ldarg_2: indexArgIdx = 2; return; + case ILOpCode.Ldarg_3: indexArgIdx = 3; return; case ILOpCode.Ldarg_s: case ILOpCode.Ldarg: - indexArgIdx = Instructions[k].Integer ?? 0; break; - case ILOpCode.Ldloc_0: indexLocalIdx = 0; break; - case ILOpCode.Ldloc_1: indexLocalIdx = 1; break; - case ILOpCode.Ldloc_2: indexLocalIdx = 2; break; - case ILOpCode.Ldloc_3: indexLocalIdx = 3; break; + indexArgIdx = Instructions[k].Integer ?? 0; return; + case ILOpCode.Ldloc_0: indexLocalIdx = 0; return; + case ILOpCode.Ldloc_1: indexLocalIdx = 1; return; + case ILOpCode.Ldloc_2: indexLocalIdx = 2; return; + case ILOpCode.Ldloc_3: indexLocalIdx = 3; return; case ILOpCode.Ldloc_s: case ILOpCode.Ldloc: - indexLocalIdx = Instructions[k].Integer ?? 0; break; + indexLocalIdx = Instructions[k].Integer ?? 0; return; case ILOpCode.Ldsfld: - indexStaticField = Instructions[k].String; break; - default: continue; + indexStaticField = Instructions[k].String; return; } - break; } + } + + /// + /// Scans backwards from for the value being stored: + /// constant, local, or static field. Skips pass-through conversions. + /// + void ScanForValueExpr(int valueScanFrom, + out int? storeValueConst, out int? storeValueLocalIdx, out string? storeValueStaticField) + { + storeValueConst = null; storeValueLocalIdx = null; storeValueStaticField = null; + if (Instructions is null || valueScanFrom < 0) return; - // Scan for the value (last constant immediately before stind.i1). - if (valueScanFrom >= 0) + for (int k = valueScanFrom; k >= 0; k--) { - for (int k = valueScanFrom; k > 0; k--) + var op = Instructions[k].OpCode; + int? c = op switch { - var op = Instructions[k].OpCode; - int? c = op switch - { - ILOpCode.Ldc_i4_0 => 0, ILOpCode.Ldc_i4_1 => 1, ILOpCode.Ldc_i4_2 => 2, - ILOpCode.Ldc_i4_3 => 3, ILOpCode.Ldc_i4_4 => 4, ILOpCode.Ldc_i4_5 => 5, - ILOpCode.Ldc_i4_6 => 6, ILOpCode.Ldc_i4_7 => 7, ILOpCode.Ldc_i4_8 => 8, - ILOpCode.Ldc_i4 or ILOpCode.Ldc_i4_s => Instructions[k].Integer ?? 0, - _ => (int?)null, - }; - if (c != null) { storeValueConst = c; return; } - // Skip pass-through conversions that don't change the value - if (op is ILOpCode.Conv_u1 or ILOpCode.Conv_i1 or ILOpCode.Nop) continue; - // Stop search at first non-skip instruction - break; + ILOpCode.Ldc_i4_0 => 0, ILOpCode.Ldc_i4_1 => 1, ILOpCode.Ldc_i4_2 => 2, + ILOpCode.Ldc_i4_3 => 3, ILOpCode.Ldc_i4_4 => 4, ILOpCode.Ldc_i4_5 => 5, + ILOpCode.Ldc_i4_6 => 6, ILOpCode.Ldc_i4_7 => 7, ILOpCode.Ldc_i4_8 => 8, + ILOpCode.Ldc_i4 or ILOpCode.Ldc_i4_s => Instructions[k].Integer ?? 0, + _ => (int?)null, + }; + if (c != null) { storeValueConst = c; return; } + switch (op) + { + case ILOpCode.Ldloc_0: storeValueLocalIdx = 0; return; + case ILOpCode.Ldloc_1: storeValueLocalIdx = 1; return; + case ILOpCode.Ldloc_2: storeValueLocalIdx = 2; return; + case ILOpCode.Ldloc_3: storeValueLocalIdx = 3; return; + case ILOpCode.Ldloc_s: case ILOpCode.Ldloc: + storeValueLocalIdx = Instructions[k].Integer ?? 0; return; + case ILOpCode.Ldsfld: + storeValueStaticField = Instructions[k].String; return; + case ILOpCode.Conv_u1: case ILOpCode.Conv_i1: case ILOpCode.Nop: + continue; + default: + return; } } } + /// + /// Emits the LDA that loads the value (constant or runtime) into A for a buffer store. + /// + void EmitLoadValueIntoA(int? constVal, int? localIdx, string? staticField) + { + if (constVal != null) + { + Emit(Opcode.LDA, AddressMode.Immediate, (byte)(constVal.Value & 0xFF)); + return; + } + if (localIdx != null && Locals.TryGetValue(localIdx.Value, out var lcl) && lcl.Address is int la) + { + Emit(Opcode.LDA, AddressMode.Absolute, (ushort)la); + return; + } + if (staticField != null && Variables.StaticFieldAddresses.TryGetValue(staticField, out var sfa)) + { + Emit(Opcode.LDA, AddressMode.Absolute, sfa); + return; + } + throw new TranspileException("Could not resolve buffer store value to an addressable value.", MethodName); + } + void EmitLoadIndexIntoX(int? localIdx, int? argIdx, string? staticField) { if (localIdx != null && Locals.TryGetValue(localIdx.Value, out var lcl) && lcl.Address is int la) diff --git a/src/dotnes.tasks/Utilities/IL2NESWriter.cs b/src/dotnes.tasks/Utilities/IL2NESWriter.cs index f35450b6..99588be6 100644 --- a/src/dotnes.tasks/Utilities/IL2NESWriter.cs +++ b/src/dotnes.tasks/Utilities/IL2NESWriter.cs @@ -431,6 +431,22 @@ bool _dupPendingSave ///
internal int? SkipUntilIndex { get; set; } + /// + /// Returns true if the current dispatch iteration at + /// should be skipped due to a multi-instruction handler having claimed it. Side effect: + /// clears once the skip window is past. Centralises the + /// skip-loop logic so the main and user-method dispatch loops can't drift apart. + /// + internal bool ConsumeSkip(int instructionIndex) + { + if (SkipUntilIndex is not int sk) + return false; + if (instructionIndex < sk) + return true; + SkipUntilIndex = null; + return false; + } + /// /// The IL offset where execution should resume after the current finally handler. /// Set by Leave/Leave_s, consumed by Endfinally. diff --git a/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs b/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs index 60460836..9bb310b3 100644 --- a/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs +++ b/src/dotnes.tasks/Utilities/Transpiler.StructAnalysis.cs @@ -153,7 +153,7 @@ partial class Transpiler return length; } catch (TranspileException) { throw; } - catch { /* fall through */ } + catch (BadImageFormatException) { /* malformed attribute blob; ignore */ } } // 2. Inspect the field's signature: if it is a VALUETYPE referencing a TypeDef @@ -214,7 +214,7 @@ partial class Transpiler int length = blob.ReadInt32(); return length; } - catch { /* fall through */ } + catch (BadImageFormatException) { /* malformed attribute blob; ignore */ } } return null; } diff --git a/src/dotnes.tasks/Utilities/Transpiler.cs b/src/dotnes.tasks/Utilities/Transpiler.cs index 8f07ea52..b78f5485 100644 --- a/src/dotnes.tasks/Utilities/Transpiler.cs +++ b/src/dotnes.tasks/Utilities/Transpiler.cs @@ -300,10 +300,8 @@ public Program6502 BuildProgram6502(out ushort sizeOfMain, out ushort locals) for (int i = 0; i < writer.Instructions.Length; i++) { writer.Index = i; - if (writer.SkipUntilIndex is int sk && i < sk) + if (writer.ConsumeSkip(i)) continue; - if (writer.SkipUntilIndex is int sk2 && i >= sk2) - writer.SkipUntilIndex = null; var instruction = writer.Instructions[i]; // Record IL instruction labels for branch targets @@ -401,10 +399,8 @@ public Program6502 BuildProgram6502(out ushort sizeOfMain, out ushort locals) for (int i = 0; i < methodWriter.Instructions.Length; i++) { methodWriter.Index = i; - if (methodWriter.SkipUntilIndex is int sk && i < sk) + if (methodWriter.ConsumeSkip(i)) continue; - if (methodWriter.SkipUntilIndex is int sk2 && i >= sk2) - methodWriter.SkipUntilIndex = null; var instruction = methodWriter.Instructions[i]; var labelName = $"{methodName}_instruction_{instruction.Offset:X2}"; diff --git a/src/dotnes.tests/StructsTests.cs b/src/dotnes.tests/StructsTests.cs index 87172a7c..c79d10c8 100644 --- a/src/dotnes.tests/StructsTests.cs +++ b/src/dotnes.tests/StructsTests.cs @@ -170,17 +170,16 @@ unsafe struct Cluster { Assert.NotEmpty(bytes); var hex = Convert.ToHexString(bytes); - // Stores of the four constant values, written to successive absolute RAM addresses - // (struct buffer base + 0..3). These are absolute addressing in RAM, not zero-page. - Assert.Contains("A910", hex); // LDA #$10 (sprite) - Assert.Contains("A905", hex); // LDA #$05 - Assert.Contains("A906", hex); // LDA #$06 - Assert.Contains("A907", hex); // LDA #$07 - Assert.Contains("A908", hex); // LDA #$08 - // Absolute store byte (8D = STA abs) writing into the struct's allocated RAM (e.g. $0325+). - Assert.Contains("8D", hex); - // Absolute load byte (AD = LDA abs) for the read - Assert.Contains("AD", hex); + // Cluster: sprite@$0325, id@$0326, layout[0..3]@$0327..$032A + // Each store is LDA #imm (A9 imm) + STA absolute (8D lo hi). + Assert.Contains("A9108D2503", hex); // sprite = 0x10 -> $0325 + Assert.Contains("A9018D2603", hex); // id = 1 -> $0326 + Assert.Contains("A9058D2703", hex); // layout[0] = 5 -> $0327 + Assert.Contains("A9068D2803", hex); // layout[1] = 6 -> $0328 + Assert.Contains("A9078D2903", hex); // layout[2] = 7 -> $0329 + Assert.Contains("A9088D2A03", hex); // layout[3] = 8 -> $032A + // Read of cur.layout[2] for the pal_col call: LDA $0329 + Assert.Contains("AD2903", hex); } [Fact] @@ -206,11 +205,10 @@ unsafe struct Cluster { Assert.NotNull(bytes); Assert.NotEmpty(bytes); var hex = Convert.ToHexString(bytes); - Assert.Contains("A963", hex); // LDA #$63 (99) - // STA absolute,X (9D) for AbsoluteX store - Assert.Contains("9D", hex); - // LDX absolute (AE) to load the runtime index - Assert.Contains("AE", hex); + // Layout: i@$0325, cur.sprite@$0326, cur.id@$0327, cur.layout[0..3]@$0328..$032B + Assert.Contains("A9028D2503", hex); // i = 2 -> $0325 + Assert.Contains("AE2503", hex); // LDX $0325 (load index i) + Assert.Contains("A9639D2803", hex); // LDA #99; STA $0328,X (layout[i] = 99) } [Fact] @@ -242,13 +240,14 @@ struct Cluster { Assert.NotNull(bytes); Assert.NotEmpty(bytes); var hex = Convert.ToHexString(bytes); - Assert.Contains("A910", hex); - Assert.Contains("A905", hex); - Assert.Contains("A906", hex); - Assert.Contains("A907", hex); - Assert.Contains("A908", hex); - Assert.Contains("8D", hex); - Assert.Contains("AD", hex); + // Same layout as the fixed-buffer variant: sprite@$0325, id@$0326, layout[0..3]@$0327..$032A + Assert.Contains("A9108D2503", hex); // sprite = 0x10 + Assert.Contains("A9018D2603", hex); // id = 1 + Assert.Contains("A9058D2703", hex); // layout[0] = 5 + Assert.Contains("A9068D2803", hex); // layout[1] = 6 + Assert.Contains("A9078D2903", hex); // layout[2] = 7 + Assert.Contains("A9088D2A03", hex); // layout[3] = 8 + Assert.Contains("AD2903", hex); // LDA $0329 (read layout[2]) } [Fact] @@ -275,8 +274,9 @@ struct Cluster { Assert.NotNull(bytes); Assert.NotEmpty(bytes); var hex = Convert.ToHexString(bytes); - Assert.Contains("A963", hex); // LDA #$63 (99) - Assert.Contains("9D", hex); // STA abs,X - Assert.Contains("AE", hex); // LDX abs (runtime index) + // Layout: i@$0325, cur.sprite@$0326, cur.id@$0327, cur.layout[0..3]@$0328..$032B + Assert.Contains("A9018D2503", hex); // i = 1 -> $0325 + Assert.Contains("AE2503", hex); // LDX $0325 (load index i) + Assert.Contains("A9639D2803", hex); // LDA #99; STA $0328,X (layout[i] = 99) } }