joeylib2/PERF-AUDIT.md

174 KiB

JoeyLib Core Audit -- 2026-07-04

Deep audit of src/core/, src/generic/, include/joey/ (plus codegen and hot spots the completeness critic pulled in from src/iigs, src/m68k, src/atarist). Method: 7 lens-specific finder agents + a 3-gap critic round, every finding adversarially verified against the actual file on three lenses (factual accuracy, fix validity on real hardware, materiality). 77 raw findings -> 71 confirmed, 6 refuted. Where the verifier tightened a claim, the correction is included -- read it before acting on a finding.

Severity: critical = correctness bug or >2x slowdown on a hot op; high = measurable per-op win (>10%) or real bug on a cold path; medium = clear cleanup or small win; low = style-level.

Confirmed: 2 critical, 22 high, 32 medium, 15 low

Read this first: the benchmark inflates every small-op number

The UBER loop (examples/uber/uber.c:66) calls jlFrameCount() between every op iteration. On the IIgs that is a THREE-layer chain -- jlFrameCount -> jlpFrameCount -> iigsGetTickWord -> Misc Toolset GetTick ($2503) via the $E10000 tool dispatcher. The toolbox dispatch alone is several hundred cycles; with the forwarding layers, the indirect op() call, and the op wrapper, each benchmark iteration carries roughly 600-800 cycles of pure harness tax. The uber.c comment claiming the poll costs "~10-30 cyc per op" is wrong by more than an order of magnitude on the IIgs.

Cross-check: jlKeyDown is a bounds check plus an array read (~80 cycles including the call), yet it measures 3382 ops/sec = ~830 cycles/iter. The missing ~750 cycles are the harness. The same constant sits inside EVERY small-op number in PERF.md: jlDrawPixel's honest cost is roughly half of what the table implies, jlSamplePixel/jlPaletteSet/input predicates are 3-10x faster than their entries suggest, and the cross-port ratios for fast ops partly measure each port's clock-read cost, not library code.

Fix in uber.c, not in the library: poll the clock every N iterations (e.g. run the op 8-16 times per while-check and count in batches), or calibrate an iteration count first and time a fixed batch. Then re-baseline PERF.md. Until that lands, treat full-screen ops (which amortize the tax) as honest and every op above ~500 ops/sec as significantly under-reported. Finding 16 (jlFrameCount double pass-through) trims the library-side share; the toolbox dispatch cost itself is only avoidable by polling less often.

Index

  1. [FIXED 2026-07-05] [bug] src/core/draw.c:549 -- jlDrawPixel (non-IIgs) marks dirty with unclipped x/y, causing out-of-bounds writes to the dirty-band arrays
  2. [CRITICAL] [perf] src/core/sprite.c:193 -- Interpreted sprite inner loop re-derives dst byte address and parity per pixel via a real function call
  3. [HIGH ] [perf] include/joey/platform.h:98 -- IIgs (perf reference) silently runs the generic C nibble loop for jlTilePasteMono -- no JL_HAS_TILE_PASTE_MONO, no port.h macro
  4. [HIGH ] [perf] include/joey/platform.h:205 -- Atari ST defines neither JL_HAS_FLOOD_WALK_PLANES nor JL_HAS_FLOOD_SCAN_ROW_PLANES -- flood fill decodes 4 bitplanes per pixel via a function call
  5. [FIXED 2026-07-05: compact-and-retry + coreSetError] [perf] src/codegen/spriteCompile.c:202 -- jlSpriteCompile ignores the arena's compact-and-retry contract: one fragmented alloc failure silently demotes the sprite to the interpreter forever
  6. [HIGH ] [perf] src/core/audioSfxMix.c:83 -- SFX mixer does a full 32x32 multiply per output sample; on 68000 this is a __mulsi3 library call
  7. [HIGH ] [perf] src/core/audioSfxMix.c:110 -- 64-bit posFp arithmetic executed per sample in the mixer inner loop
  8. [FIXED 2026-07-05] [bug] src/core/codegenArena.c:236 -- IIgs 64KB attrNoCross cap on the codegen arena is unenforced; codegenBytes > 64KB hard-fails jlInit (adventure.c passes 256KB)
  9. [HIGH ] [perf] src/core/draw.c:484 -- jlDrawLine Bresenham fallback accumulates the dirty bounding box with 4 compares per pixel although it equals the endpoint box
  10. [HIGH ] [perf] src/core/draw.c:533 -- IIgs stage-identity test is a cross-TU jlStageGet() function call per primitive (per pixel on the hottest op) and is re-checked again inside surfaceMarkDirtyRect
  11. [FIXED 2026-07-05, incl. the #65 h==2 companion] [bug] src/core/draw.c:591 -- jlDrawRect general path wraps w/h > 32767 through (int16_t) casts, drawing spurious edges on-surface
  12. [FIXED 2026-07-05; bonus: HEAD also INFINITE-LOOPED at exactly r==32767 (int16 tautology), fixed by the same rewrite] [bug] src/core/draw.c:656 -- jlFillCircle (and jlDrawCircle fallback) cast r to int16_t: r > 32767 draws nothing; r in [~380, 32767] iterates up to 32k clipped spans
  13. [HIGH ] [perf] src/core/draw.c:703 -- jlFillRect runs 32-bit clip math on every call; the common on-surface case needs only 16-bit compares
  14. [HIGH ] [perf] src/core/draw.c:776 -- jlSamplePixel pays a second cross-TU function hop for a ~30-cycle nibble read on chunky ports
  15. [HIGH ] [perf] src/core/init.c:148 -- jlFrameCount is a double pass-through on IIgs: jlFrameCount -> jlpFrameCount -> iigsGetTickWord, paid every benchmark iteration and every game-loop pacing check
  16. [HIGH ] [perf] src/core/palette.c:106 -- jlPaletteSet re-masks all 15 caller entries against the documented $0RGB contract instead of memcpy + forcing color 0
  17. [HIGH ] [perf] src/core/port.h:116 -- jlStageGet() function call buried in hot-path port.h macros and jlDrawPixel costs a cross-TU hop per primitive
  18. [HIGH ] [perf] src/core/sprite.c:189 -- No opaque-pair whole-byte fast path; 5 conditional branches per pixel in the interpreted draw loop
  19. [HIGH ] [perf] src/core/sprite.c:524 -- Save/restore interpreted fallbacks bypass SURFACE_ROW_OFFSET and pay two software multiplies per row
  20. [HIGH ] [perf] src/core/surface.c:234 -- surfaceMarkDirtyRect is a cross-TU function hop on every drawing op, re-deriving stage-ness the dispatch macro just computed
  21. [HIGH ] [perf] src/core/surface.c:317 -- stageDirtyClearAll runs a 200-iteration byte loop every present while its sibling uses memset for the same arrays
  22. [HIGH ] [perf] src/core/tile.c:54 -- jlDrawText pays the full public jlTileCopyMasked wrapper plus a multi-row dirty-mark call per glyph
  23. [FIXED 2026-07-05; check-millis.sh is the regression gate] [bug] src/generic/genericPort.c:40 -- jlpGenericMillisElapsed wraps backward every ~18 minutes and does a 32-bit multiply+divide per call on IIgs/Amiga
  24. [HIGH ] [perf] src/generic/genericSurface.c:55 -- jlpGenericSamplePixel pays a software multiply (y*160) instead of the SURFACE_ROW_OFFSET LUT the codebase built for exactly this
  25. [MEDIUM ] [multi-source-of-truth] PERF.md:38 -- PERF.md IIgs baseline predates the Phase-2 jlDrawPixel/dirty-mark rewrite; all cross-port ratios are computed against a stale floor
  26. [MEDIUM ] [multi-source-of-truth] include/joey/platform.h:100 -- Eleven IIgs JL_HAS flags are decorative -- the port.h macro overrides bypass the registry, so entries can be added/removed with zero effect
  27. [MEDIUM ] [dead-code] src/codegen/spriteCompile.c:246 -- Amiga compiled save/restore dispatchers and the shift-alias loop are dead code contradicted by the emitters
  28. [MEDIUM ] [perf] src/codegen/spriteCompile.c:311 -- IIgs spriteCompiledDraw still pays the 2D-array multiply helper the same file dodges everywhere else
  29. [MEDIUM ] [duplication] src/codegen/spriteCompile.c:423 -- Dispatcher gate and spriteCompiled* callees each re-derive shift, route offset, and function address per call
  30. [MEDIUM ] [multi-source-of-truth] src/core/assetLoad.c:41 -- assetLoad.c re-spells TILE_BYTES and the 40x25 tile-grid limits as private literals
  31. [FIXED 2026-07-04 during the stabilization cluster] [bug] src/core/assetLoad.c:167 -- jlSpriteBankLoad allocates up to 32000-byte cels with malloc, violating core.h's own IIgs allocator contract
  32. [MEDIUM ] [duplication] src/core/assetLoad.c:180 -- jlSpriteBankLoad re-implements spriteInitFields field by field, and the spriteInternal.h comment contradicts it
  33. [MEDIUM ] [perf] src/core/audioSfxMix.c:131 -- Mixer re-reads slot fields through the pointer every sample because dst stores are char-typed and may alias
  34. [FIXED 2026-07-05 via the sprite.c registry + spriteSystemShutdown] [bug] src/core/codegenArena.c:294 -- codegenArenaShutdown frees every ArenaSlotT while live sprites still hold pointers; shutdown -> re-init -> jlSpriteDestroy corrupts the heap
  35. [MEDIUM ] [perf] src/core/draw.c:168 -- floodFillInternal C fallback evaluates the loop-invariant matchEqual ternary and a jlpSamplePixel function call per pixel
  36. [MEDIUM ] [unnecessary-conditional] src/core/draw.c:345 -- jlDrawCircle fallback runs 8 bounding-box compares per iteration that are provably dead after the first iteration
  37. [MEDIUM ] [unnecessary-conditional] src/core/draw.c:484 -- Bresenham fallbacks accumulate a bounding box per pixel that is statically known before the loop
  38. [MEDIUM ] [perf] src/core/draw.c:668 -- jlFillCircle C span loop pays surfaceMarkDirtyRect (plus IIgs jlStageGet) per span instead of one bounding-box mark
  39. [MEDIUM ] [perf] src/core/init.c:20 -- DEFAULT_CODEGEN_BYTES (8KB) is sized to IIgs code density; a single 32x32 sprite's ST/Amiga DRAW routines nearly fill or overflow it, silently forcing the interpreter
  40. [MEDIUM ] [perf] src/core/input.c:92 -- jlKeyDown's real cost is ~5x its body: cross-TU call overhead around a two-load predicate
  41. [MEDIUM ] [perf] src/core/port.h:130 -- port.h comment says 'the asm derives the fill word' but the jlpTileFill/jlpFillCircle macros compute (c & 0x0F) * 0x1111 in C -- a software multiply helper per call on the 65816
  42. [MEDIUM ] [perf] src/core/port.h:179 -- DOS pays real cross-TU calls to no-op planar hooks on every sprite draw/save/restore and surface copy -- port.h's own comment admits it
  43. [MEDIUM ] [perf] src/core/random.c:23 -- jlRandom's 32-bit shift triple (13/17/5) is a worst case for the 16-bit 65816; bit-identical 16-bit-half rewrite available
  44. [MEDIUM ] [perf] src/core/sprite.c:103 -- isFullyOnSurface uses 32-bit adds/compares on every hot sprite entry gate, against the file's own stated convention
  45. [MEDIUM ] [perf] src/core/sprite.c:179 -- spriteDrawInterpreted row setup redoes a per-row multiply and recomputes loop-invariant column math every row
  46. [FIXED 2026-07-05] [bug] src/core/sprite.c:346 -- IIgs/DOS draw fast path never checks SPRITE_NOT_COMPILED before jumping into the arena
  47. [MEDIUM ] [multi-source-of-truth] src/core/surfaceInternal.h:54 -- Four divergent implementations of the 'row is dirty' test; the STAGE_DIRTY_ROW_CLEAN macro that should be the single truth is unused
  48. [MEDIUM ] [perf] src/core/tile.c:79 -- tile.c wrappers derive pixel coords with * TILE_PIXELS_PER_SIDE multiplies where port.h deliberately uses shifts to dodge the ORCA-C multiply helper
  49. [MEDIUM ] [perf] src/generic/genericDraw.c:42 -- jlpGenericFillRect recomputes loop-invariant span bytes/edge masks and the row offset every row
  50. [MEDIUM ] [duplication] src/generic/genericTile.c:27 -- Identical 4-byte row-copy inner loop triplicated byte-at-a-time across TileCopy, TilePaste, and TileSnap
  51. [MEDIUM ] [perf] src/generic/genericTile.c:59 -- jlpGenericTileCopyMasked does a read-modify-write merge even when both source nibbles are opaque
  52. [MEDIUM ] [dead-code] src/iigs/hal.c:298 -- iigsBlitStageToShr's SCB/palette MVN upload paths are dead, yet jlpPresent still marshals both far pointers into the asm every present
  53. [MEDIUM ] [perf] src/iigs/joeyDraw.s:1416 -- iigsDrawPixelInner carries dead PHB/PLB framing and a long-absolute scratch round-trip in the hottest inner op
  54. [MEDIUM ] [multi-source-of-truth] src/iigs/spriteEmitIigs.c:3 -- spriteEmitIigs.c top-of-file contract describes the deleted self-modifying-stub design; the code below it emits the opposite (C-ABI prologue)
  55. [MEDIUM ] [dead-code] src/m68k/spriteEmit68k.c:179 -- spriteEmit68k.c (chunky 68k emitter) is compiled into Amiga and ST builds but nothing calls it
  56. [MEDIUM ] [duplication] src/m68k/spriteEmitInterleaved68k.c:83 -- c2pColumn/putWord duplicated byte-for-byte between the two 68k emitters; shiftedByteAt/spriteSourceByte duplicated between IIgs and x86 emitters
  57. [LOW ] [cleanup] include/joey/surface.h:66 -- surface.h documents jlSurfaceHash as FNV-1a but the implementation is a 31/251 two-stream product hash
  58. [FIXED 2026-07-05, incl. the jlpTileSnap twin] [unnecessary-conditional] src/atarist/hal.c:1410 -- ST jlpTilePasteMono dereferences dst before its own NULL check, and both the dst and duplicate pd NULL checks are dead
  59. [LOW ] [perf] src/codegen/spriteCompile.c:362 -- SPLIT_BACKUP_CACHED global single-entry cache thrashes and costs more than it saves with multiple backup buffers
  60. [LOW ] [multi-source-of-truth] src/core/assetLoad.c:28 -- assetLoad.c comment claims file-IO lives in a named IIgs segment, but no segment directive exists anywhere
  61. [LOW ] [unnecessary-conditional] src/core/draw.c:276 -- plotPixelNoMark re-checks s == NULL per pixel although every caller has already validated it
  62. [LOW ] [duplication] src/core/draw.c:574 -- isFullyOnSurface predicate duplicated between sprite.c helper and jlDrawRect's inline test
  63. [LOW ] [multi-source-of-truth] src/core/sprite.c:344 -- jlSpriteDraw's fast-path comment claims the IIgs never takes it, contradicting the codegen default
  64. [LOW ] [duplication] src/core/sprite.c:409 -- routineOffsets shift*3+op byte-offset idiom hand-expanded at three sites in sprite.c
  65. [LOW ] [unnecessary-conditional] src/core/surface.c:245 -- surfaceMarkDirtyRect re-validates w/h that its contract already guarantees positive
  66. [LOW ] [dead-code] src/core/surfaceInternal.h:36 -- SURFACE_PIXELS_PER_WORD is defined and never used anywhere in the repo
  67. [LOW ] [magic-number] src/core/surfaceInternal.h:125 -- gRowOffsetLut declared with magic literal 200 instead of SURFACE_HEIGHT
  68. [LOW ] [dead-code] src/core/tile.c:17 -- Stale comment in tile.c still claims the inner row copies are spelled out in this file
  69. [LOW ] [unnecessary-conditional] src/generic/genericTile.c:84 -- fg/bg colors are masked to 4 bits twice on the TilePasteMono path
  70. [LOW ] [duplication] src/generic/genericTile.c:142 -- jlpGenericTileFill re-derives the tile row-0 address with its own math instead of the file's GENERIC_TILE_ROW0 macro
  71. [LOW ] [cleanup] src/iigs/hal.c:358 -- Stale 15-line comment on IIgs jlpFrameCount documents a deleted $C019-polling/gFrameCount implementation; body is one GetTick call
  72. [FIXED 2026-07-05] [bug] src/core/draw.c:417 -- jlDrawLine H/V fast paths clamp the span to the surface size BEFORE clipping, so a line with a far-off-surface start draws nothing (found during Phase 0 harness work)
  73. [CRITICAL] [bug] src/dos/hal.c:321 -- DOS jlpFrameCount edge-polled $3DA and lost every frame whose ~1ms retrace pulse fell between calls; the ENTIRE old DOS PERF.md column was inflated 5-40x on slow ops (FIXED during Phase 0: now derived from the millis clock)
  74. [FIXED 2026-07-05: pass 2 now emits into the roomy scratch and memcpys into the arena, so sizing and emit agree by construction; DOS jlSpriteDraw jumped 284 -> 6094 ops/sec on the compiled path with bit-identical pixels] [bug] src/codegen/spriteCompile.c:149 -- jlSpriteCompile returns false on DOS (pre-existing at HEAD; UBER logs the failure), so every DOS sprite benchmark and game has been running the interpreter; root cause not yet isolated
  75. [FIXED 2026-07-05: both planar overrides decoded the mono tile as their PRIVATE plane-major layout instead of the contract's chunky nibble-pairs; both now fold each 4-byte chunky row into the plane shape mask] [bug] src/generic/genericTile.c:77 -- jlTilePasteMono produces THREE different pixel outputs on chunky (DOS/IIgs generic) vs Amiga vs ST overrides; caught by the Phase 0 harness the first time the op was ever hashed cross-port
  76. [CRITICAL] [bug] src/iigs/hal.c:348 -- IIgs jlpWaitVBL polled $C019's VBL bit, which never reads high under MAME's apple2gs; every program pacing with jlWaitVBL hung forever in emulation, and UBER (the only in-tree caller) could never complete headless (FIXED during Phase 0: now waits on a GetTick change)
  77. [CRITICAL] [bug] src/iigs/spriteEmitIigs.c + src/codegen/spriteCompile.c -- IIgs sprite codegen corrupts memory under the clang build: with JOEY_IIGS_SPRITE_CODEGEN=1 (the default), UBER's stage/surface state is corrupted between setupSprite and drawShowcase and the app dies in a wild memset over bank 0; with codegen disabled UBER runs. The old "#19 rewrite fixed it" claim was validated on the ORCA build, not clang
  78. [FIXED 2026-07-05: the ST fillCircle asm read its color argument at a 6-arg function's stack offset (copy-paste from lineSpan.s) and filled with a garbage color; its span math was also rewritten from midpoint-Bresenham to the exact-disk reference. jlTileFill was EXONERATED -- simulator-proven pure fallout of the fillCircle corruption] -- ST jlFillCircle and jlTileFill produce different pixels than the chunky reference (DOS): first-ever DOS-vs-ST hash comparison diverges at those two ops while Amiga matches DOS on every timed op
  79. [HIGH ] [perf] toolchains/iigs/llvm-mos/runtime/src/libc.c:191 -- clang-era IIgs links byte-at-a-time memset/memcpy (~30 cyc/byte) where the ORCA baseline used MVN (~7 cyc/byte); jlScbSetRange measures 163 ops/sec vs the ORCA-era 1005 -- every memset/memcpy-based core path regressed ~4x on the reference platform (toolchain-owned; flag to the llvm816 session)
  80. [CRITICAL] [bug] src/core/surface.c:279 + toolchain libc -- the IIgs C heap is 665 bytes of unreserved bank-0 leftover, and at exhaustion its malloc/calloc returns 1 instead of NULL (the libc's own "over-heap miscompile" comment, still live). stageAlloc's calloc(~720) returned 1; the whole port ran on a phantom stage struct at bank-0 zero page whose pixels field ALIASED the real framebuffer address by coincidence -- rendering worked while scb/palette writes scribbled over system bank 0. Root cause of the divergent IIgs hashes, the phase-5 hang, and (pending re-test) #77 (FIXED JoeyLib-side during Phase 1: surface/sprite structs + owned tileData moved to jlpBigAlloc/jlpBigFree -- finding #31 executed; toolchain asks filed)
  81. [CRITICAL] [bug] src/core/debug.c (IIgs) -- per-line log writes cost ~8 emulated seconds each through the floppy FST and can wedge indefinitely in SmartPort firmware retries (PC-sampled: slot-5 ROM spin stalled UBER 18+ emulated minutes). FIXED during Phase 1: IIgs logging now writes a RAM ring (gJoeyLogRing, drained live by the MAME Lua probes); jlLogFlush writes the ring to disk in one open/write/close pass
  82. [HIGH ] [bug] src/iigs/joeyDraw.s -- iigsDrawLineInner and iigsFillCircleInner stashed the pixels pointer with a DBR store but dereferenced it D-relative (bank 0): every plot went through a garbage pointer -- deterministic wrong pixels vs the chunky reference (FIXED during Phase 1 with the drawPixel-pattern D-relative stash)
  83. [LOW ] [bug] src/core/draw.c diagonal Bresenham -- dx/dy computed in int16_t, so endpoints more than 32767 apart (e.g. x0=-32768, x1=100) corrupt the walk and can infinite-loop. Pre-existing and latent (such inputs draw nothing useful and the H/V fast paths and endpoint-box marking are unaffected); noted during the Phase 3 review. Fix opportunistically when the fallback is next touched
  84. [LOW ] [bug] cross-port jlTileT semantics -- jlTileSnap on planar ports stores platform-private plane-major bytes in jlTileT.pixels, but jlTilePasteMono (per contract) decodes the same struct as chunky nibble-pairs: a tile snapped on ST/Amiga then mono-pasted decodes differently than on DOS/IIgs. Latent wart flagged during the #75 fix; consider a contract note or a chunky-normalizing snap in Phase 5
  85. [MEDIUM ] [bug] src/iigs/joeyDraw.s -- the odd-column nibble mask in the M=16 plot RMW was BYTE-SWAPPED ($F0FF instead of $FFF0 on the little-endian 65816): it never cleared the target nibble and erased the adjacent byte's low nibble instead. 8 sites in iigsDrawCircleInner's octant plots + 1 latent in iigsDrawLineInner. Simulation-validated root cause (Bresenham geometry was identical to C; first diff at the octant-7/8 pole pairs); fix = the one-constant change at all 9 sites, byte-identical to the C reference across 20 simulated cases (FIXED during Phase 1, pending MAME re-verification with the Wave-1 batch)
  86. [CRITICAL] [bug] src/iigs/joeyDraw.s -- the #83 garbage plot pointer is layout-dependent: when a memory shift (e.g. the #85 reservation) changed what it aliased, line plots landed ON the dln* Bresenham state, mutating the endpoint mid-line and hanging the walk in an infinite coordinate wrap that scribbles via out-of-bounds gRowOffsetLut offsets -- a wild-write generator and plausible contributor to the pre-#81 corruption (FIXED with #83)
  87. [HIGH ] [bug] src/core/sprite.c + IIgs codegen-off path -- IIgs interpreted sprite ops are pathologically slow (jlSpriteSaveUnder 1 iter/65 frames, jlSpriteDraw 1 iter/368 frames -- ~6 SECONDS for one 16x16 interpreted draw) and pixel-divergent (sprite-clip-roundtrip FAILs; all post-sprite hashes fall out). Needs its own hunt: even finding #2's per-pixel call overhead cannot explain three orders of magnitude
  88. [CRITICAL] [bug] src/iigs/hal.c -- the pinned stage framebuffer ($01:2000-$9CFF) was never reserved from the GS/OS Memory Manager, so NewHandle legally placed jlpBigAlloc blocks INSIDE the display: the sprite struct landed at $01:8B04 and every surface clear repainted its fields (widthTiles read as the fill color -- 0x44 after clear(4)); clipRect then clamped draws to near-full-screen, producing finding #84's ~1000x sprite slowness, the 32KB backup-buffer overrun, and the BSS carnage (FIXED during Phase 1: jlpStageAllocPixels now claims the range via NewHandle attrAddr, and all JoeyLib NewHandle sites pass attrNoSpec)
  89. [MEDIUM ] [bug] src/core/debug.c:47 -- per-line log durability does not hold on the clang IIgs runtime: jlLogF+fflush lines do not reach the disk image until a clean fclose/exit (GS/OS FST or libc buffering), so a hung run leaves a joeylog.txt that dramatically under-reports progress -- the exact failure debug.c was designed to prevent. The SHR mailbox in uber.c is the reliable progress channel under emulation; fflush-commit semantics are queued as a toolchain question (LLVM816-ASKS.md)

Findings

1. jlDrawPixel (non-IIgs) marks dirty with unclipped x/y, causing out-of-bounds writes to the dirty-band arrays

  • Where: src/core/draw.c:549
  • Severity: critical | Category: bug

The IIgs branch of jlDrawPixel bounds-checks and returns before marking (draw.c:529), but the #else branch (DOS/Amiga/ST) calls surfaceMarkDirtyRect with the RAW coordinates after plotPixelNoMark silently clipped the plot. surfaceMarkDirtyRect's contract (surface.c:230, 'already clipped to surface bounds') is violated: for s == stage and y outside [0,199], the row loop calls widenRow(y,...) which reads/writes gStageMinWord[y] and gStageMaxWord[y] with no clamp -- e.g. jlDrawPixel(stage, 5, 250, c) writes gStageMinWord[250], corrupting the adjacent gStageMaxWord array or other .bss. Negative x with valid y is also broken: SURFACE_WORD_INDEX(-5) = 0xFE widens gStageMaxWord[y] to 254 (rows are only 80 words), so the next jlStagePresent slams ~3 rows of bytes past the intended row band. Off-surface pixel draws are documented-legal ('Out-of-bounds draws are silent no-ops', draw.h:3) and routine in games, so this fires in normal use on DOS/Amiga/ST.

Fix: Mirror the IIgs branch: in the #else path do if ((uint16_t)x >= SURFACE_WIDTH || (uint16_t)y >= SURFACE_HEIGHT) { return; } after the NULL check, then call jlpDrawPixel directly (skipping the plotPixelNoMark layer and its duplicate checks) and mark. Optionally also add a debug-build assert/clamp in surfaceMarkDirtyRect to catch future contract violations.

Impact: Memory corruption (dirty-band arrays + neighboring globals) and present-time over-blit on DOS/Amiga/ST for any off-surface stage pixel; fix also removes one redundant function call + NULL check per on-surface pixel

2. Interpreted sprite inner loop re-derives dst byte address and parity per pixel via a real function call

  • Where: src/core/sprite.c:193
  • Severity: critical | Category: perf

For every opaque pixel the loop computes (dx + col), then writeDstNibble (lines 87-96) redoes x >> 1 to find the byte and branches on x & 1 for parity -- all rederivable state the loop could carry incrementally, exactly as it already carries the src inTileX phase (lines 195-199). Worse, ORCA/C 2.2.1 performs no function inlining, so on the IIgs this is a genuine JSL/RTL + 3-arg stack frame per opaque pixel (~60-90 cycles of pure call overhead on top of the ~30-cycle merge). The in-file comment at lines 343-345 states jlSpriteCompile leaves sp->slot NULL on IIgs, so this loop is the ONLY IIgs sprite-draw path (bench: jlSpriteDraw 438 ops/sec = ~6400 cycles/op total). writeDstNibble also masks nibble & 0x0F twice (lines 92, 94) even though the caller's extract at lines 190-191 already yields a 4-bit value.

Fix: Before the col loop set dst = dstRow + ((uint16_t)dx >> 1) and dstOdd = (uint8_t)(dx & 1). Per opaque pixel merge inline: if (dstOdd) { *dst = (uint8_t)((*dst & 0xF0) | nibble); dst++; } else { *dst = (uint8_t)((*dst & 0x0F) | (nibble << 4)); } then dstOdd ^= 1 (advance dst only after the odd half). Drop the redundant & 0x0F masks. Keep writeDstNibble only if a cold path still needs it; otherwise delete it.

Impact: Saves ~70-95 cycles per opaque pixel on 65816 (call overhead + add + shift + redundant masks). An 8x8 mostly-opaque sprite saves ~5000 of its ~6400 cycle budget: roughly 2-3x on IIgs jlSpriteDraw, the perf-floor reference op. DOS gains the per-pixel address arithmetic even where Watcom inlines.

Verifier correction: The per-pixel writeDstNibble call and redundant masks are real, and the fix is valid, but this is the clip-edge/uncompiled FALLBACK path, not every IIgs sprite draw: IIgs sprite codegen is ON by default (spriteCompile.c:144-146) and UBER's 438 ops/sec measured the compiled path. Expected win is 2-3x on the interpreted draw itself (edge-clipped sprites, pre-compile draws), not on the headline bench number. Bonus: the comment at sprite.c:341-345 is stale (contradicts spriteCompile.c #19 note) -- a multiple-sources-of-truth item worth fixing in the same pass.

3. IIgs (perf reference) silently runs the generic C nibble loop for jlTilePasteMono -- no JL_HAS_TILE_PASTE_MONO, no port.h macro

  • Where: include/joey/platform.h:98
  • Severity: high | Category: perf

The IIgs table jumps from TILE_PASTE (line 98) to TILE_SNAP (line 99) with no JL_HAS_TILE_PASTE_MONO, and the IIgs macro block in src/core/port.h (lines 37-192) has no jlpTilePasteMono macro, while Amiga (platform.h:146) and ST (platform.h:201) both override it. So jlTilePasteMono (src/core/tile.c:140) dispatches to jlpGenericTilePasteMono (src/generic/genericTile.c:77), a cross-TU call running a 32-iteration per-byte colorize loop ('hi = (srcByte >> 4) ? fgColor : bgColor' etc., lines 86-91) before finally reaching the asm jlpTilePaste. This is a hot path: Space Taxi repaints all 1000 level tiles through jlTilePasteMono (examples/spacetaxi/stRender.c:425) and re-pastes 103 logo cells every 4th frame on the title screen (stRender.c:527). Every sibling tile op on the IIgs is hand asm; this is the one silent miss the flag/override split hides.

Fix: Add JL_HAS_TILE_PASTE_MONO to the IIgs block in platform.h and a jlpTilePasteMono macro in port.h's IIgs section calling a new iigsTilePasteMonoInner in joeyDraw.s that LUT-expands mono bytes (16-entry nibble-pair table) directly into the surface rows, skipping the intermediate jlTileT + separate paste pass.

Impact: Generic path ~= 100 cyc call + ~1500-2200 cyc colorize + ~2530 cyc paste (jlTilePaste = 1106 ops/s) => ~600 ops/s; fused asm ~2x faster per op. Title-screen logo repaint alone is ~450K cycles every 4 frames (~2.5 full 60Hz frame budgets at 2.8MHz).

Verifier correction: All substantive claims hold. Two nits: (1) the '~2.5 full 60Hz frame budgets' figure is the amortized per-frame cost ? the 484K-cycle logo repaint is ~10 frame budgets per 4-frame period at 46.7K cyc/frame, so the impact is understated, not overstated; (2) the '~2530 cyc paste' component uses the full jlTilePaste API cost (1106 ops/s) while the generic path pays only the jlpTilePaste macro/asm inner (the jlTilePasteMono wrapper + dirty mark supply the rest), so the breakdown misattributes but the ~600 ops/s total is still about right.

4. Atari ST defines neither JL_HAS_FLOOD_WALK_PLANES nor JL_HAS_FLOOD_SCAN_ROW_PLANES -- flood fill decodes 4 bitplanes per pixel via a function call

  • Where: include/joey/platform.h:205
  • Severity: high | Category: perf

The ST table goes straight from SPRITE_RESTORE (204) to SURFACE_COPY_PLANES (205); the Amiga table has JL_HAS_FLOOD_WALK_PLANES / JL_HAS_FLOOD_SCAN_ROW_PLANES between them (platform.h:150-151, implemented at src/amiga/hal.c:2131 and 2181). Without them, floodFillInternal (src/core/draw.c) takes the portable tier: the walk-out loop calls 'pix = jlpSamplePixel(s, (int16_t)(leftX - 1), y);' (draw.c:166) and the row scan calls 'pix = jlpSamplePixel(s, (int16_t)(leftX + i), scanY);' (draw.c:237) once per pixel. ST surfaces are pure planar (pixels==NULL, atarist/hal.c:2131/2159), so each call lands in jlpSamplePixel (atarist/hal.c:2013) -> stPlanarGetPixel: y*160 address math plus 4 interleaved plane-word tests -- per PIXEL, through a function call. Each filled pixel is visited ~3x (walk + scan above + scan below). Flood fill is about to become per-frame-hot: the planned AGI interpreter draws every pic resource with flood fills.

Fix: Add the two flags to the ST block and implement jlpFloodWalkPlanes / jlpFloodScanRowPlanes in src/atarist/hal.c against the word-interleaved StPlanarT layout (walk runs 16 pixels per 4-word group by combining plane words into a match mask, same structure as the Amiga versions at amiga/hal.c:2131/2181, adjusted for ST_BYTES_PER_GROUP interleave).

Impact: ~180-220 cycles per sampled pixel today; a 100x50 flood ~= 5000 px x 3 visits x ~200 cyc = ~3M cycles = ~0.4s at 8MHz. Plane-word walkers process 16 px per 4 word-reads: estimated 10-20x speedup on ST flood fill.

Verifier correction: Directionally and factually right. Minor additions: port.h:153's comment claiming the planar flood hooks are 'Amiga-only' must be updated with the fix, and the realistic speedup is ~5-15x rather than the quoted 10-20x since the run-edge markBuf walk in draw.c stays per-pixel C either way.

5. jlSpriteCompile ignores the arena's compact-and-retry contract: one fragmented alloc failure silently demotes the sprite to the interpreter forever

  • Where: src/codegen/spriteCompile.c:202
  • Severity: high | Category: perf

codegenArenaInternal.h:41-44 states the contract: 'Returns NULL if no free slot is large enough -- the caller should run codegenArenaCompact and retry, or surface the failure.' The only caller does neither: it bails, sp->slot stays NULL, and every subsequent jlSpriteDraw/SaveUnder/RestoreUnder for that sprite runs the interpreter (benched at 0.13-0.22x of the compiled path -- the #1 perf gap). codegenArenaCompact is only reachable via the public jlSpriteCompact (src/core/sprite.c:435), which nothing calls automatically, so a game that creates/destroys sprites across levels accumulates holes until compiles fail even with ample total free space. jlSpritePrewarm additionally swallows the false return ('(void)jlSpriteCompile(sp);', sprite.c:360), so the failure is invisible.

Fix: On the NULL return, call codegenArenaCompact() and retry codegenArenaAlloc(totalSize) once before giving up (failure-path only, zero hot-path cost). Optionally coreSetError on the final failure so prewarm failures are diagnosable.

Impact: Prevents a permanent 5-8x slowdown on the hottest op after arena fragmentation; ~10 lines

Verifier correction: jlSpriteCompile technically satisfies the contract's second arm (it surfaces the failure via the documented false return, sprite.h:86), so 'ignores the contract' is overstated. The real defect: no code anywhere acts on that failure -- jlSpritePrewarm and both example apps swallow it, and nothing ever calls jlSpriteCompact -- so arena fragmentation silently and permanently demotes sprites to the interpreter. Fix as proposed: on codegenArenaAlloc returning NULL, call codegenArenaCompact() and retry the alloc once before returning false; failure-path only, zero hot-path cost.

6. SFX mixer does a full 32x32 multiply per output sample; on 68000 this is a __mulsi3 library call

  • Where: src/core/audioSfxMix.c:83
  • Severity: high | Category: perf

interp() runs once per output sample per active slot inside audioSfxOverlayMix. delta is at most +/-255 and frac at most 65535, but both are int32_t, so on the ST (compiled -m68000 per make/atarist.mk line 10, no mulu.l) gcc emits a __mulsi3 library call: 3x mulu.w plus shifts/adds plus call overhead, roughly 250-350 cycles, versus ~74 cycles for a single muls.w. At the ST's 12288 Hz MIX_RATE with one active slot that is ~2.5-4M wasted cycles/sec, ~30-45% of the 8 MHz CPU, and the ST HAL already warns (src/atarist/audio.c:413) that audioSfxOverlayMix can approach the 83 ms refill budget -- this multiply is the single biggest reason.

Fix: Reformulate as a 16x16->32 signed multiply that matches gcc's mulhisi3 pattern: sample = (int)s0 + (int)(((int32_t)(int16_t)delta * (int32_t)(int16_t)(uint16_t)(frac >> 1)) >> 15); -- delta fits int16, frac>>1 fits a positive int16, and the result differs from the current value by at most 1 LSB of 8-bit audio (inaudible). Emits one muls.w on 68000; DJGPP is unaffected.

Impact: ~200-280 cycles/sample saved; roughly halves audioSfxOverlayMix cost on ST (~30% of ST CPU while SFX plays)

Verifier correction: Everything as stated, except the proposed one-line expression does not deliver the muls.w on the shipped toolchain (still compiles to __mulsi3). Write it with explicit int16_t locals so gcc matches mulhisi3: 'int16_t delta = (int16_t)(s1 - s0); int16_t fracH = (int16_t)(uint16_t)(frac >> 1); return (int)s0 + (int)(((int32_t)delta * fracH) >> 15);' -- verified to emit a single muls.w with m68k-atari-mint-gcc 15.2.0 at the real ST flags.

7. 64-bit posFp arithmetic executed per sample in the mixer inner loop

  • Where: src/core/audioSfxMix.c:110
  • Severity: high | Category: perf

pos is uint64_t (48.16 fixed point), so every sample pays a 64-bit >>16, a 64-bit mask, and a 64-bit add (pos += (uint64_t)step; line 142). On the 68000 each of these is a multi-register sequence (~40-70 cycles combined vs ~10 for 32/16-bit ops). The uint64_t exists only to lift the old 65535-input-sample cap (header comment, audioSfxMixInternal.h:37-41), but a split representation gives the same range without any 64-bit ops: a uint32_t sample index covers 4G input samples, far beyond the 48-bit rationale.

Fix: Replace posFp with uint32_t posIdx + uint16_t posFrac, and split stepFp once per slot into stepInt = stepFp >> 16 and stepFrac = stepFp & 0xFFFF. Inner loop becomes: frac += stepFrac; idx += stepInt + (frac carry); (carry via if (frac < stepFrac) idx++; after 16-bit wrap, or a uint32_t accumulator for frac). Update audioSfxSlotArm/ArmStream/streamRefill resets accordingly. Output is bit-identical.

Impact: ~30-60 cycles/sample on 68000, ~10-15% of the mixer inner loop

Verifier correction: Impact is ~25-50 cycles/sample (~5-10% of the loop today, ~10-17% after the multiply fix), not a guaranteed 30-60. Use the carry-compare formulation (uint16_t frac; frac += stepFrac; if wrap then idx++) or keep frac in a 16-bit register -- the suggested uint32_t-accumulator '(fracAcc >> 16)' add compiles to a swap chain on m68k-atari-mint-gcc that eats most of the win. Part of the benefit is indirect: freeing the second pos register un-spills step from the stack (add.l 50(%sp) becomes add.l Dn).

8. IIgs 64KB attrNoCross cap on the codegen arena is unenforced; codegenBytes > 64KB hard-fails jlInit (adventure.c passes 256KB)

  • Where: src/core/codegenArena.c:236
  • Severity: high | Category: bug

attrNoCross means the block may not cross a 64KB bank boundary, so any totalBytes > 0x10000 is unsatisfiable and NewHandle returns NULL; codegenArenaInit returns false and jlInit (src/core/init.c:99-105) aborts with the generic message 'failed to allocate codegen arena'. Nothing in the arena, init.c, or joey/core.h documents or clamps the cap. examples/adventure/adventure.c:2273 sets config.codegenBytes = 256UL * 1024, which would make the app refuse to start on the reference platform even though clamping to one bank would work fine (routineOffsets are uint16_t and per-sprite fnAddr math already fits a bank).

Fix: In codegenArenaInit, under JOEYLIB_PLATFORM_IIGS, clamp: if (totalBytes > 65536ul) { totalBytes = 65536ul; } (with a comment naming the attrNoCross bank cap), or fail with a specific error string so the caller knows to shrink codegenBytes.

Impact: Whole-app init failure on IIgs for any config >64KB; one concrete caller (adventure) already trips it

Verifier correction: One refinement: clamping to exactly 65536 may still fail at runtime -- a 64KB attrFixed+attrNoCross+attrPage block requires a completely free bank, which GS/OS may not have. The clamp should be paired with a specific error string (so failure is diagnosable) and/or adventure.c should set codegenBytes per-platform (#ifdef: ~16KB on chunky ports as its own comment notes, 256KB only on Amiga). Also document the IIgs cap at the jlConfigT.codegenBytes declaration in include/joey/core.h.

9. jlDrawLine Bresenham fallback accumulates the dirty bounding box with 4 compares per pixel although it equals the endpoint box

  • Where: src/core/draw.c:484
  • Severity: high | Category: perf

Bresenham walks monotonically from (x0,y0) to (x1,y1) and terminates exactly at (x1,y1), so the set of visited points has bounding box [min(x0,x1),max(x0,x1)] x [min(y0,y1),max(y0,y1)] -- exactly what the JL_HAS_DRAW_LINE fast path already computes once up-front at lines 459-462 (bbx/bby/bbw/bbh). The in-loop accumulation therefore recomputes a value that is fully known before the loop, at 4 compare+branch pairs per plotted pixel. This fallback is the ONLY diagonal-line path on DOS (no JL_HAS_DRAW_LINE) and runs for every clipped diagonal line on IIgs/Amiga/ST.

Fix: Before the loop, save the original endpoints and compute minX/maxX/minY/maxY from them (same 4 ternaries as the fast path), then delete the 4 in-loop compares and keep the existing post-loop clamp+mark unchanged. Output is bit-identical.

Impact: ~40-50 cycles/pixel on 68000 (4x cmp.w+bcc) vs a ~150-250 cycle per-pixel plot: ~15-25% off the fallback diagonal-line inner loop; ~20-30 cycles/pixel on 65816

10. IIgs stage-identity test is a cross-TU jlStageGet() function call per primitive (per pixel on the hottest op) and is re-checked again inside surfaceMarkDirtyRect

  • Where: src/core/draw.c:533
  • Severity: high | Category: perf

gStage is static in surface.c (line 27), so the stage-identity test everywhere outside surface.c must go through jlStageGet() -- a real cross-segment JSL + ORCA-C prologue/epilogue. The IIgs jlDrawPixel (measured 1755 ops/sec ~= 1600 cyc/call) pays this call per pixel just to compare two pointers. The IIgs macros jlpFillCircle, jlpSurfaceClear, and jlpFillRect in src/core/port.h (lines 97, 106, 116: ((_s) == jlStageGet() ?) pay it once per invocation -- i.e. once per SPAN in jlFillCircle/jlDrawLine/jlDrawRect span loops via fillRectOnSurface. Worse, fillRectOnSurface then calls surfaceMarkDirtyRect, which re-derives the same fact with its own s != gStage compare -- the stage test is evaluated twice per span through two different mechanisms (function call vs direct global), a duplication of the same truth.

Fix: Make gStage non-static, declare extern jlSurfaceT *gStage; in surfaceInternal.h (gStageMinWord/gStageMaxWord are already extern there), and replace jlStageGet() in the port.h macros and draw.c hot paths with a direct (_s) == gStage compare (two LDA-long + CMP, ~12-16 cycles vs ~40-70 for the call). Keep jlStageGet() as the public accessor.

Impact: ~30-60 cycles per primitive invocation on IIgs: ~3-4% of jlDrawPixel per call, multiplied per-span in fillCircle/drawRect/line loops; also removes the double stage check per span

11. jlDrawRect general path wraps w/h > 32767 through (int16_t) casts, drawing spurious edges on-surface

  • Where: src/core/draw.c:591
  • Severity: high | Category: bug

In the clipped (general) path, the bottom-edge row is computed as y + (int16_t)h - 1 and the right-edge column as x + (int16_t)w - 1 (line 595). h and w are uint16_t, so any value > 32767 goes negative in the cast and the edge coordinate wraps by -65536. When the true edge coordinate lands in [65536, 65855+], the wrapped value falls back inside [0, 319]/[0, 199] and a phantom edge is drawn ON-surface: jlDrawRect(s, 100, 5, 10, 65533, c) computes bottom row (int16_t)(5 + (-3) - 1) = 1 and draws a horizontal line at y=1 that should be at y=65537 (off-surface, i.e. nothing). Same for the right edge with large w (e.g. x=300, w=65530 draws a vertical edge at x=293). The on-surface fast path above is safe because it is gated by the 32-bit test at lines 574-576.

Fix: Compute the edge coordinates in int32_t in the general path (int32_t bottom = (int32_t)y + h - 1; skip the edge if bottom > SURFACE_HEIGHT-1 or < 0, else cast), or early-clip the outer rect in 32-bit like jlFillRect does and derive edges from the clipped box.

Impact: Wrong pixels rendered for w or h > 32767 on all four ports; cold path, ~4 extra 32-bit ops only in the already-slow clipped case

Verifier correction: Details are accurate as stated. Worth noting the realistic trigger is unsigned-wrap in caller width math, not literal 32k-wide rects.

12. jlFillCircle (and jlDrawCircle fallback) cast r to int16_t: r > 32767 draws nothing; r in [~380, 32767] iterates up to 32k clipped spans

  • Where: src/core/draw.c:656
  • Severity: high | Category: bug

r is uint16_t. For r >= 32768, (int16_t)r is negative, so the span loop never executes and jlFillCircle draws NOTHING -- but a disk of radius 40000 centered anywhere near the surface mathematically covers the entire 320x200 surface, so the correct output is a full-surface fill. jlDrawCircle's fallback has the same wrap at line 338 (x = (int16_t)r; makes while (x >= y) false immediately), which silently drops perimeter pixels that can legitimately cross the surface (e.g. cx=-39900, cy=100, r=40000 passes through x~100). Separately, for r in [~380, 32767] with the circle not fully on-surface, the loop runs r+1 iterations emitting two clipped jlFillRect calls each even though at most 200 rows can intersect the surface -- 32767 iterations of 32-bit while-loop math plus 65k clipped fill calls is multiple seconds at 2.8 MHz.

Fix: Before the span loop: 32-bit test whether the farthest surface corner from (cx,cy) satisfies dist2 <= r*r; if so, jlFillRect(s, 0, 0, SURFACE_WIDTH, SURFACE_HEIGHT) and return (this also fixes all r >= 32768 full-coverage cases). Then clamp the y loop to rows that can intersect the surface (y limited by cy vs [0,199] bounds, computed in int32 once) and use a uint16_t loop variable.

Impact: Correctness: whole-surface fills silently dropped for huge r; perf: worst case ~32k wasted iterations (seconds per call) reduced to <=200

Verifier correction: Fix must seed yy=y0*y0 when clamping the y range (the incremental (y+1)^2 chain breaks on a jump), and the initial x walk-down from r is still O(r) once per call unless an integer sqrt seeds x; the wasted-iteration window is any r >= 200 with clipping, not just r >= ~380.

13. jlFillRect runs 32-bit clip math on every call; the common on-surface case needs only 16-bit compares

  • Where: src/core/draw.c:703
  • Severity: high | Category: perf

The clip (draw.c:703-717) does 4 sign/zero extensions to int32_t, two 32-bit adds, four 32-bit clamps, and two 32-bit compares on a 16-bit-native CPU where every int32 op is a multi-instruction carry-chained sequence -- roughly 120-200 cycles -- even when the rect is fully on-surface (the overwhelmingly common case: 16x16 HUD fills, tile-sized fills, benchmark ops). The 32-bit width is only needed to survive x<0 with huge w, or w>32767. The library already recognizes this cost elsewhere: jlDrawLine (draw.c:425-427), jlDrawRect (draw.c:574-576), and jlFillCircle (draw.c:664-667) all grew on-surface pre-tests specifically to 'skip jlFillRect's 32-bit clip'. jlFillRect itself never got the fast path, so direct API users always pay it.

Fix: Add a 16-bit fast path before the 32-bit clip: 'if (x >= 0 && y >= 0 && w > 0 && h > 0 && w <= SURFACE_WIDTH && h <= SURFACE_HEIGHT && x <= (int16_t)(SURFACE_WIDTH - w) && y <= (int16_t)(SURFACE_HEIGHT - h)) { fillRectOnSurface(s, x, y, (int16_t)w, (int16_t)h, colorIndex); return; }' -- all uint16/int16 ops -- and keep the 32-bit clip as the slow path. This also lets the three callers above drop their duplicated pre-tests (one source of truth for the fast path).

Impact: ~100-180 cycles per on-surface jlFillRect: ~10-25% of an honest 16x16 fill (450 ops/sec measured), plus dedupes three caller-side on-surface tests

Verifier correction: Add the 16-bit on-surface fast path to jlFillRect, but keep the three span emitters calling fillRectOnSurface directly (do not reroute spans through the public jlFillRect). Realistic impact: ~100-180 cycles per on-surface call on the 65816, ~2-3% of the measured 16x16 fill (more, proportionally, for smaller rects); severity medium, not high.

14. jlSamplePixel pays a second cross-TU function hop for a ~30-cycle nibble read on chunky ports

  • Where: src/core/draw.c:776
  • Severity: high | Category: perf

IIgs and DOS define no JL_HAS_SAMPLE_PIXEL, so jlpSamplePixel resolves to the out-of-line jlpGenericSamplePixel function (port.h:394-400). The chain is: public jlSamplePixel call (~60-80 cyc marshalling s/x/y) -> NULL+bounds checks -> a SECOND full call into jlpGenericSamplePixel (~60-80 cyc re-marshalling the exact same three args) -> a 3-line nibble extract. Measured 1916 ops/sec = ~1460 cyc/call for what is fundamentally a ~30-40 cycle indexed read; the inner hop alone is 30-50% of the honest post-fix cost. This is the exact wrapper tax port.h:44-46 documents ('a C wrapper would cost ~60-80 cyc/call of pure overhead') and eliminated for the draw ops via macros -- the read op was left behind.

Fix: On chunky builds (JOEYLIB_NATIVE_CHUNKY without JL_HAS_SAMPLE_PIXEL), inline the read directly in jlSamplePixel after the bounds check: 'uint8_t b = s->pixels[SURFACE_ROW_OFFSET((uint16_t)y) + ((uint16_t)x >> 1)]; return (x & 1) ? (uint8_t)(b & 0x0F) : (uint8_t)(b >> 4);' (draw.c already includes surfaceInternal.h). Equivalently, make jlpGenericSamplePixel a static inline in a shared internal header so all chunky call sites collapse the hop.

Impact: ~80-150 cycles per call, 30-50% of jlSamplePixel; also speeds the flood-fill C walk-out loops on ports without asm flood tiers (DOS/blank) which call it per pixel

Verifier correction: Win on IIgs is larger than stated per-mechanism (inner call ~60-80 cyc PLUS the y*160 software-multiply helper the generic pays because it skips the gRowOffsetLut), but is ~10-17% of the measured op, not 30-50%. To also speed the flood-fill walk loops, the read must be exposed as a macro (not static inline -- ORCA/C 2.2.1) in a shared internal header, since floodFillInternal calls jlpSamplePixel, not the public jlSamplePixel.

15. jlFrameCount is a double pass-through on IIgs: jlFrameCount -> jlpFrameCount -> iigsGetTickWord, paid every benchmark iteration and every game-loop pacing check

  • Where: src/core/init.c:148
  • Severity: high | Category: perf

core.h tells callers to poll jlFrameCount faster than 2*jlFrameHz, and UBER polls it once per op iteration. On IIgs the call chain is three deep: the init.c wrapper JSLs to jlpFrameCount in src/iigs/hal.c:372-374, whose entire body is return iigsGetTickWord(); (asm in peislam.s). Two of the three layers are pure forwarding, ~150-250 cycles per call on the 65816. This is also where a large slice of the 'jlKeyDown = 3382 ops/sec' mystery goes: 2.8 MHz / 3382 is ~830 cycles/iter, of which the frame-count chain in the bench loop is ~250-350 cycles -- it systematically inflates the measured cost of every fast op.

Fix: Use the existing #if !defined(jlpFrameCount) hook (src/core/port.h:495): in the IIgs port header declare iigsGetTickWord and #define jlpFrameCount() iigsGetTickWord(), deleting the hal.c wrapper. Same treatment for jlpFrameHz (body is return gFrameHz;). That leaves one unavoidable public-ABI layer instead of three.

Impact: ~100-170 cycles per jlFrameCount call on IIgs; corrects downward bias in every fast-op UBER number

Verifier correction: Directionally right, three details need fixing. (1) Savings are ~60-80 cycles per call (one removed forwarding layer), not ~100-170: the public jlFrameCount ABI layer remains, and the dominant cost of the chain is the Misc Toolset GetTick dispatch inside iigsGetTickWord (peislam.s:29-35, jsl $E10000), which no wrapper removal touches. The benchmark bias is real but only partially removable this way. (2) The jlpFrameHz 'same treatment' is unsafe as proposed: gFrameHz is static in hal.c (line 59); exporting it for direct reads from CORESYS-segment code risks the documented cross-load-segment DBR/absolute-addressing trap (hal.c:363-371) if the compiler emits DBR-relative addressing, and jlpFrameHz is cold (once per bench op, not per iteration) -- leave it a function or verify long-addressing codegen first. (3) Also remove the now-stale JL_HAS_FRAME_COUNT at include/joey/platform.h:108 and fix the wrong '~10-30 cyc per-iter poll' comment at examples/uber/uber.c:12-14; a cheaper long-term option for the residual toolbox cost is having the asm cache the tick in a long-addressed word updated by an edge-detect, but that changes timing semantics and is a separate decision.

16. jlPaletteSet re-masks all 15 caller entries against the documented $0RGB contract instead of memcpy + forcing color 0

  • Where: src/core/palette.c:106
  • Severity: high | Category: perf

include/joey/palette.h:15-16 states the contract: 'colors16 must point to exactly 16 uint16_t values in $0RGB format', and the only documented silent rewrite is color 0 forced to $000. The & 0x0FFF on entries 1-15 is undocumented defensive validation of already-guaranteed input, paid on every call of an op measured at only 678 ops/sec (~4100 cyc). Under ORCA/C the loop costs two far-pointer indirect accesses, two 32-bit pointer increments, and SEP/REP churn from the uint8_t counter per iteration (~45-60 cyc x 15). The read-side twin jlPaletteGet (line 82) already does a plain 32-byte memcpy of the same row, which lowers to MVN (~7 cyc/byte) on IIgs.

Fix: memcpy(row, colors16, SURFACE_COLORS_PER_PALETTE * sizeof(uint16_t)); row[0] = 0x0000; This keeps the documented color-0-black behavior bit-exact for contract-conforming input and mirrors jlPaletteGet. (If the defensive mask must be kept, at minimum change uint8_t i to uint16_t i to stop the 8-bit-counter SEP/REP churn on the 65816.)

Impact: ~400-600 cyc/call, roughly 10-15% of jlPaletteSet on IIgs

Verifier correction: Directionally and quantitatively right; two mechanism details are off: ORCA/C keeps M=16 and handles the uint8_t counter via AND #$00FF masking (plus 32-bit far-pointer post-increments), not SEP/REP churn per iteration; and ORCA/C memcpy is a library call whose inner loop uses MVN, not an inlined MVN. The proposed fix (memcpy row, then row[0] = 0x0000) is correct as written ? palette.c's own paletteInitDefault comment already treats the re-masking as avoidable overhead.

17. jlStageGet() function call buried in hot-path port.h macros and jlDrawPixel costs a cross-TU hop per primitive

  • Where: src/core/port.h:116
  • Severity: high | Category: perf

The IIgs jlpFillRect (port.h:115-120), jlpSurfaceClear (port.h:105-109), and jlpFillCircle (port.h:96-101) macros, plus jlDrawPixel itself (draw.c:533 'if (s == jlStageGet())'), each call the jlStageGet() accessor to answer 'is this the stage?'. gStage is a static in surface.c (surface.c:27), so every jlDrawPixel, every jlFillRect, every fill-circle span (81 spans for r=40), every H/V line, and every flood-fill span pays a JSL+RTL plus a far-pointer load (~25-60 cyc) to fetch a pointer that changes only at init/shutdown. The repo already fixed this exact pattern for the codegen arena: codegenArenaInternal.h:62-66 says 'Direct extern access (instead of a getter function) so per-frame hot paths ... skip the JSL/PHB/RTL/PLB the wrapper would impose.' On the post-Phase-2 jlDrawPixel chain (~400-500 cyc) this call is ~8-12% of the whole op.

Fix: Declare 'extern jlSurfaceT *gStage;' in surfaceInternal.h (renaming or keeping jlStageGet() as the public accessor shim, same as codegenArenaBase()), and change the port.h macros and draw.c:533 to compare '(_s) == gStage' directly. Read-only cross-TU global reads are safe on 65816 (only '++' on globals is the DBR trap).

Impact: ~25-60 cycles removed per jlDrawPixel/jlFillRect/jlSurfaceClear/jlFillCircle-span call; ~8-12% on jlDrawPixel, multiplied 81x inside one jlFillCircle r=40 C-span fallback

Verifier correction: Direct-extern gStage fix is correct and safe, but: (1) drop the flood-fill claim ? IIgs flood spans go through iigsFloodWalkAndScansInner and never reach jlpFillRect, so flood is unaffected on every port; (2) impact today is ~25-50 cyc per call = ~1.5-3% of the measured jlDrawPixel (1755 ops/sec ? 1600 cyc) and ~0.5-1% of jlFillRect16x16; the 8-12% figure only holds for the hypothetical post-optimization ~400-500 cyc chain. Severity: medium, not high. The 81x-per-fill-circle multiplier applies only to the off-stage/off-surface C span fallback.

18. No opaque-pair whole-byte fast path; 5 conditional branches per pixel in the interpreted draw loop

  • Where: src/core/sprite.c:189
  • Severity: high | Category: perf

Each pixel executes 5 conditional branches: src-parity ternary (line 190), transparency test (line 192), dst-parity branch inside writeDstNibble (line 91), tile-boundary test (line 196), and the loop bound (line 185). Src parity (sx + col) & 1 and dst parity (dx + col) & 1 differ by the constant (dx ^ sx) & 1 for the whole draw, so the common aligned case (even x, no odd clip) can be specialized to a byte-wise loop: read one src byte = two pixels; byte == 0 skips both; both nibbles opaque = one whole-byte store instead of two read-modify-write merges. The first review round flagged this exact missing fast path in genericTile.c (the byte-merge structure at src/generic/genericTile.c lines 52-67 is the template, itself still missing the both-opaque single store at line 59) but not here, where it matters more: this loop is every IIgs sprite draw and the IIgs/DOS partial-clip fallback.

Fix: Hoist alignedPhase = (((dx ^ sx) & 1) == 0) out of the row loop. Aligned case: loop over src bytes -- b = *src++; if (b == 0) { dst++; continue; } if ((b & 0xF0) && (b & 0x0F)) { *dst++ = b; } else merge the single opaque nibble; handle one odd leading/trailing pixel outside the loop. Keep the current per-pixel walker only for the misaligned phase.

Impact: Aligned opaque pair drops from ~2 RMW merges (~50-60 cycles even after inlining) to one load+test+store (~15 cycles) on 65816; branch count falls from 10 to ~3 per pair. Combined with the dst-pointer fix this is ~3-5x on the aligned-opaque inner loop, the dominant case for typical sprites.

Verifier correction: Valid missing fast path, but on the interpreted fallback (edge-clipped and uncompiled draws) rather than 'every IIgs sprite draw' -- IIgs codegen is on by default. Implementation must chunk the byte loop per tile column (4 contiguous bytes, then advance srcTileRow by TILE_BYTES), which the sketched *src++ loop glosses over.

19. Save/restore interpreted fallbacks bypass SURFACE_ROW_OFFSET and pay two software multiplies per row

  • Where: src/core/sprite.c:524
  • Severity: high | Category: perf

The restore fallback (line 524) and the save fallback twin (line 632: srcRow = &s->pixels[(dy + row) * SURFACE_BYTES_PER_ROW];) compute row addresses with a raw * 160 instead of SURFACE_ROW_OFFSET -- the macro surfaceInternal.h:124-130 exists precisely because ORCA/C emits a ~150-cycle multiply helper for this (surfaceInternal.h:72 comment), and spriteDrawInterpreted at line 173 already uses it. Both loops additionally recompute the backup index per row with a variable multiply ((uint16_t)row * (uint16_t)copyBytes, lines 526 and 633), another ~30-50 cycle helper call on 65816. Since sp->slot is NULL on IIgs (comment lines 343-345), these fallbacks ARE the per-sprite-per-frame IIgs save/restore path. This is also a multiple-sources-of-truth violation: row addressing has a blessed single mechanism and these two loops re-derive it inline.

Fix: Hoist pointers and strength-reduce both loops: dst = &s->pixels[SURFACE_ROW_OFFSET((uint16_t)by) + byteStart]; src = backup->bytes; then per row memcpy(dst, src, (size_t)copyBytes); dst += SURFACE_BYTES_PER_ROW; src += copyBytes;. Apply identically in jlSpriteSaveUnder lines 631-636.

Impact: ~180-200 cycles per row saved on IIgs (both multiply helpers replaced by two pointer adds). An 8-row sprite saves ~1500 cycles per save or restore call -- plausibly 30-50% of each interpreted save/restore op. 68000 ports unaffected (chunky loop skipped); DOS gains the variable multiply.

Verifier correction: Fix and mechanism are correct, but on IIgs these loops are the clipped/uncompiled fallback, not the primary per-frame path (compiled MVN save/restore is on by default per spriteCompile.c). The cycle savings apply to the fallback op; the SURFACE_ROW_OFFSET bypass is independently a concrete multiple-sources-of-truth cleanup.

20. surfaceMarkDirtyRect is a cross-TU function hop on every drawing op, re-deriving stage-ness the dispatch macro just computed

  • Where: src/core/surface.c:234
  • Severity: high | Category: perf

Every primitive tail pays a full cross-TU call (~60-80 cyc of push/JSL/prologue) into surfaceMarkDirtyRect just to run 's != gStage' (surface.c:242) -- a test jlpFillRect's macro already evaluated via jlStageGet() one line earlier (two computations of the same fact per op) -- plus two shifts and a branch, before either the inline h==1 widen or a second JSL into iigsMarkDirtyRowsInner for h>1. Call sites on the hot five: fillRectOnSurface (draw.c:54), jlTilePaste (tile.c:161), jlTileCopy/Masked/Fill (tile.c:83/102/122), jlSpriteDraw via sprite.c:351 and spriteDrawInterpreted (sprite.c:203). For jlTilePaste the mark chain (~150-200 cyc: call + checks + JSL + 8-row widen) rivals the 32-byte paste itself; Phase 2.1 of IIGS_PERF_PLAN.md fixed this for jlDrawPixel only.

Fix: Move surfaceMarkDirtyRect into surfaceInternal.h as a static inline (it needs only the already-extern gStageMinWord/gStageMaxWord, a widenRow inline, and the extern gStage from the previous finding). h==1 widens fully inline; h>1 keeps the single JSL to iigsMarkDirtyRowsInner. Non-stage surfaces then cost one inline compare instead of a wasted function call.

Impact: ~60-100 cycles per jlFillRect/jlTile*/jlSpriteDraw call (10-25% of an honest 8x8 tile op); also removes the dead call entirely for non-stage surfaces

Verifier correction: Impact is ~60-120 cycles per op, which is ~2-4% of the MEASURED per-call times (jlTilePaste 1106 ops/s = ~2530 cyc, jlFillRect16x16/jlSpriteDraw ~6200-6400 cyc); the 10-25% figure only holds against the op's intrinsic cost with harness/API overhead excluded. Requires making gStage extern (it is static in surface.c). Note the redundant stage-ness applies to the jlpFillRect/jlpSurfaceClear/jlpFillCircle macros (which call jlStageGet(), itself a ~60-cyc cross-TU call worth converting to an extern-gStage compare in the same pass); the jlpTile* macros never compute stage-ness. On IIgs the sprite path hits this via sprite.c:203 (interpreted), since sp->slot is always NULL there; sprite.c:351/420 apply to the 68k compiled path. One caveat: if any IIgs build still uses ORCA/C 2.2.1, its 'inline' is parsed but not honored, making the win 68k-only under that compiler.

21. stageDirtyClearAll runs a 200-iteration byte loop every present while its sibling uses memset for the same arrays

  • Where: src/core/surface.c:317
  • Severity: high | Category: perf

jlStagePresent calls stageDirtyClearAll after every frame (src/core/present.c:25), so this loop is on the flagship hot op. Yet surfaceMarkDirtyAll, 90 lines up in the same file (lines 225-226), already switched to memset(gStageMinWord, 0, SURFACE_HEIGHT); memset(gStageMaxWord, STAGE_DIRTY_FULL_MAX, SURFACE_HEIGHT); with a comment explaining memset 'lowers to a tight fill (MVN-seeded on IIgs) and drops the 200-iter indexed-store loop'. The clear-all variant was left on the slow form: ~200 iterations of two 8-bit indexed stores plus loop overhead under ORCA/C is roughly 5000-8000 cycles per present (the per-present budget at the honest ~29-42 ops/sec is ~66-96k cycles), and the byte-at-a-time loop also runs per-frame on the 68k ports.

Fix: void stageDirtyClearAll(void) { memset(gStageMinWord, STAGE_DIRTY_CLEAN_MIN, SURFACE_HEIGHT); memset(gStageMaxWord, STAGE_DIRTY_CLEAN_MAX, SURFACE_HEIGHT); } (constant fills 0xFF / 0x00, same values the loop stores; deletes the row variable).

Impact: ~5000-8000 cyc per jlStagePresent on IIgs (~7-10% of a present); every frame on all four ports

Verifier correction: Net saving is the loop cost (~4000-6000 cyc) minus the two memsets (~2800 cyc for 400 bytes), i.e. roughly 2000-4000 cyc per present on IIgs (~3-6% of the honest ~66-96k cycle present budget), not the full 5000-8000. Still a clear every-frame win on all four ports and removes a duplicated slow variant of logic the author already converted to memset in surfaceMarkDirtyAll.

22. jlDrawText pays the full public jlTileCopyMasked wrapper plus a multi-row dirty-mark call per glyph

  • Where: src/core/tile.c:54
  • Severity: high | Category: perf

Per character, jlDrawText re-enters the public wrapper which re-checks dst/src for NULL (already checked once at tile.c:43), re-validates cx/cy against block bounds (guaranteed by the cursor-wrap logic at lines 57-62; only srcBx/srcBy from asciiMap actually need validation), and then calls surfaceMarkDirtyRect for an 8-row rect. On IIgs an h=8 mark takes the JSL-to-iigsMarkDirtyRowsInner path (surface.c:262), so every glyph costs a wrapper prologue (~60-80 cyc) plus a cross-segment dirty-mark call that re-widens the same 8 rows the previous glyph just widened.

Fix: In jlDrawText, validate srcBx/srcBy inline, call jlpTileCopyMasked(...) directly, and accumulate a bounding block rect (minCx/maxCx/minCy/maxCy) across the string; issue one surfaceMarkDirtyRect for the union after the loop (over-approximation on wrap is safe).

Impact: ~300-400 cycles saved per glyph on IIgs (~10-15% of text throughput); a 20-char HUD string saves ~7000 cycles per redraw

Verifier correction: The fix must also bounds-check the caller's bx/by once at jlDrawText entry (e.g. return, or clamp, when bx >= TILE_BLOCKS_PER_ROW || by >= TILE_BLOCKS_PER_COL): the wrap logic at tile.c:56-62 only guarantees in-range cx/cy after the first wrap, so with a direct jlpTileCopyMasked call an out-of-range initial by would produce up to 40 out-of-bounds tile writes (past the 200-entry gRowOffsetLut on IIgs) that the current wrapper's validation silently absorbs. Note this is a tiny behavior change for garbage inputs (chars before the first wrap were previously skipped, not drawn); the header documents bx in [0,39], by in [0,24], so rejecting them is fine. With that added, the rest of the fix (inline srcBx/srcBy validation from asciiMap, direct jlpTileCopyMasked, one unioned surfaceMarkDirtyRect after the loop) is sound on all four ports.

23. jlpGenericMillisElapsed wraps backward every ~18 minutes and does a 32-bit multiply+divide per call on IIgs/Amiga

  • Where: src/generic/genericPort.c:40
  • Severity: high | Category: bug

jlpFrameCount() returns uint16_t, so the product wraps to 0 after 65536 frames: 18.2 minutes at 60Hz, 21.8 at 50Hz. The uint32_t return type advertises a monotonic millisecond clock, so any game computing now - last deltas sees a huge underflow at the wrap and stalls or skips. Only ST and DOS override JL_HAS_MILLIS_ELAPSED (platform.h lines 218/248); the comment at line 37 confirms "This is the IIgs/Amiga path" -- the reference platform is affected. Each call also pays a 32-bit software multiply and a 32-bit divide (hundreds of cycles on the 65816).

Fix: Keep a static uint32_t accumulated frame count: read jlpFrameCount(), extend by tracking the uint16_t delta since the previous call ((uint16_t)(now - last) handles wrap), add to the 32-bit total, then convert. Cache jlpFrameHz() once; for the common 50/60Hz values replace the generic divide with *20 (50Hz) or *50/3 (60Hz) fast cases, keeping the general path as fallback.

Impact: correctness: eliminates the 18-min timer wrap on IIgs/Amiga; perf: drops a 32-bit div per call except in cold generic case

Verifier correction: All core claims stand. Refinements to the fix: (1) do not convert an accumulated total frame count (totalFrames1000/hz re-overflows uint32 at ~19.9h @60Hz) -- accumulate milliseconds incrementally per uint16_t frame delta, carrying the fractional-frame remainder (e.g. ms += delta20 at 50Hz; at 60Hz ms += delta*50/3 with mod-3 remainder carry), which pushes the wrap to the natural 2^32 ms (~49.7 days) where uint32_t delta arithmetic is exact for callers; (2) the fix only observes every wrap if called at least once per 65535 frames (~18 min) -- true for any per-frame caller, but document it; (3) on ORCA-C update the statics with explicit load/add/store (the gFrameCount pattern, src/iigs/hal.c:363-371), never ++ on file-scope data (inc abs DBR trap); (4) nuance on failure mode: pure 'delta >= threshold then reset' callers self-heal with one spurious fire -- the severe damage is accumulator-style callers like agi.c:787-789 (freeze + v11 script-clock jump); (5) related cleanup: src/iigs/hal.c:382-386 is a dangling comment for the removed IIgs jlpMillisElapsed acknowledging the old wrap limitation -- delete it, and core.h:61's 'decoupled from frame rate' claim is also currently false on IIgs/Amiga (frame-derived), which the accumulator fix does not change and the doc should note.

24. jlpGenericSamplePixel pays a software multiply (y*160) instead of the SURFACE_ROW_OFFSET LUT the codebase built for exactly this

  • Where: src/generic/genericSurface.c:55
  • Severity: high | Category: perf

port.h has no IIgs macro override for jlpSamplePixel, so every IIgs jlSamplePixel call (measured 1916 ops/sec, ~1460 cyc/call) lands here and pays the ORCA/C software multiply helper (~100-150 cyc) for the constant *160. surfaceInternal.h (included by this TU at line 12) defines SURFACE_ROW_OFFSET precisely to dodge this: on IIgs it is a single ASL + indexed read of gRowOffsetLut, and on gcc/Watcom ports it compiles to the same shift+add chain as the raw multiply. The same multiply is also paid per-pixel by the flood-fill C fallback walks in draw.c (lines 151, 166, 177, 237) on DOS.

Fix: uint8_t byte = s->pixels[SURFACE_ROW_OFFSET((uint16_t)y) + ((uint16_t)x >> 1)]; -- identical output on all ports, drops the multiply helper on IIgs.

Impact: ~100-150 cyc/call saved on IIgs, roughly 7-10% of jlSamplePixel; zero cost/change on other ports

Verifier correction: Directionally and numerically correct as written. Small precision fix: the per-pixel DOS flood-walk mention is not a real cost (x86 compilers strength-reduce the constant multiply, and the fix expands to the identical expression there); the concrete IIgs beneficiaries are jlSamplePixel itself (~8-10%/call), the once-per-call flood seed reads, and the per-pixel sprite-capture path in src/core/sprite.c:296-297.

25. PERF.md IIgs baseline predates the Phase-2 jlDrawPixel/dirty-mark rewrite; all cross-port ratios are computed against a stale floor

  • Where: PERF.md:38
  • Severity: medium | Category: multi-source-of-truth

IIGS_PERF_PLAN.md declares Phase 2 DONE ('jlDrawPixel fully inlined on IIgs ... This is the 5-10x path', plan line ~28-31), yet PERF.md (regenerated 2026-06-30) still shows 1755 ops/sec -- the exact figure the plan's pre-fix analysis derived its '~1595 cyc/call' from (plan line 196: 'jlDrawPixel costs ~1595 cyc/call (2.8e6 / 1755)'). PERF.md's own preamble admits the IIgs column is 'unchanged from the last IIgs capture'. So the reference floor that the whole perf directive ('every target must meet or beat the IIgs') is measured against does not describe the shipped code: the current draw.c:521-542 chain models at ~400-500 cyc (~5600-7000 ops/sec). Amiga's 2.04x and ST's 1.18x on jlDrawPixel are therefore probably violations, not passes -- the same class of measurement lie the plan's Phase 0 was created to kill for SEI ops. The ~1500 'missing' cycles in the task brief are roughly half this stale baseline and half the jlStageGet/asm-framing findings.

Fix: Re-capture the IIgs UBER column on MAME (make iigs-verify path) before any further cross-port tuning, and mark stale-capture columns explicitly in PERF.md (date per column) so a ratio can never silently mix captures from different code revisions.

Impact: Scope: every IIgs row and every Amiga/ST/DOS ratio in PERF.md for the pixel-family ops; misdirects which ports are 'passing' the perf floor

Verifier correction: The IIgs PERF.md column is genuinely one stale capture predating the whole IIGS_PERF_PLAN implementation (identical since commit 087bc77, 2026-05-27), so a recapture with per-column capture dates is warranted. But the jlDrawPixel impact is ~7%, not 3-4x: draw.c:524-527 records the shipped inlined form measured at 1875 ops/sec on hardware (vs 1672 for an all-C inline), so Amiga (~1.91x) and ST (~1.10x) remain passes against the honest floor. The rows where staleness plausibly changes conclusions are the fillRect/clear family: Phase 0.1 removed the fillRect SEI (table still shows the inflated 60), Phase 3's computed-jump slam should raise fillRect 16x16/80x80, and Phase 5.1's PHA-slam should raise surfaceClear -- meaning some 68k 'passes' near 1.0-1.3x on those rows (e.g. ST fillRect 1.11x/1.33x, Amiga 0.95x) could flip to violations after recapture, not the pixel rows.

26. Eleven IIgs JL_HAS flags are decorative -- the port.h macro overrides bypass the registry, so entries can be added/removed with zero effect

  • Where: include/joey/platform.h:100
  • Severity: medium | Category: multi-source-of-truth

For every IIgs op that port.h implements as a macro (SURFACE_CLEAR, DRAW_PIXEL, FILL_RECT, TILE_FILL, TILE_COPY, TILE_COPY_MASKED, TILE_PASTE, TILE_SNAP, SPRITE_DRAW, SPRITE_SAVE, SPRITE_RESTORE), the dispatch stanza's '#if !defined(jlp)' guard (e.g. port.h:205, 275) short-circuits before the JL_HAS flag is ever tested, and no core gate consults them (core only gates on JL_HAS_DRAW_LINE/DRAW_CIRCLE/FILL_CIRCLE/FLOOD_* -- draw.c:111,313,456,630). So the registry that platform.h line 81-86 declares authoritative is silently non-authoritative for the reference machine: deleting JL_HAS_TILE_PASTE from the IIgs table changes nothing, and forgetting to add a flag (the TILE_PASTE_MONO gap above) produces no error. This is the systemic mechanism behind silent-generic-fallback rot. Full sweep result: all other flags in all four tables do have matching jlp symbols, and no port defines a jlp function without its flag -- the drift today is confined to the IIgs macro set and the missing entries reported separately.

Fix: In port.h's IIgs macro block, pair each macro with a consistency check ('#if !defined(JL_HAS_TILE_PASTE) #error "IIgs macro override without registry flag" #endif' or one grouped check), so the table and the macro set can never diverge silently in either direction.

Impact: Compile-time guard; zero runtime cost. Closes the exact hole that let jlTilePasteMono run generic C on the perf-reference machine unnoticed.

Verifier correction: Accurate version: the 11 IIgs flags are decorative and the registry comment (platform.h:84-86) is a second, false source of truth; pairing each IIgs port.h macro with '#if !defined(JL_HAS_) #error #endif' (covering all 15 macro ops, including DRAW_LINE/DRAW_CIRCLE/FILL_CIRCLE/FLOOD_WALK_AND_SCANS whose flags stay load-bearing via core gates) prevents future macro/flag drift and keeps the table trustworthy as documentation. It does NOT close the finding-0 class of hole: an op missing from both the macro set and the registry (TILE_PASTE_MONO) triggers no check and silently takes the generic ? catching that class requires a deliberate per-port audit of which generic-fallback ops are intentional (DOS relies on generics by design), not a mechanical guard.

27. Amiga compiled save/restore dispatchers and the shift-alias loop are dead code contradicted by the emitters

  • Where: src/codegen/spriteCompile.c:246
  • Severity: medium | Category: dead-code

The comments here (lines 174-178, 241-245) claim Amiga 'SAVE/RESTORE emit only shift 0 and shift 1' and that this loop aliases slots 2..7 -- but spriteEmitSavePlanar68k and spriteEmitRestorePlanar68k both just return 0u; ('Save/restore are not compiled on the Amiga yet', spriteEmitPlanar68k.c lines 223-235). So every routineOffsets[*][SAVE/RESTORE] entry is SPRITE_NOT_COMPILED, the alias loop copies sentinel onto sentinel, and the entire Amiga spriteCompiledSaveUnder/spriteCompiledRestoreUnder bodies (lines 557-623) are unreachable: the sprite.c gates require a non-sentinel offset. Every Amiga sprite save/restore therefore runs the interpreted C walkers in amiga/hal.c -- consistent with the measured 0.13-0.22x sprite gap -- while this file reads as if compiled save/restore ships. Note also that when the emitters do land, jlSpriteRestoreUnder's chunky-only gate (sprite.c lines 496-497: copyBytes must equal spriteBytesPerRow or spriteBytesPerRow+1) will reject the planar shifted width (backup->width = widthPx+8 gives copyBytes = spriteBytesPerRow+4), silently keeping 7 of 8 x-alignments on the interpreted path.

Fix: Either delete the alias loop plus the dead Amiga dispatcher bodies until the emitters exist (leaving a one-line comment), or land the emitters; in either case make the restore gate platform-aware (accept the planar shifted copyBytes, e.g. via a SPRITE_RESTORE_SHIFT_FROM_BACKUP macro next to SPRITE_SHIFT_INDEX in spriteInternal.h) so the compiled path is reachable for shifted x.

Impact: ~70 lines of unreachable dispatch code; comments actively misdirect the Amiga sprite-perf work that memory notes flag as the number-one gap

Verifier correction: Core claim correct. Fix adjustment: replace the Amiga dispatcher bodies with ST-style empty linker stubs (sprite.c calls them unconditionally, so full deletion breaks the link), delete or clearly mark the alias loop and the lines 172-178/241-245 comments as aspirational, and when the emitters land make the restore gate accept the planar copyBytes = spriteBytesPerRow+4 case.

28. IIgs spriteCompiledDraw still pays the 2D-array multiply helper the same file dodges everywhere else

  • Where: src/codegen/spriteCompile.c:311
  • Severity: medium | Category: perf

The save and restore dispatchers in this same file rewrite routineOffsets[shift][op] as byte-pointer arithmetic specifically because 'uint16_t arr[N][M] indexing' and 'shift * SPRITE_OP_COUNT ... can lower to a multiply helper call' on ORCA/C (comments at lines 419-422 and 426, and sprite.c lines 559-561). But spriteCompiledDraw -- the single hottest dispatch, behind jlSpriteDraw at 438 ops/sec -- still uses the plain 2D index. DRAW is the one op where this was not applied.

Fix: Match the save/restore pattern: routineOffset = *(uint16_t *)((uint8_t *)sp->routineOffsets + (((((uint16_t)shift << 1) + shift) + SPRITE_OP_DRAW) << 1)); then add routineOffset into fnAddr (or take it from the caller per the dedup finding).

Impact: ~30-50 cycles per jlSpriteDraw on IIgs (0.5-0.8% of the op), zero risk

29. Dispatcher gate and spriteCompiled* callees each re-derive shift, route offset, and function address per call

  • Where: src/codegen/spriteCompile.c:423
  • Severity: medium | Category: duplication

The hot chain does the same derivations twice per op. jlSpriteSaveUnder computes shift, routeIdx = (shift<<1)+shift+SPRITE_OP_SAVE, and loads routeOffset to gate the fast path (sprite.c lines 565-567); spriteCompiledSaveUnder then recomputes shift (line 392), cacheIdx (line 423), and re-loads routineOffset (line 427). Restore is worse: sprite.c lines 487-501 compute spriteBytesPerRow, copyBytes, shift, routeIdx, and routeOffset, then spriteCompiledRestoreUnder calls spriteChunkyRestoreShift which recomputes copyBytes and spriteBytesPerRow (widthTiles*4 again) and re-derives shift (lines 471-482). jlSpriteSaveAndDraw triples it: it computes saveIdx/drawIdx and loads both offsets (sprite.c 408-413), then both callees redo everything, and both callees also independently compute the screen row pointer from SURFACE_ROW_OFFSET(y) (lines 398 and 308) even though sprite.h line 134 promises 'the destination ptr is computed once'. widthPx/heightPx are likewise recomputed from tiles at every entry (sprite.c 338-339, 395-396, 551-552; spriteCompile.c 394-395) when they are create-time constants.

Fix: Pass the already-derived values through: change spriteCompiledDraw/SaveUnder/RestoreUnder signatures (internal, spriteInternal.h) to take uint8_t shift and uint16_t routineOffset from the gate; in SaveAndDraw compute the row pointer once and hand it to both. Additionally precompute widthPx, heightPx, and per-shift copyBytes into jlSpriteT at spriteInitFields time.

Impact: ~40-100 cycles per compiled sprite op on IIgs (0.6-1.5% of jlSpriteDraw, more for SaveAndDraw); removes three copies of the same index-math truth

Verifier correction: Directionally right; one detail off: plain jlSpriteDraw's gate (sprite.c:346) does NOT derive shift/routeOffset today (only save/restore gates do), so the draw-path saving is just the widthPx/heightPx precompute (~10-20 cyc); the real wins are RestoreUnder (~100+ cyc: kills the spriteChunkyRestoreShift call) and SaveAndDraw (row pointer once). Refactor must touch all four platform dispatcher variants and keep Amiga/ST's SPRITE_NOT_COMPILED delegate path working.

30. assetLoad.c re-spells TILE_BYTES and the 40x25 tile-grid limits as private literals

  • Where: src/core/assetLoad.c:41
  • Severity: medium | Category: multi-source-of-truth

The file already includes joey/tile.h, which is the declared single source of truth for tile geometry ('one source of truth for the 4bpp packed layout' per sprite.c:18-19): TILE_BYTES == 32, TILE_BLOCKS_PER_ROW == 40, TILE_BLOCKS_PER_COL == 25. ASSET_TILE_DATA_BYTES is the fread size written straight into outTiles[i].pixels (assetLoad.c:238), whose real capacity is TILE_BYTES -- if tile geometry ever changed, the load would silently read the wrong byte count into the struct. The 40/25 caps would likewise silently diverge from the real surface grid.

Fix: #define ASSET_TILE_DATA_BYTES TILE_BYTES, ASSET_MAX_TILES_W TILE_BLOCKS_PER_ROW, ASSET_MAX_TILES_H TILE_BLOCKS_PER_COL (or use the tile.h names directly and delete the local defines).

Impact: Zero runtime cost; removes three shadow constants that must currently be kept in sync with tile.h by hand

31. jlSpriteBankLoad allocates up to 32000-byte cels with malloc, violating core.h's own IIgs allocator contract

  • Where: src/core/assetLoad.c:167
  • Severity: medium | Category: bug

tileBytes can reach ASSET_MAX_TILES_W * ASSET_MAX_TILES_H * 32 = 32000 bytes per cel, and a bank loads up to maxCels of them. include/joey/core.h:30-36 documents that on the IIgs the C heap lives in bank 0, is tiny, caps a single malloc at ~32 KB, and that 'multi-KB caches and frame buffers must come from the IIgs Memory Manager' via jlAlloc. Loading even a modest sprite bank (e.g. 16 cels of 32x32 = 4 KB each) exhausts the bank-0 heap and the loader silently returns a short count mid-file. src/core/sprite.c (lines 215/255/306) has the same pattern, so a fix should cover both and free with the matching jlFree.

Fix: Route cel pixel buffers (and jlSpriteT headers if desired) through jlAlloc/jlFree instead of malloc/free, after confirming the IIgs sprite emitter has no bank-0 assumption on tileData; update jlSpriteFree/ownsTileData teardown to use jlFree.

Impact: Prevents silent short-loads of sprite banks beyond ~4-8 cels on IIgs

Verifier correction: Directionally correct; three details to fix in the writeup: (1) the teardown function is jlSpriteDestroy (sprite.c:316-328, frees tileData with free()), not 'jlSpriteFree'; (2) jlSpriteCreateFromSurface's buf (sprite.c:255) and both jlSpriteT header mallocs (sprite.c:215/306, assetLoad.c:175) must switch in the same commit because jlSpriteDestroy cannot distinguish malloc'd from jlAlloc'd tileData; (3) on IIgs the heap is ~32KB TOTAL (0x4000-0xBF00), not just per-allocation capped, and the 8KB codegen arena mallocs from the same pool, so exhaustion arrives even sooner than the finding's 16-cel estimate.

32. jlSpriteBankLoad re-implements spriteInitFields field by field, and the spriteInternal.h comment contradicts it

  • Where: src/core/assetLoad.c:180
  • Severity: medium | Category: duplication

assetLoad.c lines 180-190 duplicate the exact body of spriteInitFields (sprite.c lines 117-127), including all four memsets with their subtle 0xFF-vs-0 sentinel rules. Two sources of truth for jlSpriteT initialization means the next field added to the struct (this already happened once with cachedSizeBytes) must be remembered in both places; missing the loader copy leaves bank-loaded sprites with garbage in a dispatch-critical field -- e.g. an uninitialized routineOffsets entry of 0 is a VALID arena offset and would dispatch into stale code, exactly the failure mode the 0xFF fill exists to prevent (sprite.c lines 113-116). Separately, spriteInternal.h line 39 documents tileData as 'NULL for loaded sprites', which this code disproves (loaded sprites own a malloc'd copy) -- and if it were true, jlSpriteCompile would reject every loaded sprite (spriteCompile.c line 165).

Fix: Make spriteInitFields non-static, declare it in spriteInternal.h, and call spriteInitFields(sp, buf, widthTiles, heightTiles, true); from jlSpriteBankLoad. Fix the spriteInternal.h line 39 comment.

Impact: Removes an 11-line duplicate of sentinel-sensitive init code; closes a future stale-dispatch/crash class for bank-loaded sprites

33. Mixer re-reads slot fields through the pointer every sample because dst stores are char-typed and may alias

  • Where: src/core/audioSfxMix.c:131
  • Severity: medium | Category: perf

The loop caches pos and step in locals but reads slot->length (line 127), slot->sample, and slot->streamLen (lines 114, 123-124) through the slot pointer on every sample. Because dst[i] = ... (line 141) writes through a (volatile) uint8_t pointer, C aliasing rules force the compiler to assume the store may modify *slot and reload those fields each iteration -- 3-4 extra memory operands per sample on the 68000 (~40-60 cycles). The per-sample if (isStream) branch also runs 12288 times/sec even though isStream is loop-invariant.

Fix: Hoist const uint8_t *smp = slot->sample; uint32_t length = slot->length; uint16_t streamLen = slot->streamLen; into locals before the inner loop (re-snapshot streamLen after streamRefill), and split the fixed-buffer and streaming cases into two separate inner loops so the isStream test happens once per slot instead of once per sample.

Impact: ~10-15% of the mixer inner loop on ST

Verifier correction: Real but the per-sample overhead is 2 slot-field reloads (length, sample ptr; streamLen on the stream path) plus the isStream test -- about 30-40 cycles/sample, not 3-4 operands/40-60: gcc already hoists slot->fill into a register and reads length only once per iteration, reusing it for both bounds checks. Hoisting into locals plus splitting the fixed/stream loops is the right fix and was verified to keep the fields in registers across the volatile store; expect ~7-9% of the loop today, ~10-13% after the __mulsi3 fix.

34. codegenArenaShutdown frees every ArenaSlotT while live sprites still hold pointers; shutdown -> re-init -> jlSpriteDestroy corrupts the heap

  • Where: src/core/codegenArena.c:294
  • Severity: medium | Category: bug

jlShutdown (init.c:129) calls this with no way to clear sp->slot in user-owned sprites. While the arena stays down, codegenArenaFree's gCodegenArenaBase == NULL guard (line 207) saves a later jlSpriteDestroy -- but after a second jlInit the guard passes and lines 210-214 read/write the freed struct (slot->used = false; gUsedBytes -= slot->size;) and coalesce through freed next/prev pointers: heap corruption. Drawing the old sprite instead executes stale offsets inside the NEW arena (silent garbage execution). Relatedly, the double-free 'ignore' at lines 210-212 is false safety: per codegenArenaInternal.h:46-49 the struct 'may be freed (if it coalesced into a neighbor)', so the !slot->used check itself can be the use-after-free. (Compaction, by contrast, was audited safe: all four emitters are genuinely position-independent -- 68k d16(An)-only, IIgs abs,Y with DBR from the argument and MVN bank operands that are data banks moving intact with the memmove, x86 [esi+disp] -- so the cachedDstBank/cachedSrcBank cache stays valid across moves.)

Fix: Keep a small registry (or intrusive list) of compiled jlSpriteT in sprite.c; have a spriteSystemShutdown hook walk it from jlShutdown, calling codegenArenaFree and NULLing sp->slot + routineOffsets before codegenArenaShutdown runs.

Impact: Eliminates heap corruption / wild execution across any shutdown/re-init cycle with surviving sprites

Verifier correction: Accurate as written; note the exploit window is specifically shutdown -> re-init -> destroy/draw (destroys while the arena stays down are already safe via the line-207 guard, and every current example shuts down only at exit). A documentation-only fix (sprites are invalidated by jlShutdown; destroy them first) would also close it at zero code cost if the author prefers.

35. floodFillInternal C fallback evaluates the loop-invariant matchEqual ternary and a jlpSamplePixel function call per pixel

  • Where: src/core/draw.c:168
  • Severity: medium | Category: perf

matchEqual is fixed for the whole fill, but the walk-left loop (line 168), walk-right loop (line 179), and the markBuf fill (lines 239-241) re-branch on it per pixel. Each walked pixel also pays a full jlpSamplePixel function call that re-derives the row base (SURFACE_ROW_OFFSET(y), a 160 multiply on non-IIgs generic) even though y is constant across the entire walk and scan. On the ST this is the worst case: it has no JL_HAS_FLOOD_ hooks at all (platform.h lines 189-241), so every walked/scanned pixel is a function call into the planar samplePixel that decodes 4 plane bits -- flood fill on ST will be far below the IIgs reference, violating the perf floor. Flood fill is a declared hot op for the planned AGI/Sierra picture playback.

Fix: Split the walk and markBuf loops into matchEqual/boundary variants selected once per call (compile-time-style if/else duplication, no dispatch tables), and give the chunky generic a row-walk helper that takes a precomputed row pointer and steps bytes (2 px/byte) instead of calling jlpSamplePixel per pixel; add the analogous ST plane-row walker (JL_HAS_FLOOD_WALK_PLANES / JL_HAS_FLOOD_SCAN_ROW_PLANES) like Amiga has.

Impact: Removes 1-2 branches plus a ~100-300 cycle call per walked pixel on DOS/ST; ST flood fill is currently per-pixel planar-decode calls and stands to gain several x

Verifier correction: The generic samplePixel's y*160 is strength-reduced to shifts/adds by gcc on 68000/DOS, so the per-pixel cost there is the function call + (on ST) the 4-bit plane decode, not a multiply; the ST plane-walker hook is where the multiple-x gain lives.

36. jlDrawCircle fallback runs 8 bounding-box compares per iteration that are provably dead after the first iteration

  • Where: src/core/draw.c:345
  • Severity: medium | Category: unnecessary-conditional

On the first iteration x == r and y == 0, so the accumulation immediately sets maxX = cx+r, minX = cx-r, maxY = cy+r, minY = cy-r. Every subsequent candidate uses offsets x,y <= r, so none of the 8 conditionals can ever fire again -- the box is a compile-time-derivable constant of (cx, cy, r), and the accumulation (unlike the plots) does not depend on which pixels were on-surface since it folds off-surface reflections too. All 8 compare+branch pairs per iteration are pure dead work.

Fix: Delete the 8 in-loop conditionals; before the loop compute the box once in int32 (minX = cx-r, maxX = cx+r, minY = cy-r, maxY = cy+r, then the existing clamp). The clamped result is bit-identical, including the fully-off-surface collapse to no-op.

Impact: 8 compare/branch pairs (~50-80 cycles) per iteration, ~0.7*r iterations per circle: ~5% of the fallback circle path (every circle on DOS, every clipped circle on IIgs/Amiga/ST)

37. Bresenham fallbacks accumulate a bounding box per pixel that is statically known before the loop

  • Where: src/core/draw.c:484
  • Severity: medium | Category: unnecessary-conditional

A line's dirty box is exactly min/max of its two endpoints -- the JL_HAS_DRAW_LINE fast path already computes precisely that in four ternaries BEFORE drawing (draw.c:459-462: 'int16_t bbx = (x0 < x1) ? x0 : x1;' etc.), yet the fallback re-derives it with 4 compare+branches on EVERY plotted pixel. Same pattern in jlDrawCircle (draw.c:345-352): 8 folds per iteration whose result is fully determined after the first iteration, because iteration one runs with x==r, y==0 and immediately sets the box to cx-r..cx+r / cy-r..cy+r; every later fold is dead work. DOS defines neither JL_HAS_DRAW_LINE nor JL_HAS_DRAW_CIRCLE, so every diagonal line and every circle outline there pays this; IIgs/Amiga/ST pay it on every clipped line/circle.

Fix: jlDrawLine: hoist the endpoint min/max computation above the loop (reuse the exact 4-ternary block from the fast path) and delete the in-loop folds. jlDrawCircle: set minX=cx-x, maxX=cx+x, minY=cy-x, maxY=cy+x once before the loop (x==r there; keep the existing post-loop clamp, which already handles fully-off-surface circles).

Impact: ~20-40 cycles/pixel removed from the clipped line path (4 compare+branch on 65816/68000) and ~8 folds/iteration from circle outlines; DOS pays this on every diagonal line and circle

Verifier correction: Valid, but entirely redundant with findings 3 and 5; treat as one fix. For the circle, hoist in int32 (per finding 5) rather than int16 so centers near int16 limits do not under-mark.

38. jlFillCircle C span loop pays surfaceMarkDirtyRect (plus IIgs jlStageGet) per span instead of one bounding-box mark

  • Where: src/core/draw.c:668
  • Severity: medium | Category: perf

In the onSurface branch every one of the 2r+1 scanline spans routes through fillRectOnSurface, which is jlpFillRect + surfaceMarkDirtyRect. That is 2 extra function calls per span (3 on IIgs, where the jlpFillRect macro also calls jlStageGet() per span), yet the exact final dirty rect is already known: the (cx-ir, cy-ir, 2ir+1, 2ir+1) square the JL_HAS_FILL_CIRCLE path marks once at line 634. For non-stage surfaces surfaceMarkDirtyRect is a pure wasted call per span (immediate s != gStage return). This path is hit for every on-surface fill circle on DOS, and on IIgs/Amiga/ST whenever jlpFillCircle returns false (IIgs: any non-stage surface).

Fix: In the onSurface branch call jlpFillRect(...) directly per span (no fillRectOnSurface), and after the loop mark once: surfaceMarkDirtyRect(s, (int16_t)(cx - ir), (int16_t)(cy - ir), (int16_t)(2 * ir + 1), (int16_t)(2 * ir + 1)). Identical dirty coverage, since the y=0 span already spans cx-r..cx+r.

Impact: Eliminates 2r+1 surfaceMarkDirtyRect calls (+2r+1 jlStageGet calls on IIgs): ~100-160 cycles/span on 65816, r=30 saves ~6-10k cycles (~15-25% of the C-fallback fill circle)

Verifier correction: Savings are the surfaceMarkDirtyRect + fillRectOnSurface wrapper calls per span only; the per-span jlStageGet inside the jlpFillRect macro remains unless gStage is also made directly comparable (finding 4). Dirty coverage is not identical: the single square mark over-marks rows away from the center exactly like the JL_HAS_FILL_CIRCLE asm path already does (safe; slightly more present-time slam on DOS stage fills, pure win for non-stage surfaces where marks are no-op calls).

39. DEFAULT_CODEGEN_BYTES (8KB) is sized to IIgs code density; a single 32x32 sprite's ST/Amiga DRAW routines nearly fill or overflow it, silently forcing the interpreter

  • Where: src/core/init.c:20
  • Severity: medium | Category: perf

The '~3-4 KB per 32x32 sprite' claim is IIgs math (5-9 bytes/destbyte, opaque pairs 3 bytes). The 68k emitters are 4-8x less dense: ST emits 2 DRAW shifts at 4 planes * 6 bytes per opaque column-row (spriteEmitInterleaved68k.c:191-196) = ~3.2KB/shift for an all-opaque 32x32 (~6.4KB for both shifts), and mixed columns cost 12-16 bytes/plane (line 198), up to ~8KB per shift. Amiga is ~3.1KB (shift 0 only) opaque, ~8KB mixed. So with the default (used by examples draw, hello, keys, joy, pattern, audio), the second sprite -- or even the first partially-transparent one -- fails codegenArenaAlloc and runs interpreted with no error. This matches the benched 'compile fails -> interpreted' symptom on the 68k ports.

Fix: Make the default per-platform, e.g. #if 68k/DOS: 32KB, IIgs: 8KB (68k heaps are large; only the IIgs justification in the comment holds there), and correct the comment to state per-target densities.

Impact: Removes silent interpreter fallback (5-8x sprite slowdown) for every app using the default on Amiga/ST

Verifier correction: Directionally right, two details wrong. (1) The named examples (draw/hello/keys/joy/pattern/audio) set codegenBytes=8*1024 explicitly and create no sprites; every sprite-using example sets 32KB+ ? so nothing in the repo currently hits this. It is a latent trap only for user apps that pass codegenBytes=0. (2) The 68k bench fallback is NOT this bug: uber uses 32KB; its interpreter fallback comes from shift coverage (ST compiles only byte-aligned x, Amiga only shift 0; save/restore emit 0 on both). Accurate math: ST all-opaque 32x32 needs ~6.4KB (both shifts, one slot) so the FIRST fits the 8KB default and the second fails; a substantially-mixed 32x32 (~10-16.6KB) fails alone. Amiga: opaque ~3.1KB, fully-mixed ~8.2KB fails alone. Fix: make the default per-platform (e.g. 32KB on 68k/DOS, 8KB IIgs), rewrite the comment with per-target densities, and consider setting an error string or documenting jlSpriteCompiledSize()==0 as the failure probe so the fallback is diagnosable.

40. jlKeyDown's real cost is ~5x its body: cross-TU call overhead around a two-load predicate

  • Where: src/core/input.c:92
  • Severity: medium | Category: perf

The function body is one compare plus one long-addressed byte load (~30 cycles on 65816), but every call pays JSL/RTL plus prologue/epilogue plus parameter marshalling (~120-180 cycles). Games are told to call the predicates per key per frame (include/joey/input.h header comment), so a typical 6-10 key poll burns ~1-1.5K cycles/frame in pure call overhead. Combined with the jlFrameCount forwarding finding, this fully accounts for the 3382 ops/sec bench: jlKeyDown itself is a minority of the measured 830 cycles/iter.

Fix: The state arrays are already extern (src/core/inputInternal.h). Move the declarations into joey/input.h (documented as read-only) and make jlKeyDown/jlKeyPressed/jlKeyReleased and the mouse predicates static inline (or macros) in the header so the compare+load inlines at the call site; keep the out-of-line definitions for ABI/linkage compatibility.

Impact: ~5x per predicate call (~150 -> ~30 cycles); ~1K cycles/frame in a typical input handler on IIgs

Verifier correction: Directionally correct, but the fix must not rely on 'static inline' for the IIgs: ORCA/C 2.2.1 accepts at best but never performs inlining, so under the current baseline compiler header inlines are a no-op (or a parse error). Use macro-form predicates in joey/input.h (like the existing jlp macros in port.h), or guard true 'static inline' behind #if defined(clang)/GNUC for the clang-IIgs/m68k-gcc builds while ORCA keeps the extern functions. Keep the out-of-line definitions for linkage. Note the macro caveat: args are evaluated 2-3 times, so a public macro jlKeyPressed(keys[i++]) would misbehave ? either document plain-lvalue args or restrict macros to the internal build. Realistic win: ~120-150 -> ~30-35 cyc per predicate call on 65816 (~4x, not quite 5x), roughly 1K cyc/frame in a typical handler.

41. port.h comment says 'the asm derives the fill word' but the jlpTileFill/jlpFillCircle macros compute (c & 0x0F) * 0x1111 in C -- a software multiply helper per call on the 65816

  • Where: src/core/port.h:130
  • Severity: medium | Category: perf

Line 126 documents 'tileFill: takes colorIndex; the asm derives the fill word ((c&0x0F) * 0x1111)' -- but the macro body performs the multiply in C and passes the finished fillWord to iigsTileFillInner (prototype at port.h:62 takes 'uint16_t fillWord'). jlpFillCircle does the same at line 99. The comment and code disagree (second source of truth), and the code picked the losing option: when the caller's colorIndex is a variable (the normal case -- core jlTileFill passes the API argument straight through), ORCA-C lowers *0x1111 to its 16x16 software multiply helper. Meanwhile the surrounding comments (port.h:125, surfaceInternal.h:117-120) document dodging that exact helper for *160.

Fix: Make the comment true: change iigsTileFillInner / iigsFillCircleInner to accept the raw nibble and derive the fill word in asm (ASL x4 + ORA, twice: ~12 cycles), and pass '(uint16_t)((_c) & 0x0F)' from the macros. Constant-color call sites lose nothing (the asm expansion is still cheaper than a helper call).

Impact: ~100-300 cycles saved per variable-color jlTileFill / jlFillCircle call on the reference machine (roughly 5-10% of a tileFill-class op), plus removes a comment/code contradiction.

Verifier correction: Directionally right with three detail fixes: (1) the asm derivation costs ~20-30 cycles (pha / 4x asl / ora 1,s gives n*0x11, then sta/xba/ora doubles it into a word), not ~12; still 5-10x cheaper than the multiply helper. (2) 'Constant-color call sites lose nothing' is moot ? the only call sites of these macros (tile.c:121, draw.c:633) pass runtime variables, so nothing constant-folds today anyway. (3) Under the in-progress clang/llvm-mos migration the *0x1111 may be strength-reduced inline (~30-80 cyc with 65816 register spills), shrinking but not eliminating the win; on the ORCA/C baseline the helper-call estimate stands. Also update the fillCircle comment at port.h:90-93, which asserts the C multiply is 'still cheaper than three sequential ORs/shifts' ? false by the repo's own ~150 cyc multiply figure.

42. DOS pays real cross-TU calls to no-op planar hooks on every sprite draw/save/restore and surface copy -- port.h's own comment admits it

  • Where: src/core/port.h:179
  • Severity: medium | Category: perf

The ((void)0) elision block for jlpSurfaceCopyPlanes / jlpSprite{Draw,Save,Restore}Planes (port.h:185-190) is scoped to JOEYLIB_PLATFORM_IIGS only. DOS defines no JL_HAS_SPRITE_* flags, so those hooks resolve to jlpGenericSpriteDrawPlanes/SavePlanes/RestorePlanes -- empty bodies in src/generic/genericSprite.c:10-22 -- called for real: jlSpriteSaveAndDraw's compiled fast path makes two no-op calls per sprite (sprite.c:417-418, since COMPILED_SPRITE_WRITES_PLANES is 0 on DOS per sprite.c:36-39), jlSpriteRestoreUnder one (sprite.c:505/530), jlSpriteDraw one (sprite.c:355), and jlSurfaceCopy one (surface.c:70). All args are pushed, the call executes, nothing happens.

Fix: Move the four planar-hook elision macros out of the IIgs-only block into a '#if defined(JOEYLIB_NATIVE_CHUNKY)' block (covers IIgs, DOS, and the blank template, which is chunky by default); genericSprite.c then compiles to dead code on every shipping chunky port and can be dropped from those builds.

Impact: 3 wasted far-ish calls (~30-60 cycles each with 4-6 pushed args in real-mode x86) per sprite save+draw+restore cycle -- a few percent of DOS per-sprite cost, and deletes an entire misleading TU from chunky builds.

Verifier correction: Mechanism and fix are correct, but the cost model is wrong: DOS builds with DJGPP (i586-pc-msdosdjgpp-gcc, 32-bit protected mode per toolchains/env.sh:61-62), not real mode ? there are no far calls. An empty cdecl call with 4-6 pushed 32-bit args is ~10-25 cycles on 486/Pentium class, not 30-60, so the per-sprite win is ~1-3%, on the port that is already the fastest. The real value is the cleanup: gate the four elision macros on JOEYLIB_NATIVE_CHUNKY (covers IIgs/DOS/blank), making genericSprite.c + jlpGenericSurfaceCopyPlanes dead on all shipping chunky builds. Note the behavior is documented as intentional at port.h:179-181, so also update that comment when applying the fix.

43. jlRandom's 32-bit shift triple (13/17/5) is a worst case for the 16-bit 65816; bit-identical 16-bit-half rewrite available

  • Where: src/core/random.c:23
  • Severity: medium | Category: perf

On the IIgs each 32-bit shift lowers to either a helper call or a per-bit asl/rol loop; 13+17+5 = 35 single-bit 32-bit shifts is ~400-700 cycles per jlRandom call, for a generator explicitly chosen (file header) to avoid multiply cost on the 65816. The same update can be computed on uint16_t halves with far cheaper shifts: x<<13 is hi'=(hi<<13)|(lo>>3), lo'=lo<<13; x>>17 touches only the low half (lo ^= hi>>1, hi unchanged); x<<5 is hi'=(hi<<5)|(lo>>11), lo'=lo<<5. Output stays bit-identical, preserving the cross-port determinism contract in core.h.

Fix: Store gRngState as two uint16_t halves (or unpack/repack locally) and apply the three xors with the 16-bit shift identities above; keep jlRandom returning the recombined uint32_t. Optionally hand-roll the 65816 version per the perf directive -- the >>17 step in particular collapses to a single 16-bit shift and xor.

Impact: ~2-3x per jlRandom call on IIgs (~500 -> ~200 cycles); matters for per-entity per-frame RNG use

Verifier correction: Directionally correct, but make the 16-bit-halves path IIgs-only (or hand-rolled 65816 asm per the perf directive) rather than a portable rewrite: on the 68000, GCC emits native 32-bit shifts (lsl.l/lsr.l at 8+2n cycles plus swap tricks for >>17, ~100 cycles total for all three shifts), so the halves form is a wash-to-slight-loss there. Keep the existing 32-bit C for GCC targets (Amiga/ST/DOS) and gate the decomposed version behind the IIgs platform define; output is bit-identical either way so cross-port determinism is preserved.

44. isFullyOnSurface uses 32-bit adds/compares on every hot sprite entry gate, against the file's own stated convention

  • Where: src/core/sprite.c:103
  • Severity: medium | Category: perf

isFullyOnSurface runs on every jlSpriteDraw (line 346), jlSpriteSaveAndDraw (line 403), and jlSpriteSaveUnder (line 562) call -- the hottest per-frame gates -- and does two int32 add+compare sequences (lines 103 and 106). On ORCA/C 65816 each 32-bit add/compare is a multi-instruction software sequence or helper call. The same file explicitly avoids this pattern 370 lines later: jlSpriteRestoreUnder's comment (lines 469-472) says the edge tests 'don't need 32-bit arithmetic (which would invoke a costly long-compare helper)'. The 32-bit cast exists only because x + widthPx can overflow 16-bit int; rearranging the comparison keeps everything in native 16-bit: widthPx is at most 255*8 = 2040, so SURFACE_WIDTH - (int16_t)widthPx fits int16_t.

Fix: Rewrite as pure 16-bit arithmetic: if (x < 0 || y < 0) { return false; } if (x > (int16_t)(SURFACE_WIDTH - (int16_t)widthPx)) { return false; } if (y > (int16_t)(SURFACE_HEIGHT - (int16_t)heightPx)) { return false; } return true;. No overflow: widthPx/heightPx <= 2040 so the subtraction stays in [-1720, 319].

Impact: ~60-200 cycles per sprite op on IIgs (roughly 1-3% of a 6,400-cycle jlSpriteDraw), on all three hot entry points; also drops two 32-bit compares per call on 68k

Verifier correction: Cost is likely at the low end of the claimed range (~50-120 cycles) since llvm-mos inlines 32-bit adds as lda/adc pairs rather than helper calls, but the win is real on all three gates. Equivalent simpler form also works: with x >= 0 and widthPx <= 2040, (uint16_t)x + widthPx cannot wrap 16 bits, so an unsigned 16-bit add+compare is also correct.

45. spriteDrawInterpreted row setup redoes a per-row multiply and recomputes loop-invariant column math every row

  • Where: src/core/sprite.c:179
  • Severity: medium | Category: perf

(srcY >> 3) * wTiles is a variable 16-bit multiply executed every row (~30-50 cycle helper on 65816) even though the tile band only changes once every 8 rows, and the column terms at lines 181-183 -- inTileX = sx & 7 and ((sx >> 3) << 5) -- are fully loop-invariant (sx never changes across rows) yet are recomputed per row. srcY itself just counts up from sy, so all of tileRowBase is derivable incrementally.

Fix: Hoist srcColBase = ((uint16_t)sx >> 3) << 5 and inTileXStart = sx & 7 above the row loop. Track inTileY = srcY & 7 and a bandBase pointer: seed bandBase once with the (sy >> 3) * wTiles multiply, then per row srcTileRow = sp->tileData + bandBase + (inTileY << 2) + srcColBase; when ++inTileY wraps to 8, reset it and bandBase += (uint16_t)wTiles << 5.

Impact: ~60-90 cycles per row on 65816 (multiply helper + invariant shifts/adds), ~500-700 cycles per 8-row draw = roughly 8-10% of the measured jlSpriteDraw op on IIgs; also removes a MULU-class op per row on DOS.

Verifier correction: Real per-row redundancy with a valid incremental fix, but impact is ~30-100 cycles/row on the interpreted FALLBACK path only (a few percent of that op, not 8-10% of the benchmarked jlSpriteDraw, which ran the compiled path). Worth doing as part of the same interpreted-loop rework as findings 0/1.

46. IIgs/DOS draw fast path never checks SPRITE_NOT_COMPILED before jumping into the arena

  • Where: src/core/sprite.c:346
  • Severity: medium | Category: bug

jlSpriteDraw gates only on sp->slot != NULL, and the IIgs and DOS spriteCompiledDraw variants (spriteCompile.c lines 309-313 and 705-706) add sp->routineOffsets[shift][SPRITE_OP_DRAW] to the slot address with no sentinel check. jlSpriteCompile explicitly stores SPRITE_NOT_COMPILED (0xFFFF) per (shift,op) whenever an emitter returns 0 (spriteCompile.c line 233-234), so if any DRAW shift ever emits 0 bytes while the slot still allocates, fnAddr = arenaBase + slot->offset + 0xFFFF and the CPU JSLs/calls into arbitrary arena bytes -> crash. The sibling dispatchers already guard: jlSpriteSaveUnder checks routeOffset != SPRITE_NOT_COMPILED (sprite.c line 568), and the Amiga/ST spriteCompiledDraw check and delegate to jlpSpriteDrawPlanes (spriteCompile.c lines 543, 658). Today the IIgs/x86 draw emitters happen to emit both shifts, so this is latent, but it is the only dispatch path with no guard.

Fix: In jlSpriteDraw, fold the DRAW offset check into the fast-path gate (read routineOffsets[shift][SPRITE_OP_DRAW] once, compare against SPRITE_NOT_COMPILED, and pass the offset down per the duplication finding below), falling back to spriteDrawInterpreted when absent -- mirroring the SaveUnder/RestoreUnder gates.

Impact: Removes a latent jump-to-garbage crash; combined with the dispatch-dedup fix it costs zero extra cycles

Verifier correction: Not triggerable today: the IIgs/x86 DRAW emitters structurally always emit for shifts 0/1 (the only shifts those dispatchers select), so this is a latent contract violation, not an active crash. The stronger framing: spriteInternal.h:74-76 documents that dispatchers must check routineOffsets != SPRITE_NOT_COMPILED before calling spriteCompiledDraw, and jlSpriteDraw is the only dispatcher that skips it. Hoisting the guard (via SPRITE_SHIFT_INDEX + one table read passed down to spriteCompiledDraw) is cycle-neutral and lets the redundant Amiga/ST in-callee checks be deleted.

47. Four divergent implementations of the 'row is dirty' test; the STAGE_DIRTY_ROW_CLEAN macro that should be the single truth is unused

  • Where: src/core/surfaceInternal.h:54
  • Severity: medium | Category: multi-source-of-truth

grep shows this macro has zero uses. Each present hand-rolls its own predicate: DOS if (minWord > maxWord) { continue; } (src/dos/hal.c:289), ST if (gStageMinWord[y] <= gStageMaxWord[y]) (src/atarist/hal.c:665), IIgs asm cmp gStageMaxWord,x / bcc+beq (joeyDraw.s:2903-2911), but Amiga tests if (gStageMinWord[y] != STAGE_DIRTY_CLEAN_MIN) (src/amiga/hal.c:752 and 845). The Amiga form is semantically different for a band where only maxWord was widened (min still 0xFF, max > 0 -- reachable today via the unclipped jlDrawPixel bug): IIgs/DOS/ST treat that row dirty, Amiga treats it clean. Four expressions of one invariant is exactly the multiple-sources-of-truth drift the cleanup checklist targets.

Fix: Make the C ports use STAGE_DIRTY_ROW_CLEAN(y) (or a positive STAGE_DIRTY_ROW_DIRTY twin) and change Amiga's min!=0xFF test to the min>max form; keep a comment in joeyDraw.s pinning the asm compare to the macro's definition. If unification is declined, delete the unused macro.

Impact: Consistency across 3 C ports + 1 asm port; removes a latent Amiga skip-row divergence

Verifier correction: The multi-source-of-truth cleanup is real, but the 'latent Amiga skip-row divergence' cannot drop real content. (1) Amiga's min!=0xFF tests at hal.c:752/845 are only the idle-frame early-out and the anyDirty snapshot flag; the actual per-row copy decision (hal.c:786) uses rowMin > rowMax, identical semantics to the other ports. (2) The divergent state (min=0xFF, max=0xFF) is reachable only through the non-IIgs jlDrawPixel path (draw.c:549 marks dirty with raw unclipped x,y after plotPixelNoMark discarded the off-surface pixel; x in [-4,-1] gives word index 0xFF) ? a state where no pixel was written, so Amiga treating the row clean is correct and the other ports merely do wasted copy work. widenRow (surface.c:47-54) guarantees any in-bounds write sets min<=79<0xFF, so Amiga never misses real content. Amiga's comment at hal.c:742-743 documents this invariant deliberately. Right fix: have DOS/ST use STAGE_DIRTY_ROW_CLEAN (or a dirty-positive twin), keep Amiga's cheaper single-array test but name it as a second macro (e.g. STAGE_DIRTY_ROW_TOUCHED) documented as equivalent under the clipped-marks invariant, and pin the joeyDraw.s compare to the macro with a comment. Separately worth flagging: the real bug this exposes is draw.c:549 passing raw x,y into surfaceMarkDirtyRect ? an out-of-range y indexes gStageMinWord[y] out of bounds on DOS/Amiga/ST (potential OOB write via widenRow), which is more severe than the predicate drift itself.

48. tile.c wrappers derive pixel coords with * TILE_PIXELS_PER_SIDE multiplies where port.h deliberately uses shifts to dodge the ORCA-C multiply helper

  • Where: src/core/tile.c:79
  • Severity: medium | Category: perf

All five public tile wrappers (jlTileCopy 79-80, jlTileCopyMasked 99-100, jlTileFill 117-118, jlTilePasteMono 141-142, jlTilePaste 157-158) compute the dirty-rect pixel coords with a runtime multiply. The IIgs macro layer in port.h uses (_by) << 3 / (_bx) << 2 for the same derivation, with the comment "Use SURFACE_ROW_OFFSET (LUT lookup) to dodge a software multiply helper", and surfaceInternal.h documents that ORCA-C emits a multiply helper even for the implicit *2 of uint16_t array indexing. If ORCA-C does not strength-reduce *8 (the surrounding code assumes it does not), each tile-op call pays up to two ~150-cycle helper calls purely for dirty-rect bookkeeping -- against jlTilePaste's measured ~2530 cyc/call that is up to ~12%. Two derivations of the same block-to-pixel mapping (wrapper multiplies vs port.h shifts) is also a mild dual-source-of-truth.

Fix: Replace with shifts: dstPixelX = (uint16_t)((uint16_t)dstBx << 3); (same for the other nine sites), or add a TILE_BLOCK_TO_PIXEL(_b) macro next to TILE_PIXELS_PER_SIDE in tile.h and use it in both places.

Impact: up to ~300 cyc/call (~12% of jlTilePaste) on IIgs under ORCA-C; zero-risk on all other ports

Verifier correction: Impact is 'up to ~300 cyc' contingent on ORCA-C 2.2.1 not strength-reducing *8 (strongly indicated by surfaceInternal.h's documented *2-indexing helper, but worth a one-off disassembly check); on gcc/Watcom ports it is a pure wash. Also note jlpGenericTilePasteMono/jlpGenericTileFill in src/generic/genericTile.c:134-135 repeat the same * TILE_PIXELS_PER_SIDE pattern -- if a TILE_BLOCK_TO_PIXEL(_b) macro is added to tile.h, use it there too for one source of truth (harmless on the gcc ports that compile it, but consistent).

49. jlpGenericFillRect recomputes loop-invariant span bytes/edge masks and the row offset every row

  • Where: src/generic/genericDraw.c:42
  • Severity: medium | Category: perf

x and w are constant across the row loop, so pxStart/pxEnd, the odd-left-edge test (line 44), midBytes (line 48), and the odd-right-edge test (line 53) resolve identically for every row -- but all are re-derived per row, including two conditional branches. The row base is also re-derived per row via SURFACE_ROW_OFFSET(y + row) (a LUT read on IIgs, a *160 on DOS if OpenWatcom fails to strength-reduce) instead of advancing one pointer by SURFACE_BYTES_PER_ROW. This is the fill used by every IIgs NON-stage fillRect (the jlpFillRect macro defers to it, port.h:120) and every DOS fillRect.

Fix: Hoist before the loop: compute firstByteIndex, hasLeftNibble, midBytes, hasRightNibble once from x/w; inside the loop keep a single uint8_t *line advanced by line += SURFACE_BYTES_PER_ROW, doing (optional left RMW) + memset + (optional right RMW) with the precomputed constants.

Impact: ~20-40 cycles/row on 65816 (2 branches + adds + LUT read): ~10% of a 16x16 non-stage fill on IIgs; smaller but free win on DOS

Verifier correction: Directionally right and if anything understated: per-row waste under ORCA-C is ~40-90+ cycles once the SURFACE_ROW_OFFSET LUT read plus the 32-bit pointer rebuild from s->pixels are counted, so the win on an IIgs non-stage 16x16 fill is likely 15-30%, not ~10%. Two scope caveats: (1) this does NOT affect the UBER 450 ops/sec jlFillRect16x16 number, which measures the IIgs stage asm path (iigsFillRectInner); (2) h==1 span fills (circle scanlines / axis-aligned lines routed through fillRectOnSurface on DOS) gain nothing from the hoist since the 'loop' runs once. Beneficiaries are IIgs offscreen-surface fills, all DOS fills, and the rare Amiga/ST portData==NULL fallback.

50. Identical 4-byte row-copy inner loop triplicated byte-at-a-time across TileCopy, TilePaste, and TileSnap

  • Where: src/generic/genericTile.c:27
  • Severity: medium | Category: duplication

The exact same statement appears in jlpGenericTileCopy (line 27), jlpGenericTilePaste (line 107), and jlpGenericTileSnap (line 125); the three loops differ only in the two stride constants. Beyond the duplication, the copy is 4 single-byte moves where the surface side is always 4-byte aligned (heap base plus by8160 + bx*4). On the 16-bit DOS target (the only port that runs these -- IIgs uses asm macros, Amiga/ST override all five tile ops per platform.h lines 95-99/141-146/196-201) two word moves or an inlined memcpy(d, s, 4) halves the memory operations. jlTileT.pixels is only byte-aligned, so use memcpy(d, s, TILE_BYTES_PER_ROW), which Watcom/gcc inline optimally and which stays safe for a future 68k-family blank port where a misaligned word move is a crash.

Fix: Add one static inline helper, e.g. static inline void genericTileRowsCopy(uint8_t *d, uint16_t dStride, const uint8_t *s, uint16_t sStride) { for (row...) { memcpy(d, s, TILE_BYTES_PER_ROW); d += dStride; s += sStride; } } and call it from all three functions with the appropriate strides.

Impact: ~2x fewer inner-loop memory ops on DOS tile copy/paste/snap; collapses 3 duplicate loops into 1 shared routine

Verifier correction: Two detail fixes: (1) DOS is DJGPP gcc, 32-bit protected mode (env.sh:62), not '16-bit'; memcpy compiles to one 32-bit move per row, so the win is 8->2 memory ops (~4x), better than claimed. (2) The reason the current form is slow is that uint8_t* d/s may alias, which blocks gcc store merging ? memcpy asserts non-overlap and unlocks it. Nit: jlTileCopy documents src==dst surfaces as legal; same-block copy gives d==s, technically UB for memcpy though harmless with the generated dword move (blocks at distinct coords never partially overlap). Keep the helper static inline; it only needs to serve DOS/BLANK.

51. jlpGenericTileCopyMasked does a read-modify-write merge even when both source nibbles are opaque

  • Where: src/generic/genericTile.c:59
  • Severity: medium | Category: perf

For a byte whose two nibbles are both non-transparent (the majority of bytes in solid glyph strokes and most tileset art), the result is simply srcByte, yet the code loads d[col], performs two masked merges, and stores. Only the mixed case (exactly one transparent nibble) needs the destination read. This is the DOS masked-copy inner loop (IIgs uses iigsTileCopyMaskedInner, Amiga/ST override with planar functions), running 32 times per tile.

Fix: Restructure the per-byte body: if (srcHi != transHi && srcLo != transLo) { d[col] = srcByte; continue; } then fall through to the existing skip/merge logic for the transparent and mixed cases.

Impact: saves one memory load and ~4 ALU ops per fully-opaque byte (typically 50-90% of non-skipped bytes) in the DOS masked tile copy

Verifier correction: Correct as stated, but scope is DOS/BLANK only (plus unreachable Amiga/ST portData==NULL fallbacks); severity is closer to low-medium since DOS is rarely the bottleneck. IIgs, Amiga, and ST hot paths are untouched by this fix.

52. iigsBlitStageToShr's SCB/palette MVN upload paths are dead, yet jlpPresent still marshals both far pointers into the asm every present

  • Where: src/iigs/hal.c:298
  • Severity: medium | Category: dead-code

uploadFlags is hard-coded 0 at the only call site, so the flag-gated SCB (200-byte) and palette (512-byte) MVN blocks in joeyDraw.s (lines 2787-2851, lda bsFrame+8 ; uploadFlags gates both) can never execute -- hal.c's own comment (lines 290-292) says 'The asm's now-unreached SCB/palette MVN code is dead -- a removal candidate.' The cost is not just dead bytes in the bank-limited DRAWPRIMS load segment: every present still pushes two 4-byte far pointers plus the flags word, and the asm entry copies all ten bytes into its rebuilt D-frame (joeyDraw.s:2763-2772) before ever looking at a dirty band.

Fix: Change the entry to iigsBlitStageToShr(void): delete the scbPtr/palettePtr/uploadFlags marshalling and D-frame rebuild in joeyDraw.s, delete the two flag-gated MVN blocks, and call it with no args from jlpPresent (uploadScbAndPaletteIfNeeded already owns SCB/palette upload).

Impact: ~50-100 cyc per present of argument marshalling + frame rebuild; frees dead code space in the crowded DRAWPRIMS bank

Verifier correction: Two detail fixes: (1) ABI ? per the clang w65816 cdecl documented at joeyDraw.s:2716-2719, scbPtr (arg0) arrives in A:X and is never pushed; only palettePtr (4 bytes) + uploadFlags (2 bytes) go on the stack, so it is one pushed far pointer plus a word, not two. (2) Fix scope ? when deleting the marshalling (joeyDraw.s:2765-2772) and both flag-gated MVN blocks (2787-2859), KEEP the caller-DBR stash at joeyDraw.s:2780-2784 (sep/lda 3,s/sta 0xE19DFE/rep): the narrow-row MVN path in phase 3 reloads $E1:9DFE at line 3122 after every MVN to restore DBR. Also delete the now-orphaned bsFrame BSS block (joeyDraw.s:4417-4420), change the extern at hal.c:98 and the call at hal.c:298 to zero-arg, and update the stale jlpInit comment at hal.c:270-273 that still says iigsBlitStageToShr uploads SCB/palette. Impact restated: ~90-110 cyc/present (~0.15% of a full-screen present, ~1-2% of an all-clean one) plus ~150 bytes freed in the crowded DRAWPRIMS bank ? value is primarily dead-code cleanup, not measurable perf.

53. iigsDrawPixelInner carries dead PHB/PLB framing and a long-absolute scratch round-trip in the hottest inner op

  • Where: src/iigs/joeyDraw.s:1416
  • Severity: medium | Category: perf

The routine (joeyDraw.s:1412-1487) never writes DBR -- every instruction between phb and plb is rep/sep, stack-relative loads, DP stores after tcd, absolute loads, and [pix],y accesses -- so the phb/plb pair (3+4 cyc) is pure dead weight. The offset math also stores the LUT row into memory and immediately re-adds it ('sta dpxlTmp ... lda 8,s / lsr a / clc / adc dpxlTmp', lines 1441-1446) where reordering to 'lda 8,s ; lsr a ; clc ; adc gRowOffsetLut,x' saves the ~11-cycle round-trip; and dpxlTmp/dpxlNibPart live in absolute bss (4-cyc accesses) even though D is already repointed at a bank-0 scratch block, so widening dpxlScratch and addressing them D-relative saves ~2 cyc per access. This is the Phase-2.2 residual from IIGS_PERF_PLAN.md, still unfixed. Total ~25-35 cycles out of a ~140-cycle inner routine that runs once per jlDrawPixel and per Bresenham plot.

Fix: Delete phb/plb (DBR is never modified); fold the row-offset add into 'adc gRowOffsetLut,x' directly instead of via dpxlTmp; extend dpxlScratch to 8 bytes and address tmp/nibPart as D-relative offsets 4/6.

Impact: ~25-35 cycles of ~140 (about 20% of the inner plot, ~6-8% of the full post-Phase-2 jlDrawPixel call); same pattern audit applies to the line/circle inners

Verifier correction: Valid parts: (1) delete phb/plb ? DBR is never written in iigsDrawPixelInner ? saving 7 cycles, and renumber the stack-relative arg offsets from 8/10/12,s to 7/9/11,s (and the comment at lines 1402-1405); (2) fold the row offset directly with 'lda 8,s / lsr a / clc / adc gRowOffsetLut,x' (X still holds y*2), eliminating dpxlTmp and its store/reload for ~10 cycles. Drop the third sub-fix: relocating scratch to D-relative offsets in dpxlScratch saves ~0 cycles (DL != 0 makes dp access 5 cyc, same as absolute) and is hazardous ? D-relative stores land in bank 0 at (dpxlScratch & 0xFFFF), not at the data-bank .bss symbol (the routine's own comment documents this disagreement), so widening the .bss reservation does not reserve the bank-0 bytes actually written. Net win ~17 cycles of a ~150-cycle inner (~11% of the inner, ~1% of the measured 1595-cycle jlDrawPixel call, roughly 1755 -> 1774 ops/sec), not 25-35 cycles / 6-8% of the call. Also: iigsDrawPixelInner runs once per jlDrawPixel and per plot only in the clipped Bresenham C fallback; the on-surface line/circle fast paths plot inline in their own asm inners (which deserve the same phb/plb and round-trip audit, per IIGS_PERF_PLAN.md Phase 4).

54. spriteEmitIigs.c top-of-file contract describes the deleted self-modifying-stub design; the code below it emits the opposite (C-ABI prologue)

  • Where: src/iigs/spriteEmitIigs.c:3
  • Severity: medium | Category: multi-source-of-truth

The #19 rewrite (documented in spriteCompile.c:133-146) replaced the self-modifying stub with plain C function pointers, and this same file emits the C-ABI prologue at lines 336-342 (PHB/TAY/SEP/TXA/PHA/PLB) -- directly contradicting its own header ('No stack arg, no prologue', 'DBR = program bank', 'Y = destRow loaded by stub'). spriteEmitter.h:75 repeats the stale claim ('IIgs uses a self-modifying MVN-stub on top of these bytes'), and spriteEmitter.h:6-9 claims 'The planar 68k emitters now return 0 bytes after Phase 11', contradicted by spriteEmitPlanar68k.c emitting real shift-0 DRAW bytes (and by spriteEmitter.h's own lines 94-96). Anyone editing the IIgs emitter against its stated contract would break the real one (e.g., dropping the prologue back out).

Fix: Rewrite the spriteEmitIigs.c header to describe the C-ABI convention (arg in A:X, PHB prologue, PLB/RTL epilogue), and fix spriteEmitter.h lines 5-9 and 75 to match the current emitters.

Impact: Removes actively-misleading contracts on the two files most likely to be hand-edited for perf work

Verifier correction: Finding is accurate as written. One addition for the fix scope: src/codegen/spriteCompile.c:172-178 is a third stale source of truth ? it claims the Amiga planar DRAW 'emits a unique pre-shifted variant per shift in 0..7', but spriteEmitDrawPlanar68k returns 0 for any shift != 0 (only shift 0 is emitted, matching spriteEmitter.h:94-96). The doc rewrite should fix that comment too. Precise conventions for the new header text: DRAW is void draw(uint8_t *dstRow0) with the 24-bit pointer arriving as A = low 16 bits, X = bank; SAVE/RESTORE are void copy(uint32_t packed) with packed = srcOff | (dstOff << 16), arriving as A = srcOff, X = dstOff; all preserve caller DBR via PHB/PLB and return via RTL with M=16.

55. spriteEmit68k.c (chunky 68k emitter) is compiled into Amiga and ST builds but nothing calls it

  • Where: src/m68k/spriteEmit68k.c:179
  • Severity: medium | Category: dead-code

spriteCompile.c dispatches exclusively to spriteEmitDrawPlanar68k (Amiga), spriteEmitDrawInterleaved68k (ST), spriteEmitDrawX86, and spriteEmitDrawIigs (lines 52-99). No source file references spriteEmitDraw68k/spriteEmitSave68k/spriteEmitRestore68k (grep across src/, include/, make/ finds only the definitions, the prototypes in spriteEmitter.h lines 64/83-84, and the .o rules), yet make/amiga.mk line 57 and make/atarist.mk line 42 both build and link spriteEmit68k.o. This is the pre-planar chunky emitter that the planar/interleaved rewrites replaced; it also redefines TILE_PIXELS/TILE_BYTES/TILE_BYTES_PER_ROW locally instead of using joey/tile.h.

Fix: Remove spriteEmit68k.c, its prototypes in spriteEmitter.h, and the $(BUILD)/obj/68k/spriteEmit68k.o entries from make/amiga.mk and make/atarist.mk (or move the file to stuff/ if it is wanted as reference for a future chunky-68k target).

Impact: ~330 dead lines removed from two ports' builds and link images; eliminates a stale second copy of the tile-geometry constants

Verifier correction: Correct as a dead-code finding, with two refinements: (1) the 'link images' impact is overstated ? nothing references the symbols, so the static-archive member is never pulled into final binaries; the real cost is ~330 dead source lines, two compile units, and a duplicate copy of the tile-geometry constants. (2) The fix should also sweep stale references the finding missed: the 'spriteEmit68k.c shared with ST/Amiga' comments at make/amiga.mk:45-47 and make/atarist.mk:31-33, the '68k chunky (unused)' sizing comment block in spriteEmitter.h (keep SPRITE_EMIT_MAX_BYTES_PER_DESTBYTE_68K itself ? it still sizes the Amiga/ST scratch), and docs/amiga_planar.md:5 / docs/atarist_planar.md:6 which cite the file under its old src/codegen/ path.

56. c2pColumn/putWord duplicated byte-for-byte between the two 68k emitters; shiftedByteAt/spriteSourceByte duplicated between IIgs and x86 emitters

  • Where: src/m68k/spriteEmitInterleaved68k.c:83
  • Severity: medium | Category: duplication

spriteEmitInterleaved68k.c:83-116 (c2pColumn + putWord) is character-identical to spriteEmitPlanar68k.c:89-122; spriteEmitIigs.c:52-113 (shiftedByteAt + spriteSourceByte) is identical to spriteEmitX86.c:168-232 (modulo comments); the dead spriteEmit68k.c carries a third copy. These helpers ARE the transparency/c2p contract that must match the interpreters bit-for-bit (the planar file even says 'Identical pixel walk to jlpSpriteDrawPlanes'), so a fix applied to one copy and not the others produces per-platform sprite corruption that only shows on the un-fixed target. Constants TILE_BYTES/TILE_BYTES_PER_ROW/TILE_PIXELS/TRANSPARENT_NIBBLE are also re-defined per file instead of coming from joey/tile.h as sprite.c does.

Fix: Add src/codegen/spriteEmitCommon.c/.h with c2pColumn, putWord (68k), shiftedByteAt, spriteSourceByte, and take the tile geometry macros from joey/tile.h; each emitter includes it. Emit-time only, so the extra call is free at runtime.

Impact: Removes ~160 duplicated lines and 3-way drift risk in the pixel-walk contract

Verifier correction: Directionally right; refine the fix: (1) split the common code by family instead of one spriteEmitCommon.c -- put c2pColumn/putWord in src/m68k/ (only Amiga/ST build that dir) and shiftedByteAt/spriteSourceByte in src/codegen/ shared by DOS+IIgs -- otherwise the linker's whole-archive-member pull drags 68k c2p helpers into DOS/IIgs binaries; (2) joey/tile.h names the constant TILE_PIXELS_PER_SIDE, not TILE_PIXELS, so emitters need the rename, and TRANSPARENT_NIBBLE is NOT in tile.h (src/core/sprite.c:22 re-defines it locally too) -- hoist it into spriteInternal.h, which every emitter and sprite.c already includes; (3) delete the dead spriteEmit68k.c outright along with its LIB_OBJS lines in make/amiga.mk:57 and make/atarist.mk:42 and its prototypes in spriteEmitter.h:64,83-84, rather than de-duplicating into it; (4) on IIgs, moving statics out of spriteEmitIigs.c changes ORCA-Linker segment packing (known order-sensitive), so rebuild and re-verify link order after the refactor.

57. surface.h documents jlSurfaceHash as FNV-1a but the implementation is a 31/251 two-stream product hash

  • Where: include/joey/surface.h:66
  • Severity: low | Category: cleanup

jlpGenericSurfaceHash (src/generic/genericSurface.c:61-90) mixes via SURFACE_HASH_MIX_BYTE, the strength-reduced lo *= 31 + b / hi *= 251 + b pair defined in surfaceInternal.h:77-80 -- not FNV-1a (no 0x811c9dc5 offset basis, no xor-then-multiply, different prime). This is not just a stale comment: the hash's whole purpose is bit-exact cross-port comparison against the IIgs golden reference, and Amiga/ST supply their own plane-reading overrides. Anyone writing a new port override from the public header's description would implement real FNV-1a and silently fail every UBER pixel-compare.

Fix: Rewrite the header comment to describe the actual algorithm ('two-stream 16-bit multiplicative hash, streams combined hi<<16|lo; must match SURFACE_HASH_MIX_BYTE in surfaceInternal.h byte-for-byte across ports'), pointing overriders at the macro as the single definition.

Impact: Prevents a future port override from breaking cross-port hash validation; doc-only change

Verifier correction: The header is wrong about more than the algorithm: it also misstates the hashed domain. The real hash covers the packed chunky byte stream (32000 bytes, 2 px/byte -- not 64000 per-pixel indices), then the 200 SCB bytes, then the 256 palette uint16s folded high-byte-then-low, seeded lo=0xACE1/hi=0x1357, returned as (hi<<16)|lo. The comment rewrite should state all of that (algorithm, seeds, domain, combine order) and name SURFACE_HASH_MIX_BYTE in surfaceInternal.h as the single source of truth that any port override must reuse byte-for-byte.

58. ST jlpTilePasteMono dereferences dst before its own NULL check, and both the dst and duplicate pd NULL checks are dead

  • Where: src/atarist/hal.c:1410
  • Severity: low | Category: unnecessary-conditional

Line 1410 reads dst->portData, then line 1414 checks 'dst == NULL' -- if dst could be NULL the check is too late; in fact it cannot be (the only caller, jlTilePasteMono at src/core/tile.c:129, validates 'dst == NULL || in == NULL' before dispatch), so lines 1414-1416 are dead either way. Line 1418's 'pd == NULL' recheck is also dead: line 1410 already returned on portData == NULL. The Amiga counterpart (amiga/hal.c:1816-1826) has none of these extra checks. This is on the ST mono-tile hot path (Space Taxi logo repaint, 103 calls every 4th frame).

Fix: Delete lines 1414-1416 and the redundant pd NULL check at 1418-1420; keep only the portData==NULL generic-fallback branch (or drop even that, since ST surfaces always carry portData post-Phase-9 and the generic fallback writes to pixels which is always NULL on ST, i.e. it silently does nothing anyway).

Impact: Removes 2 dead conditionals (~20-30 cycles) per mono tile paste on 68000, and removes a misleading 'fallback works' impression -- the generic fallback branch is actually a silent no-op on ST.

Verifier correction: All details check out. Two refinements: (1) the caller reference should be tile.c:140 (dispatch) with the NULL validation at tile.c:129; (2) the identical dead-check pattern (deref at 1454, dst/chunkyOut NULL check at 1458, pd recheck at 1462) exists in the adjacent jlpTileSnap and should be cleaned in the same pass.

59. SPLIT_BACKUP_CACHED global single-entry cache thrashes and costs more than it saves with multiple backup buffers

  • Where: src/codegen/spriteCompile.c:362
  • Severity: low | Category: perf

The cache assumes one backup buffer per program ('effectively never changes after the first call', line 354), but the normal pattern is one jlSpriteBackupT + buffer per moving sprite. With two or more sprites saved/restored per frame the pointer ping-pongs, so every call misses: it pays a 24-bit pointer compare (a multi-instruction 32-bit compare on the 65816) and then does the full SPLIT_POINTER anyway plus three global stores. Even on a hit, the 32-bit compare costs a comparable amount to SPLIT_POINTER itself, which is just four byte loads and two stores. The right home for the cached split is the backup record: save already computes it, and restore reuses the same backup.

Fix: Add uint16_t bytesLo; uint8_t bytesBank; to jlSpriteBackupT (or a per-sprite cache beside cachedDstBank), have spriteCompiledSaveUnder store the split it computes, and have spriteCompiledRestoreUnder read it; delete gLastBackupBytes/-Lo/-Bank and SPLIT_BACKUP_CACHED.

Impact: ~20-40 cycles per save/restore call on IIgs whenever a game uses more than one backup buffer (the common case); removes global mutable state

Verifier correction: Impact estimate is right but the fix touches the public jlSpriteBackupT struct; either accept that (source-compatible addition, document the fields as internal) or take the simpler fix: delete gLastBackupBytes/SPLIT_BACKUP_CACHED and use SPLIT_POINTER unconditionally ? it already beats the cache whenever more than one backup buffer is live, which every real example in the repo is.

60. assetLoad.c comment claims file-IO lives in a named IIgs segment, but no segment directive exists anywhere

  • Where: src/core/assetLoad.c:28
  • Severity: low | Category: multi-source-of-truth

The comment describes a load-bearing mitigation for the documented ORCA linker 64KB-bank failure mode, but the file contains no #pragma/segment directive and make/iigs.mk assigns none -- the mitigation it documents does not exist. Whoever next hits 'Code exceeds code bank size' will be misled into believing the stdio cluster is already segregated. Either the directive was lost in the ORCA->clang migration or the comment was aspirational.

Fix: Either implement it (a section attribute / link-script placement under the clang toolchain) or delete the comment and note the real current placement; one source of truth between comment and build.

Impact: Debugging-time hazard only; no runtime cost

Verifier correction: Accurate as filed; one refinement: the stdio-bank-bursting failure mode itself was an ORCA-Linker-era problem, and the clang pipeline's multi-segment emitter now handles >64KB overflow automatically, so the comment is doubly stale -- the right fix is almost certainly to delete/rewrite it rather than implement a named segment under link816.

61. plotPixelNoMark re-checks s == NULL per pixel although every caller has already validated it

  • Where: src/core/draw.c:276
  • Severity: low | Category: unnecessary-conditional

All three callers -- jlDrawCircle (line 299), jlDrawLine (line 399), and non-IIgs jlDrawPixel (line 518) -- test s == NULL before reaching plotPixelNoMark, so the NULL branch inside it can never be taken; in the Bresenham circle fallback it executes 8 times per iteration. This is the 'validation repeated at multiple levels' pattern: on 68000 a tst.l+beq on a 32-bit pointer is ~10-12 cycles per plotted pixel, on top of the per-pixel function call itself (plotPixelNoMark is not inlined by ORCA/C 2.2.1).

Fix: Delete the NULL check from plotPixelNoMark and document the non-NULL precondition in its comment (its callers are all in this TU and already comply); keep the bounds check, which is the function's actual job.

Impact: ~10 cycles per plotted pixel in the line/circle fallback loops (x8 per circle iteration); pure dead branch removal

62. isFullyOnSurface predicate duplicated between sprite.c helper and jlDrawRect's inline test

  • Where: src/core/draw.c:574
  • Severity: low | Category: duplication

This is the exact predicate implemented by the static isFullyOnSurface() in src/core/sprite.c:99-110 (including the same int32_t-widening trick for the right/bottom edges). The same rect-on-surface question is also answered ad hoc in jlDrawLine's H/V span guards (draw.c:425-426, 443-444). Two hand-maintained spellings of one clip invariant is precisely the 'same clip computation in draw.c and sprite.c' class on the cleanup checklist.

Fix: Move isFullyOnSurface into surfaceInternal.h as a static inline (or macro, for ORCA/C which does not inline) named e.g. SURFACE_RECT_ON_SURFACE(x, y, w, h), and use it from both sprite.c (3 call sites) and jlDrawRect.

Impact: One source of truth for the on-surface fast-path gate used by sprites, rects, and spans; no runtime change

63. jlSpriteDraw's fast-path comment claims the IIgs never takes it, contradicting the codegen default

  • Where: src/core/sprite.c:344
  • Severity: low | Category: multi-source-of-truth

spriteCompile.c lines 144-146 set JOEY_IIGS_SPRITE_CODEGEN to 1 by default ('Codegen is ON by default'), and the file's #19 block describes the rewritten C-ABI IIgs codegen as live -- so on IIgs sp->slot is NOT left NULL and this fast path IS the production path (jlSpriteDraw 438 ops/sec is the compiled path). A reader trusting this comment would conclude the IIgs compiled branch is dead and could 'clean it up' or skip optimizing it. Related stale docs in the public header compound this: sprite.h line 88 says codegen is 'currently x86/DOS' and lines 17-21 describe jlSpritePrewarm as a no-op under an interpreter-only v1, while all four ports now have emitters wired.

Fix: Rewrite the sprite.c comment to describe current behavior (IIgs compiled path live; interpreter is the clip-edge/uncompiled fallback), and update sprite.h's jlSpriteCompile/jlSpritePrewarm prose to name the actual per-port status.

Impact: Documentation-only, but it misdirects optimization effort on the reference platform's hottest sprite path

Verifier correction: Add a third stale instance the finding missed: spriteInternal.h:74 still describes the IIgs compiled entry points as 'inline asm + self-modifying stub on IIgs', contradicting the rewritten C-ABI dispatch documented in spriteCompile.c:133-143 and :285-293. All three doc sites should be fixed together.

64. routineOffsets shift*3+op byte-offset idiom hand-expanded at three sites in sprite.c

  • Where: src/core/sprite.c:409
  • Severity: low | Category: duplication

The multiply-free routineOffsets[shift][op] access ((shift<<1)+shift+op, then <<1 for bytes) is spelled out independently in jlSpriteSaveAndDraw (lines 409-413), jlSpriteRestoreUnder (lines 500-501), and jlSpriteSaveUnder (lines 566-567), each with its own copy of the 'dodges ~MUL4' comment. The encoding must stay in lockstep with SPRITE_OP_COUNT == 3 and with the emitters in src/codegen/spriteCompile.c that fill the same table; a fourth op added to SPRITE_OP_COUNT would require finding and fixing three hand-rolled *3 expansions.

Fix: Add to spriteInternal.h, next to SPRITE_NOT_COMPILED: #define SPRITE_ROUTINE_OFFSET(_sp, _shift, _op) (*(uint16_t *)((uint8_t *)(_sp)->routineOffsets + ((uint16_t)((((uint16_t)(_shift) << 1) + (_shift) + (_op))) << 1))) and use it at all three sites (identical codegen, since it is the same expression).

Impact: Collapses 3 duplicated pointer-math blocks into one macro co-located with the table's layout constants; no runtime change

Verifier correction: Scope is slightly understated: the same shift*3+op idiom also appears twice in src/codegen/spriteCompile.c (:423-427 spriteCompiledSaveUnder, :478-482 spriteCompiledRestoreUnder), where the index additionally feeds the cachedDstBank/cachedSrcBank pointers ? so the shared definition should expose the index computation (e.g. a SPRITE_ROUTINE_INDEX(shift, op) macro) that both the offset-read macro and the IIgs cache-pointer math build on, covering all five sites.

65. surfaceMarkDirtyRect re-validates w/h that its contract already guarantees positive

  • Where: src/core/surface.c:245
  • Severity: low | Category: unnecessary-conditional

The function's own contract comment (lines 230-233: 'Drawing primitives pass the rect they actually wrote (already clipped to surface bounds, w and h positive)') makes this check dead for conforming callers, and I verified every call site: fillRectOnSurface is caller-proven positive, the line/circle accumulated boxes only mark after a minX <= maxX && minY <= maxY clamp (draw.c:379, 511), flood spans are rightX-leftX+1 >= 1, sprite paths mark only after clipRect/isFullyOnSurface succeed, and tile.c passes constant 8s after tile-coord validation. Two signed compares + branches per draw op (~10-15 cyc on the 65816) on the most-called internal function in the library. Note this check does NOT protect against the real contract violation (finding 1) -- jlDrawPixel passes w=h=1 with a bad y -- so it must not be treated as the safety net; it can be dropped once the jlDrawPixel clip fix lands.

Fix: After fixing jlDrawPixel's unclipped mark, delete the if (w <= 0 || h <= 0) early-out (optionally demote to a debug-build assert), leaving only the s != gStage gate.

Impact: ~10-15 cyc on every dirty-marking draw op on IIgs; two branches on 68k ports

Verifier correction: The check is not literally dead today: jlDrawRect(h==2) reaches surfaceMarkDirtyRect with h==0 via fillRectOnSurface (draw.c:582/584). Deleting the check is still behaviorally safe because both the portable mark loop and the top-tested IIgs asm treat empty ranges as no-ops (and iigsFillRectInner is also h==0-safe), but the cleanest fix is to also make jlDrawRect skip the interior-edge fills when h==2, restoring fillRectOnSurface's documented contract before dropping the guard.

66. SURFACE_PIXELS_PER_WORD is defined and never used anywhere in the repo

  • Where: src/core/surfaceInternal.h:36
  • Severity: low | Category: dead-code

A repo-wide grep (all .c/.h including src//, src/codegen/, and examples/) finds exactly one occurrence: this definition. The related quantity that code actually uses is baked into SURFACE_WORD_INDEX's '>> 2' shift two macros below, so this name is a stale second spelling of the same fact that nothing reads.

Fix: Delete the define, or -- if the name is worth keeping as documentation -- express SURFACE_WORD_INDEX's shift in terms of it so it has exactly one consumer.

Impact: Dead macro removal; no runtime effect

Verifier correction: Prefer the deletion option. The alternate fix ('express SURFACE_WORD_INDEX's shift in terms of it') is not directly expressible ? a shift needs log2(4)=2, not 4, so you would either divide (risking a helper call on ORCA/C 2.2.1, exactly what the header's lines 39-43 comment engineered around) or introduce a companion SURFACE_WORD_SHIFT 2, which just trades one loose constant for another. Delete the define and let the existing comment carry the documentation.

67. gRowOffsetLut declared with magic literal 200 instead of SURFACE_HEIGHT

  • Where: src/core/surfaceInternal.h:125
  • Severity: low | Category: magic-number

Everywhere else in this same header the row count is spelled SURFACE_HEIGHT (e.g. gStageMinWord[SURFACE_HEIGHT] at lines 66-67, and the compile-time band-fits-byte guard). The LUT is indexed by SURFACE_ROW_OFFSET with y in [0, SURFACE_HEIGHT); a raw 200 here means a future height change would compile cleanly against a mis-sized extern while the IIgs hal.c builder fills a differently-sized array.

Fix: extern const uint16_t gRowOffsetLut[SURFACE_HEIGHT]; (and match the definition in src/iigs/hal.c).

Impact: Declaration-only; closes one silent-divergence point for the 320x200 contract

Verifier correction: One factual error in the proposed fix: gRowOffsetLut is not defined in src/iigs/hal.c ? it is asm BSS in src/iigs/joeyDraw.s:269-272 ('.zero 400'), filled at init by the asm row-LUT builder (referenced from hal.c:108's comment). There is no C definition for the extern size to cross-check against, so the change is hygiene/documentation only; a real height change would still require manually resizing the .zero 400 in the asm. Fix is simply: extern const uint16_t gRowOffsetLut[SURFACE_HEIGHT]; plus optionally a comment in joeyDraw.s noting the 2*SURFACE_HEIGHT byte reservation.

68. Stale comment in tile.c still claims the inner row copies are spelled out in this file

  • Where: src/core/tile.c:17
  • Severity: low | Category: dead-code

The copies the comment refers to were moved to src/generic/genericTile.c (as lines 24-25 and 29-30 themselves note), so there are no inner copies "below" and the no-<string.h> rationale no longer applies to this TU -- tile.c no longer touches pixel bytes at all. The vestigial placeholder sections at lines 22-30 ("----- Prototypes -----" and "----- Internal helpers -----" containing only moved-elsewhere notes) are also dead scaffolding. Misleading segment-placement comments are dangerous in this codebase given the ORCA linker bank-packing sensitivities.

Fix: Delete lines 17-19 and collapse the empty Prototypes/Internal-helpers placeholder sections (lines 22-30) into a single one-line pointer to genericTile.c; if the DRAWPRIMS/no-memcpy constraint still matters, move that comment to genericTile.c where the loops now live.

Impact: comment-only; prevents a future editor from applying a stale segment constraint to the wrong file

69. fg/bg colors are masked to 4 bits twice on the TilePasteMono path

  • Where: src/generic/genericTile.c:84
  • Severity: low | Category: unnecessary-conditional

jlTilePasteMono in src/core/tile.c already masks both colors before dispatch (lines 135-136: fgColor &= 0x0Fu; bgColor &= 0x0Fu;), then jlpGenericTilePasteMono masks them again. Beyond the redundant ANDs, having both layers claim responsibility for masking is a small multiple-sources-of-truth problem: Amiga/ST function overrides receive already-masked values, so the contract should be that jlp receives masked colors and the generic should not re-mask.

Fix: Delete lines 84-85 from jlpGenericTilePasteMono and document the pre-masked contract in the prototype comment in port.h (matching how the IIgs macros own (_c) & 0x0F for the ops where the wrapper does not mask).

Impact: 2 redundant ops per call; clarifies which layer owns color masking

Verifier correction: Real but as a low-severity contract-cleanup, not a perf item: the 2 redundant ANDs are noise next to the 32-iteration colorize loop. The valuable part is the port.h contract comment, since masking ownership currently differs op-by-op (TilePasteMono wrapper masks; TileFill wrapper does not and the jlp layer masks).

70. jlpGenericTileFill re-derives the tile row-0 address with its own math instead of the file's GENERIC_TILE_ROW0 macro

  • Where: src/generic/genericTile.c:142
  • Severity: low | Category: duplication

Every other function in genericTile.c derives the block's row-0 pointer via GENERIC_TILE_ROW0 (defined at line 12), but jlpGenericTileFill first expands bx/by to pixel coords with two multiplies (lines 134-135: pixelX = bx * TILE_PIXELS_PER_SIDE) and then converts back down with a shift -- a second spelling of the same block-to-address mapping that must be kept in sync by hand. The intermediate pixelX/pixelY variables serve no other purpose since this function does not mark dirty (the core wrapper does).

Fix: Delete pixelX/pixelY and use row = GENERIC_TILE_ROW0(s->pixels, bx, by); like the other four functions. Optionally write the four bytes as two uint16_t stores of (doubled<<8)|doubled via the shared row helper.

Impact: removes 2 multiplies + 2 locals and one divergent copy of the address math in the DOS tile fill

Verifier correction: The fix (use GENERIC_TILE_ROW0) is right, but the perf framing is nil: bx8/by8 are strength-reduced to shifts by every compiler that runs this generic, so this is purely a single-source-of-truth cleanup. The optional 'two uint16_t stores' suggestion is unnecessary type-punning for zero measured gain on DOS ? skip it and keep the four byte stores or a memset-style fill.

71. Stale 15-line comment on IIgs jlpFrameCount documents a deleted $C019-polling/gFrameCount implementation; body is one GetTick call

  • Where: src/iigs/hal.c:358
  • Severity: low | Category: cleanup

The comment block at lines 358-371 explains a $C019 edge-detected counter and why gFrameCount needs an explicit lda+adc+sta instead of '++' (the INC-abs DBR trap) -- but the function body (line 373) is just 'return iigsGetTickWord();' and no gFrameCount variable exists anywhere in src/iigs/ (grep finds it only in this comment). The comment now asserts a timing mechanism ('No IRQ involvement') that is the opposite of reality: GetTick is the VBL-interrupt-driven tick, which is exactly the clock whose SEI artifact already inflated UBER numbers once. The registry-side comment (platform.h:108 'GetTick word, function') is the accurate one; this in-file duplicate is a wrong second source of truth.

Fix: Replace lines 358-371 with a one-liner: '// Frame counter: low word of the VBL-interrupt GetTick (see peislam.s). SEI-holding ops distort it -- see perf notes.'

Impact: Documentation-only, but prevents re-tripping the known GetTick/SEI benchmarking trap and the INC-abs cargo-culting for a variable that no longer exists.

Verifier correction: Minor nit only: the stale block is 14 lines (358-371), not 15. Everything else in the finding is accurate as stated.

72. jlDrawLine H/V fast paths clamp the span to the surface size BEFORE clipping, so a line with a far-off-surface start draws nothing

  • Where: src/core/draw.c:417
  • Severity: high | Category: bug

Found while building the Phase 0 verification harness (not part of the original 71-agent audit; confirmed by direct code trace). The horizontal fast path computes span = (int32_t)x1 - (int32_t)x0 + 1 and then clamps if (span > SURFACE_WIDTH) { span = SURFACE_WIDTH; } BEFORE any clipping against the surface. For a line whose left endpoint is far off-surface -- jlDrawLine(s, -20000, 50, 20000, 50, c) -- the clamped span (320) is anchored at x0 = -20000, so the subsequent jlFillRect(s, -20000, 50, 320, 1, c) clips to nothing and the call is a silent no-op. The correct output is the full visible row [0..319] at y=50 (the line passes through the entire surface). The vertical path (draw.c:439-441) has the identical bug for far-off-surface y0. The diagonal Bresenham fallback is unaffected (per-pixel clipping). Off-surface endpoints are documented-legal input (draw.h: out-of-bounds draws clip).

Fix: Clip x0 into range before anchoring the clamped span: after computing the 32-bit span, do if (x0 < 0) { span += x0; x0 = 0; } (span shrinks by the off-surface distance; if span <= 0 the line is entirely off-surface and returns), THEN clamp span to SURFACE_WIDTH. Mirror for the vertical path with y0/SURFACE_HEIGHT. The on-surface fast-path test below is unchanged.

Impact: Correctness: legal H/V lines with a far-off-surface start silently vanish on every port. The uber.c "edge-lines" check captures the current (wrong) behavior; its hash changes when this fix lands.

73. DOS jlpFrameCount edge-polled $3DA and lost frames whenever callers polled slower than the retrace pulse

  • Where: src/dos/hal.c:321 (old implementation)
  • Severity: critical | Category: bug -- FIXED during Phase 0

The old counter incremented only when a CALL observed the $3DA retrace bit high after a previous call observed it low. The VGA retrace pulse is ~1ms of the 14.3ms frame, so any caller doing more than ~1ms of work between jlFrameCount() polls silently lost frames -- including any real game pacing its main loop this way. Consequence for measurement: UBER's "16-frame" windows stretched 5-40x in real time for ops slower than ~1ms, inflating every slow-op number in PERF.md's DOS column (jlSurfaceClear read 449x the IIgs; the honest number is ~2.5x). The Phase 0 batched timing loop stalled on it outright, which is how it was found. Fix (landed): jlpFrameCount = millis * 70 / 1000 off the existing PIT-ISR/uclock millisecond source -- poll-rate-independent, monotonic; jlpWaitVBL still syncs to real retrace.

74. jlSpriteCompile returns false on DOS -- every DOS sprite bench and game runs the interpreter

  • Where: src/codegen/spriteCompile.c:149 (failure surfaces here; root cause not yet isolated)
  • Severity: high | Category: bug

Pre-existing at HEAD (verified against a pristine uber.c build): UBER logs "jlSpriteCompile failed" on DOS, so sp->slot stays NULL and every jlSpriteDraw/SaveUnder/RestoreUnder runs interpreted. The old PERF.md DOS sprite rows therefore measured the interpreter (and were ALSO inflated by finding #73). spriteEmitX86.c emits real code for shifts 0-1, the arena initializes (jlInit succeeds), and the 16KB scratch is a plain malloc on DJGPP -- the failure is in the sizing/emit/arena-alloc chain and needs instrumented bisection. Root-cause lands in Phase 1 (it gates Phase 4's compiled-path work on DOS).

75. jlTilePasteMono renders differently on chunky, Amiga, and ST -- three-way pixel divergence

  • Where: src/generic/genericTile.c:77 + the Amiga/ST jlpTilePasteMono overrides
  • Severity: high | Category: bug

First-ever cross-port hash of the op (Phase 0 harness, mono pattern alternating 0xF0/0x0F bytes, fg=5/bg=9 then fg=14/bg=0): DOS(generic) = 5EE7ED99, ST = 5D8774F9, Amiga = 1C87B4F9 -- after identical hashes for every preceding check on all three ports. The three implementations disagree about the mono-tile decode (likely nibble order / bit sense). The public contract (tile.h: non-zero source nibble -> fgColor) must pick one behavior; fix the divergent ports in Phase 1 and re-golden the tilePasteMono hash line. Blocks Phase 7 #3 (the IIgs asm must match the agreed behavior, not whichever variant it was written against).

76. IIgs jlpWaitVBL polled $C019, which never asserts under MAME -- infinite hang for any VBL-paced program in emulation

  • Where: src/iigs/hal.c:348 (old implementation)
  • Severity: critical | Category: bug -- FIXED during Phase 0

The old wait spun until $C019 bit 7 read high ("active scan"), then until it read low again. Probing MAME's apple2gs at two beam positions (frame-start and frame-done callbacks, 120 frames each) shows bit 7 NEVER reads high under the emulator, so the first loop never exits. Symptom chain that hid it: DRAW and PATTERN (the examples every verification gate uses) never call jlWaitVBL or jlFrameCount, so iigs-verify passed for months while UBER -- the only caller -- stalled right after its red "running" bar (PC probe: a 46-byte call-free spin; the screen frozen at exactly the 1280 bytes of 0x11 the bar paints). This also explains why PERF.md's IIgs column was "carried over" rather than re-captured: UBER on IIgs could not complete under headless MAME at HEAD. Whether real hardware asserts the bit as the code expected is unverified; the fix removes the dependence either way.

Fix (landed): jlpWaitVBL waits for the firmware tick to advance -- tick = iigsGetTickWord(); while (iigsGetTickWord() == tick) {}. The tick increments from the VBL interrupt, so tick-edge granularity IS the VBL boundary; semantics unchanged on real hardware, and it works under MAME. Callers must not hold SEI (a masked VBL interrupt freezes the tick, exactly as it froze the old status bit).

Impact: Unblocks the IIgs UBER baseline capture (bench-iigs.sh) and fixes jlWaitVBL for every future game on the reference platform under emulation.

77. IIgs sprite codegen corrupts memory under the clang build

ROOT-CAUSED 2026-07-04 (final): with #81/#85/#86 fixed, forensic dumps show the residual mechanism exactly: the codegen arena's NewHandle landed at $08:4200 -- INSIDE the app's own entry-segment bank, overlapping seg1's BSS span -- because the GS/OS loader only MM-claims each segment's IMAGE bytes, leaving BSS+heap spans as MM-free space. The emitted routines were verified BYTE-VALID (correct C-ABI prologue, correct pixel stores); writing them simply clobbers live app state. Fix is toolchain-owned (reserve full segment spans in crt0/loader -- LLVM816-ASKS.md item 5, now with this empirical proof); codegen stays OFF on IIgs until then. No JoeyLib-side code fault remains in the emit or dispatch paths.

RETEST 2026-07-04 (post-#81 allocator fix): the original corrupt-at-first-compile failure is RESOLVED -- a codegen-on UBER run now completes all 33 timed ops and most checks. Two residues: (a) the run hangs inside checkAllocator's SECOND/THIRD jlSpriteCompile (arena churn; sub-marker 112) -- the remaining #77 hunt target; (b) finding #84 applies to the COMPILED path too (jlSpriteDraw measured 1 op/368 frames with codegen on, same hash as interpreted), so the compiled path either never truly executes or shares the interpreter's pathology. Codegen default stays 0 pending (a).

  • Where: src/iigs/spriteEmitIigs.c + src/codegen/spriteCompile.c (path known; exact write not yet isolated)
  • Severity: critical | Category: bug

Bisected during Phase 0. With codegen ON (default), UBER on IIgs stalls at drawShowcase's jlSurfaceClear: MAME stack/DP forensics show jlpGenericSurfaceClear (the stage check failed -- the stage pointer or struct was already corrupted) calling memset with a garbage destination that walked into bank 0 and shredded the direct page. The corruption happens between setupSprite (jlSpriteCompile) and drawShowcase. With JOEY_IIGS_SPRITE_CODEGEN=0 the run proceeds normally. The spriteCompile.c #19 note says the self-modifying-stub corruption was "rewritten away" -- that was validated on the ORCA build; the clang build (different ABI, different segment layout, different emitted-code assumptions in spriteEmitIigs.c's C-ABI prologue) has its own corruption. Root-cause with MAME write-watchpoints on the stage struct in Phase 1; until then the IIgs baseline is captured codegen-off and the IIgs sprite rows measure the interpreter (same situation DOS is in via finding #74).

78. ST jlFillCircle and jlTileFill draw different pixels than the chunky reference

  • Where: src/atarist/ fillCircle span fill + tile fill overrides
  • Severity: high | Category: bug

First-ever DOS-vs-ST timed-op hash comparison (Phase 0 goldens): identical hashes through jlFillRect 320x200, then divergence at jlFillCircle r=40 and again at jlTileFill. Amiga (also planar) matches DOS on every timed op, so the chunky semantics are the agreed truth and the ST implementations are the outliers. Historically only IIgs-vs-68k comparisons were run, so a DOS/ST disagreement could hide. Fix in Phase 1 alongside #75 (tilePasteMono three-way divergence), then re-golden the affected ST hash lines.

79. clang-era IIgs libc memset/memcpy are byte loops -- ~4x regression vs the ORCA baseline

  • Where: toolchains/iigs/llvm-mos/runtime/src/libc.c:191 (toolchain-owned)
  • Severity: high | Category: perf

jlScbSetRange (a 200-byte memset + flag) measures 163 ops/sec on the codegen-off clang build vs 1005 ops/sec in the old PERF.md column -- impossible unless the old column was captured on the ORCA build, whose memset lowers to MVN (~7 cyc/byte). The clang runtime's memset/memcpy (libc.c) are while (n--) *d++ = c; byte loops through 24-bit pointers (~30 cyc/byte). Every memset/memcpy-based path in the core (scb range fill, surface copy, sprite save/restore fallback memcpy, stage clear fallback, log buffering) regressed ~4x on the reference platform when the toolchain switched. The library-side plan already reduces memset/memcpy dependence (findings #9, #19, #21), but the libc routines themselves are toolchain-owned: FLAG TO SCOTT for the llvm816 session (MVN-based memset/memcpy in runtime/src/libc.c). Also note: PERF.md's IIgs column provenance is the ORCA build -- the clang build's honest numbers are materially different across the board, which changes every cross-port ratio.

80. IIgs per-line log flushing does not commit to disk under the clang runtime

  • Where: src/core/debug.c:47 (design assumption broken; root cause in the runtime/FST layer)
  • Severity: medium | Category: bug

debug.c's contract is flush-after-every-line so a crash or hang leaves every line on disk. Empirically (Phase 1 mailbox vs log comparison): UBER ran all 33 timed ops -- 33 jlLogF calls, each followed by fflush -- yet only 3 op lines were on the disk image at MAME exit. Lines only persist after a clean fclose (the completed Jun-30 capture) or when enough buffered data forces a block write. Either the llvm-mos libc fflush does not issue an FST commit, or GS/OS caches the blocks until file close. Consequences: every "the log stops at line N" inference about IIgs hangs is unreliable (this misdirected the Phase 1 hunt once); crash forensics on real hardware would hit the same wall. Workarounds in place: the uber.c SHR mailbox ($E1:9DC8) is the progress channel under emulation. Fix directions: toolchain-side fflush->FST flush call (asked in LLVM816-ASKS.md), or debug.c IIgs-side fclose+reopen-append per line (correct but seconds-per-line under emulated FST -- only acceptable behind a debug flag).

81. IIgs C heap: 665 bytes, unprotected, and malloc returns 1 (not NULL) at exhaustion -- the port ran on a phantom stage

  • Where: src/core/surface.c:279 (victim); root cause in the toolchain libc/link layout
  • Severity: critical | Category: bug -- JoeyLib side FIXED during Phase 1

The full forensic chain (MAME memory probes + in-app mailbox pokes + link-map correlation + an independent static audit):

  1. The linker defines the C heap as the leftover space in the entry bank above BSS: __heap_start=$00:BC67, __heap_end=$00:BF00 -- 665 bytes, in bank 0, and never reserved from the GS/OS Memory Manager (nothing in crt0Gsos claims it).
  2. stageAlloc's calloc(1, sizeof(jlSurfaceT)) (~720 bytes) cannot fit. The libc malloc's overflow branch -- whose own comment records a "historical over-heap miscompile (only manifested for oversized n)" and assumes the path "is never exercised" -- returns 1 instead of NULL.
  3. gStage = (jlSurfaceT *)1 passes every NULL check. Its pixels field dereferences $00:0001, which happens to contain $00012000 -- the exact pinned stage framebuffer address -- so all drawing and present work flawlessly. Its scb[200] and palette[512] fields alias bank-0 low memory: every jlScbSetRange/jlPaletteSet scribbled over system space, and jlSurfaceHash hashed that garbage (the divergent IIgs hashes in the Phase 0 goldens).
  4. Cumulative bank-0 damage + further heap exhaustion during the sprite churn produced the phase-5 checkAllocator hang, the moving freeze points, and (pending confirmation) finding #77's codegen-path corruption. An independent static audit of the emitters verified the emit-time code itself is clean and separately flagged that Memory Manager handles can legally be placed over the unreserved heap (second corruption channel, toolchain-owned).

Fix (landed, JoeyLib side): finding #31 executed ahead of schedule and extended to surfaces -- ALL core structs now come from jlpBigAlloc/jlpBigFree (IIgs Memory Manager; plain malloc elsewhere): the stage struct, jlSurfaceCreate structs, jlSpriteT headers, owned tileData, and the assetLoad cel buffers, with every error-path free converted symmetrically. DOS re-benched bit-identical to the frozen goldens (43/43 hashes). The residual C-heap load is the log FILE plus arena bookkeeping nodes (well under 665 bytes).

Toolchain asks (filed in LLVM816-ASKS.md): (a) malloc/calloc must return NULL at exhaustion -- minimal repro is calloc(1, 720) against the default heap; (b) reserve [__heap_start, __heap_end) from the Memory Manager in crt0Gsos (or back malloc with an MM handle); (c) consider attrNoSpec on NewHandle calls and reserving the pinned stage range $01:2000-$9FFF.

82. IIgs per-line log writes stall the app in floppy firmware -- replaced with a RAM ring

  • Where: src/core/debug.c (IIgs branch) -- FIXED during Phase 1
  • Severity: critical | Category: bug

PC sampling of a "hung" UBER run landed in slot-5 SmartPort firmware and ROM: the app was inside GS/OS floppy writes. Each jlLogF line cost ~8 emulated seconds of FST/floppy time (the steady ~4-ops-per-2000- frames crawl), and the end-of-run logging wedged in a firmware retry loop for 18+ emulated minutes. Combined with #80 (per-line fflush never commits anyway), the flush-per-line-to-floppy design was slow, lossy, AND hang-prone under emulation. Fix (landed): on IIgs, jlLog/jlLogF append to a RAM ring exported as gJoeyLogRing/gJoeyLogRingHead; scripts/bench-iigs.sh's Lua drains the ring live (magic-word-gated mailbox handshake) and reconstructs joeylog.txt on the host; jlLogFlush writes the ring to disk once (open/write/close -- fclose is the only committing operation) for real-hardware persistence. Result: the first complete IIgs UBER capture (33 ops + 17 checks, green screen at frame ~29000).

83. IIgs diagonal-line asm diverges from the portable Bresenham

ROOT-CAUSED AND FIXED 2026-07-04 (with #86): iigsDrawLineInner (and iigsFillCircleInner) stashed the pixels pointer with a DATA-BANK store (sta dlnScratch under DBR) but dereferenced it D-relative ([pix],y reads bank 0 at D+0) -- the exact DBR-vs-D disagreement that iigsDrawPixelInner's and iigsDrawCircleInner's comments record fixing, never applied to line/fillCircle. Every plot went through whatever garbage pointer lived at the bank-0 alias of the scratch symbol: deterministic wrong pixels (this finding), and -- when a memory-layout shift changed what the garbage pointed at -- the plots could land on the dln* walk state itself, mutating the endpoint mid-line and hanging the Bresenham in an infinite wrap (#86; PC-sampled at dlnLoop/dlnStep during the Phase 1 hunt). The wrap-walk also makes the routine a wild- write generator (gRowOffsetLut indexed far out of bounds, stores through garbage row offsets), a plausible contributor to the original pre-#81 corruption soup. Both functions now use drawPixel's canonical pattern (phx/pha, tcd, D-relative stores; arg offsets unchanged). iigsBlitStageToShr has the same stale pattern in its bsFrame stash but those dereferences are the #52 dead paths -- deleted by that cleanup rather than fixed.

  • Where: src/iigs/joeyDraw.s (iigsDrawLineInner) + the JL_HAS_DRAW_LINE gate in draw.c
  • Severity: high | Category: bug

First complete IIgs capture: hashes match DOS exactly through the H/V lines, then diverge at jlDrawLine diag (DOS and Amiga agree with each other). The on-surface diagonal goes through the asm fast path on IIgs, so either iigsDrawLineInner's Bresenham differs from the C reference (step order, error seeding, endpoint handling) or the gate feeds it different values. The drawRect/circle/fillRect hashes after it also diverge until the full-screen fill wipes the state and hashes RECONVERGE (samplePixel through tileSnap match DOS again) -- so the damage is contained to the line/circle output pixels, and the circle asm needs the same scrutiny. Hunt: compare asm vs C pixel-by-pixel for the UBER diag line (0,0)-(319,199) in a MAME framebuffer dump.

84. IIgs interpreted sprite ops: ~6 seconds per 16x16 draw and wrong pixels

  • Where: src/core/sprite.c interpreted paths as compiled by the clang w65816 backend
  • Severity: high | Category: bug

With codegen off (finding #77 mitigation), the sprite ops measure: jlSpriteSaveUnder 1 iter / 65 frames, jlSpriteDraw 1 iter / 368 frames (~6.1 SECONDS for one fully-on-surface 16x16 interpreted draw at 2.8MHz -- three orders of magnitude beyond any plausible cost; finding #2's writeDstNibble overhead accounts for ~2x, not 1000x). And the output is wrong: sprite-clip-roundtrip FAILs (save/draw/restore does not restore the pre-save pixels) and every post-sprite hash diverges. Suspects: a clang-w65816 codegen pathology in the inner loop (e.g. a software-multiply or long-pointer sequence per pixel plus something quadratic), or memory-layout aliasing that makes the loop rewalk. jlSpriteRestoreUnder (memcpy rows) runs at a sane 4023 ops/sec, so the slowness is specific to the nibble-walking loops. Needs disassembly of the compiled spriteDrawInterpreted + a MAME cycle profile.

85. The pinned framebuffer was never reserved from the Memory Manager -- MM handles landed inside the display

  • Where: src/iigs/hal.c (jlpStageAllocPixels / all NewHandle sites) -- FIXED during Phase 1
  • Severity: critical | Category: bug

The empirical chain that resolved finding #84's "1000x slow sprites": a diagnostic poke showed the interpreted draw looping over w=280-320, h=100-170 with sp->widthTiles == sp->heightTiles == 0x44 (= 68 tiles, i.e. a 544x544 sprite clamped to the surface by clipRect). 0x44 is not data -- it is COLOR 4 DOUBLED, the fill byte of the jlSurfaceClear(4) that precedes UBER's sprite section. A follow-up watch of the sprite struct's address showed it living at $01:8B04 -- INSIDE the pinned stage framebuffer ($01:2000-$9CFF) -- with its bytes tracking each successive clear's fill pattern ($AA during the showcase). Mechanism: jlpStageAllocPixels handed out the bare constant $01:2000 without ever claiming the range from the Memory Manager (the #77 static audit's candidate 2), so NewHandle considered the framebuffer free and placed the #81-fix's jlpBigAlloc blocks inside it. Every surface clear then rewrote the structs: garbage sprite dimensions (the #84 slowness and wrong pixels), a full-surface save through the 256-byte gBackupBytes (the BSS carnage: smashed gBackup, the impossible op-counter values, sprite-clip-roundtrip FAIL), and plausibly the residual #77 second-compile hang (arena bookkeeping in painted-over memory).

Fix (landed): jlpStageAllocPixels claims $01:2000 (SURFACE_PIXELS_SIZE bytes) via NewHandle attrAddr|attrFixed|attrLocked and holds the handle (released in jlpStageFreePixels); failure to claim leaves a coreSetError breadcrumb. Additionally every JoeyLib NewHandle site (jlpBigAlloc, the codegen arena, all three audio handles) now passes attrNoSpec so no JoeyLib handle can land in banks $00/$01 at all. Toolchain note appended to LLVM816-ASKS.md item 5: the runtime should consider the same reservation for any of its own MM usage.

Refuted findings (for the record)

  • src/codegen/spriteCompile.c:202 -- jlSpriteCompile never compacts-and-retries on arena exhaustion, silently demoting sprites to the interpreter forever
    • Why refuted: Accuracy: FAILS. Quoted code exists (spriteCompile.c:202-206), but the claim 'jlSpriteCompile does neither' misreads the contract. codegenArenaInternal.h:42-44 offers two sanctioned options: compact-and-retry OR surface the failure ? and jlSpriteCompile surfaces it by returning false. The public API (include/joey/sprite.h:85-87) explicitly documents 'Returns false if the arena is full (caller may
  • src/core/input.c:79 -- jlWaitForAnyKey makes 59 cross-TU predicate calls per poll instead of scanning the arrays it already owns
    • Why refuted: Accuracy: PASS ? line 79 does call jlKeyPressed for all 59 keys per spin iteration, both arrays are module globals in this same file, the bounds check is loop-dead, and ~9K cyc/iteration on 65816 is a fair estimate. Fix: technically valid (identical semantics, early return preserved). Materiality: FAIL ? jlWaitForAnyKey is a blocking busy-wait whose sole purpose is to burn cycles until a human pre
  • src/core/sprite.c:155 -- Planar ports run clipRect in spriteDrawInterpreted solely for the dirty mark, then jlpSpriteDrawPlanes re-clips the same sprite
    • Why refuted: Accuracy: mostly right -- on Amiga/ST s->pixels is NULL so spriteDrawInterpreted reduces to clipRect + surfaceMarkDirtyRect, jlSpriteDraw:354-355 then calls jlpSpriteDrawPlanes which performs its own full clip (verified amiga/hal.c:1673-1685, atarist/hal.c:1749-1764), and 68k sprite compile currently fails so this is the live path. Fix-validity: workable but weaker than claimed -- moving surfaceMa
  • src/core/sprite.c:65 -- clipRect negation of dstX/dstY overflows int16_t at INT16_MIN
    • Why refuted: Accuracy: technically correct on paper -- at sprite.c:65/70, negating INT16_MIN is formal UB where int is 16-bit (ORCA/C, Watcom) and truncating impl-defined conversion on 32-bit-int m68k gcc, and the analysis that all other extents are wrap-safe checks out (dstX+w max 2359, w<=2040). Fix-validity: the early rejection guard is correct and overflow-free, though 'zero cost' is slightly off (two extr
  • src/core/codegenArena.c:113 -- codegenArenaAlloc does no size rounding: 68000 code alignment is guaranteed only by the accident that every current 68k routine happens to be word-sized
    • Why refuted: Accuracy: the mechanics check out (no rounding at codegenArena.c:113, back-to-back compaction packing at 165-168, both 68k emitters emit exclusively via 2-byte putWord, odd-address 68000 code fetch is an address error), but the core risk claim is wrong: even-ness is NOT 'the accident that every current 68k routine happens to be word-sized' -- every 68000 instruction is architecturally a multiple o
  • src/codegen/spriteCompile.c:185 -- emitTotalSize performs a full throwaway emit of every routine, doubling sprite-compile cost on 2.8MHz/8MHz targets
    • Why refuted: ACCURACY: factually correct -- emitTotalSize (spriteCompile.c:108-130) runs all 24 (shift,op) emitters to completion into scratch and the emit loop (210-240) runs them all again; the IIgs draw emitter does real per-destbyte work (shiftedByteAt at spriteEmitIigs.c:346/367, c2pColumn on 68k), so compile cost genuinely ~doubles and the ~2000-extra-calls estimate for a 32x32 sprite is the right order.