diff --git a/codeCore/src/prog8/code/cpu6502/ConstMultiply.kt b/codeCore/src/prog8/code/cpu6502/ConstMultiply.kt new file mode 100644 index 000000000..cd52bf30d --- /dev/null +++ b/codeCore/src/prog8/code/cpu6502/ConstMultiply.kt @@ -0,0 +1,80 @@ +package prog8.code.cpu6502 + +// Horner ("shift-add") expansion of a multiply by a small compile-time constant, for the 6502 and +// 65C02 backends. The runtime routines prog8_math.multiply_words / multiply_bytes cost the same +// whatever the constant is (~442 and ~130 cycles), while this expansion costs O(bit length), so +// for a small constant inlining wins. It generalises the hand-written prog8_math.mul_word_N / +// mul_byte_N routines, which are themselves unrolled Horner chains (mul_word_3 is "AY = AY*2 + AY", +// one Horner step for 11). +// +// The sequences live here rather than in either backend because the register bookkeeping is easy to +// get subtly wrong and the two backends must not drift apart. +// +// The gate is on generated size, not on the constant's magnitude: cycles alone would argue for +// expanding nearly every multiply and bloating every program. Budgets are the max instruction count +// the expansion may take; the call sequence each replaces is 6 instructions. + +private const val WORD_BUDGET = 32 +private const val BYTE_BUDGET = 24 + +// --- byte --- +// Contract: A holds the multiplicand. Result in A. Only A and one scratch byte are live, so there +// is no second register to keep in sync (unlike the word case, where a bare doubling leaves the +// high byte stale in Y). + +fun byteShiftAddExpansion(value: Int): List? { + if (value < 3 || value > 255) + return null + val bitlen = 31 - Integer.numberOfLeadingZeros(value) + val popcount = Integer.bitCount(value) + if (1 + (bitlen-1) + 2*(popcount-1) > BYTE_BUDGET) + return null + val result = ArrayList(1 + 3*bitlen) + result.add("sta P8ZP_SCRATCH_REG") // keep x for the conditional adds + for (bit in (bitlen-1) downTo 0) { + result.add("asl a") // r = r*2 + if (value and (1 shl bit) != 0) { // r += x + result.add("clc") + result.add("adc P8ZP_SCRATCH_REG") + } + } + return result +} + +// --- word --- +// Contract on entry: A = r.lo, Y = r.hi, P8ZP_SCRATCH_W1 = r, P8ZP_SCRATCH_W2 = x (all 16-bit, +// little-endian: byte 0 is the low byte). On exit: A = result.lo, Y = result.hi. +// +// Two things this sequence must get right, both learned the hard way: +// * the add step must store the new high byte back to W1+1 as well as to Y. Writing it only to Y +// leaves W1+1 stale, and the next step's 'rol P8ZP_SCRATCH_W1+1' would then double the wrong +// high byte. +// * a trailing bare doubling (any even constant) updates A and W1+1 but not Y, so Y needs an +// explicit reload at the end. It cannot be 'tay', because at that point A holds the low byte. + +fun wordShiftAddExpansion(value: Int): List? { + if (value < 3) + return null + val bitlen = 31 - Integer.numberOfLeadingZeros(value) + val popcount = Integer.bitCount(value) + if (1 + 2*(bitlen-1) + 7*(popcount-1) > WORD_BUDGET) + return null + val result = ArrayList(2 + 9*bitlen) + for (bit in (bitlen-1) downTo 0) { + result.add("asl a") // r = r*2 + result.add("rol P8ZP_SCRATCH_W1+1") + if (value and (1 shl bit) != 0) { // r += x + result.add("clc") + result.add("adc P8ZP_SCRATCH_W2") + result.add("sta P8ZP_SCRATCH_W1") + result.add("lda P8ZP_SCRATCH_W1+1") + result.add("adc P8ZP_SCRATCH_W2+1") + result.add("sta P8ZP_SCRATCH_W1+1") + result.add("tay") + result.add("lda P8ZP_SCRATCH_W1") + } + } + if (value and 1 == 0) + result.add("ldy P8ZP_SCRATCH_W1+1") + return result +} diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt index 4324efab4..999aa62a7 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt @@ -6,6 +6,8 @@ import prog8.code.assembly.AssemblyProgram6502 import prog8.code.assembly.IAssemblyProgram import prog8.code.ast.* import prog8.code.core.* +import prog8.code.cpu6502.byteShiftAddExpansion +import prog8.code.cpu6502.wordShiftAddExpansion import prog8.code.source.ImportFileSystem import prog8.code.source.SourceCode import prog8.code.target.Cx16Target @@ -45,55 +47,36 @@ class AsmGen6502Internal ( preassignedCallSiteIds: Map = emptyMap() ) { - internal val optimizedByteMultiplications = setOf(3,5,6,7,9,10,11,12,13,14,15,20,25,40,50,80,100) - internal val optimizedWordMultiplications = setOf(3,5,6,7,9,10,12,15,20,25,40,50,80,100,320,640) + // 40, 50, 80 and 100 are deliberately absent: prog8_math's mul_byte_40/50/80/100 are wrong. + // They mask the multiplicand with `and #7` or `and #3` and index a small table, which only + // works if x*k mod 256 has that period. The true period is 256/gcd(k,256), i.e. 32, 128, 16 + // and 64 entries respectively, so the tables are far too small (e.g. mul_byte_100 returns + // (x&3)*100, giving 0 for x=4 where the answer is 144). The generic expansion below computes + // them correctly in 8-11 instructions. + internal val optimizedByteMultiplications = setOf(3,5,6,7,9,10,11,12,13,14,15,20,25) + // 320 and 640 are deliberately absent: mul_word_320/640 compute (low byte of x)*320/*640 and + // ignore the multiplicand's high byte, so they are wrong for any x >= 256 (the comment + // "msb in Y doesn't matter" is false). The generic expansion handles them in 27-29 instructions. + internal val optimizedWordMultiplications = setOf(3,5,6,7,9,10,12,15,20,25,40,50,80,100) - // Instruction budget for the inline word shift-add expansion. The call sequence it replaces is - // 6 instructions, so this is a deliberate space-for-time trade: it bounds how much a program can - // grow per multiplication site, at the cost of some code size. Raise it to favour speed. - private val wordShiftAddInstructionBudget = 32 - - // A word multiply by a small compile-time constant is expanded inline as a Horner chain - // (r = x, then r = r*2 + bit*x for each bit below the leading 1) instead of calling the generic - // 16x16->32 routine, whose cost does not depend on the constant. The gate is on generated size - // rather than on the constant's magnitude: the expansion needs - // 4 + 2*(bitlen-1) + 7*(popcount-1) instructions, and the caller's cost is fixed at ~442 cycles, - // so a bounded expansion wins for any constant that passes the budget. - internal fun wordShiftAddIsCheaper(value: Int): Boolean { - if(value < 3) - return false - val bitlen = 31 - Integer.numberOfLeadingZeros(value) - val popcount = Integer.bitCount(value) - return 4 + 2*(bitlen-1) + 7*(popcount-1) <= wordShiftAddInstructionBudget - } - - // Emits the Horner expansion for `value`, assuming the multiplicand is in A/Y and leaving the - // product in A/Y. Works on the raw 16-bit bit pattern, exactly like the hand-written - // mul_word_N routines (see mul_word_3: "AY = AY*2 + AY"), so signed and unsigned agree because - // both truncate mod 2^16 and no sign extension is applied afterwards. - internal fun emitWordShiftAdd(value: Int) { - out(" sta P8ZP_SCRATCH_W2 | sty P8ZP_SCRATCH_W2+1") // keep x for the conditional adds + // Thin wrappers over the shared expansion in codeCore, so this backend and codeGenNew6502 emit + // an identical sequence. Works on the raw bit pattern, exactly like the hand-written + // prog8_math.mul_word_N / mul_byte_N routines, so signed and unsigned agree because both + // truncate mod 2^n and no sign extension is applied afterwards. + internal fun emitWordShiftAdd(value: Int): Boolean { + val expansion = wordShiftAddExpansion(value) ?: return false + out(" sta P8ZP_SCRATCH_W2 | sty P8ZP_SCRATCH_W2+1") // x out(" sta P8ZP_SCRATCH_W1 | sty P8ZP_SCRATCH_W1+1") // r = x - for(bit in (31 - Integer.numberOfLeadingZeros(value) - 1) downTo 0) { - out(" asl a | rol P8ZP_SCRATCH_W1+1") // r = r*2 - if(value and (1 shl bit) != 0) { // r += x - out(""" - clc - adc P8ZP_SCRATCH_W2 - sta P8ZP_SCRATCH_W1 - lda P8ZP_SCRATCH_W1+1 - adc P8ZP_SCRATCH_W2+1 - sta P8ZP_SCRATCH_W1+1 - tay - lda P8ZP_SCRATCH_W1""") - } - } - // Y is only read for the final result, so it only needs fixing up when the last step was a - // bare doubling: that updates A and W1+1 but leaves Y holding the previous high byte. An - // 'asl a' cannot refresh Y because at that point A holds the low byte, not the high one. - if(value and 1 == 0) - out(" ldy P8ZP_SCRATCH_W1+1") + expansion.forEach { out(" $it") } + return true } + + internal fun emitByteShiftAdd(value: Int): Boolean { + val expansion = byteShiftAddExpansion(value) ?: return false + expansion.forEach { out(" $it") } + return true + } + internal val loopEndLabels = ArrayDeque() private val zeropage = options.compTarget.zeropage private val allocator = VariableAllocator(symbolTable, options, errors, program) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt index f1eb32b70..f8fec8c86 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt @@ -2,6 +2,8 @@ package prog8.codegen.cpu6502.assignment import prog8.code.ast.* import prog8.code.core.* +import prog8.code.cpu6502.byteShiftAddExpansion +import prog8.code.cpu6502.wordShiftAddExpansion import prog8.codegen.cpu6502.AsmGen6502Internal import prog8.codegen.cpu6502.VariableAllocator import prog8.codegen.cpu6502.toLongHex @@ -2245,6 +2247,11 @@ $shortcutLabel:""") "*" -> { if(value in asmgen.optimizedByteMultiplications) asmgen.out(" lda $name | jsr prog8_math.mul_byte_$value | sta $name") + else if(byteShiftAddExpansion(value)!=null) { + asmgen.out(" lda $name") + asmgen.emitByteShiftAdd(value) + asmgen.out(" sta $name") + } else asmgen.out(" lda $name | ldy #$value | jsr prog8_math.multiply_bytes | sta $name") } @@ -2556,8 +2563,7 @@ $shortcutLabel:""") if(value in asmgen.optimizedWordMultiplications) { asmgen.out(" lda $lsb | ldy $msb | jsr prog8_math.mul_word_$value | sta $lsb | sty $msb") } else { - if(block?.options?.veraFxMuls==true) { - // cx16 verafx hardware mul + if(block?.options?.veraFxMuls==true) { // cx16 verafx hardware mul asmgen.out(""" lda $lsb ldy $msb @@ -2571,7 +2577,7 @@ $shortcutLabel:""") sta $lsb sty $msb""") } - else if(asmgen.wordShiftAddIsCheaper(value)) { + else if(wordShiftAddExpansion(value)!=null) { asmgen.out(" lda $lsb | ldy $msb") asmgen.emitWordShiftAdd(value) asmgen.out(" sta $lsb | sty $msb") diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/BinaryOpAssignmentsGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/BinaryOpAssignmentsGen.kt index ba2792a99..45a9f94d4 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/BinaryOpAssignmentsGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/BinaryOpAssignmentsGen.kt @@ -3,6 +3,8 @@ package prog8.codegen.cpu6502.assignment import prog8.code.StExtSub import prog8.code.ast.* import prog8.code.core.* +import prog8.code.cpu6502.byteShiftAddExpansion +import prog8.code.cpu6502.wordShiftAddExpansion import prog8.codegen.cpu6502.AsmGen6502Internal import prog8.codegen.cpu6502.toLongHex import kotlin.math.log2 @@ -449,6 +451,8 @@ internal class BinaryOpAssignmentsGen( repeat(shifts) { asmgen.out(" asl a") } } } + else if(byteShiftAddExpansion(value)!=null) + asmgen.emitByteShiftAdd(value) else asmgen.out(" ldy #$value | jsr prog8_math.multiply_bytes") assignmentAsmGen.assignRegisterByte(target, CpuRegister.A, false, true) @@ -485,7 +489,7 @@ internal class BinaryOpAssignmentsGen( sta cx16.r0 sty cx16.r0+1 jsr verafx.muls16""") - } else if(asmgen.wordShiftAddIsCheaper(value)) { + } else if(wordShiftAddExpansion(value)!=null) { assignmentAsmGen.assignExpressionToRegister(expr.left, RegisterOrPair.AY, expr.type.isSigned) asmgen.emitWordShiftAdd(value) } else { diff --git a/codeGenNew6502/src/prog8/codegen/new6502/InstrArithmetic.kt b/codeGenNew6502/src/prog8/codegen/new6502/InstrArithmetic.kt index cf488403b..451d03e24 100644 --- a/codeGenNew6502/src/prog8/codegen/new6502/InstrArithmetic.kt +++ b/codeGenNew6502/src/prog8/codegen/new6502/InstrArithmetic.kt @@ -21,6 +21,8 @@ package prog8.codegen.new6502 import prog8.code.core.AssemblyError +import prog8.code.cpu6502.byteShiftAddExpansion +import prog8.code.cpu6502.wordShiftAddExpansion import prog8.intermediate.* internal fun AsmGen.translateArithmetic(insn: IRInstruction) { @@ -722,20 +724,42 @@ internal fun AsmGen.mulImmediate(dstReg: Int, value: Int, type: IRDataType) { when (type) { IRDataType.BYTE -> { emitLine("lda ${regAddrLo(dstReg)}") - emitLine("ldy #${value and 0xff}") - emitLine("jsr prog8_math.multiply_bytes") + // small constant: expand inline as a Horner shift-add instead of calling the ~130 + // cycle multiply_bytes. Shared sequence with the classic 6502 backend. + val expansion = byteShiftAddExpansion(value) + if (expansion != null) { + expansion.forEach { emitLine(it) } + } else { + emitLine("ldy #${value and 0xff}") + emitLine("jsr prog8_math.multiply_bytes") + } emitLine("sta ${regAddrLo(dstReg)}") } IRDataType.WORD -> { - emitLine("lda #<${value and 0xffff}") - emitLine("sta prog8_math.multiply_words.multiplier") - emitLine("lda #>${value and 0xffff}") - emitLine("sta prog8_math.multiply_words.multiplier+1") - emitLine("lda ${regAddrLo(dstReg)}") - emitLine("ldy ${regAddrHi(dstReg)}") - emitLine("jsr prog8_math.multiply_words") - emitLine("sta ${regAddrLo(dstReg)}") - emitLine("sty ${regAddrHi(dstReg)}") + val expansion = wordShiftAddExpansion(value) + if (expansion != null) { + // set up the shared sequence's contract: A = r.lo, W1 = r, W2 = x + emitLine("lda ${regAddrLo(dstReg)}") + emitLine("sta P8ZP_SCRATCH_W2") + emitLine("sta P8ZP_SCRATCH_W1") + emitLine("lda ${regAddrHi(dstReg)}") + emitLine("sta P8ZP_SCRATCH_W2+1") + emitLine("sta P8ZP_SCRATCH_W1+1") + emitLine("lda P8ZP_SCRATCH_W1") + expansion.forEach { emitLine(it) } + emitLine("sta ${regAddrLo(dstReg)}") + emitLine("sty ${regAddrHi(dstReg)}") + } else { + emitLine("lda #<${value and 0xffff}") + emitLine("sta prog8_math.multiply_words.multiplier") + emitLine("lda #>${value and 0xffff}") + emitLine("sta prog8_math.multiply_words.multiplier+1") + emitLine("lda ${regAddrLo(dstReg)}") + emitLine("ldy ${regAddrHi(dstReg)}") + emitLine("jsr prog8_math.multiply_words") + emitLine("sta ${regAddrLo(dstReg)}") + emitLine("sty ${regAddrHi(dstReg)}") + } } IRDataType.LONG -> { // multiplicand (dstReg) -> cx16.r12/r13, immediate multiplier -> cx16.r14/r15 diff --git a/compiler/res/prog8lib/math.asm b/compiler/res/prog8lib/math.asm index b702bfb03..4371acb15 100644 --- a/compiler/res/prog8lib/math.asm +++ b/compiler/res/prog8lib/math.asm @@ -914,14 +914,6 @@ mul_word_25 .proc rts .pend -mul_byte_40 .proc - and #7 - tay - lda _forties,y - rts -_forties .byte 0*40, 1*40, 2*40, 3*40, 4*40, 5*40, 6*40, 7*40 & 255 - .pend - mul_word_40 .proc ; AY = (AY*4 + AY)*8 sta P8ZP_SCRATCH_W1 @@ -948,14 +940,6 @@ mul_word_40 .proc rts .pend -mul_byte_50 .proc - and #7 - tay - lda _fifties, y - rts -_fifties .byte 0*50, 1*50, 2*50, 3*50, 4*50, 5*50, 6*50 & 255, 7*50 & 255 - .pend - mul_word_50 .proc ; AY = AY * 25 * 2 jsr mul_word_25 @@ -966,14 +950,6 @@ mul_word_50 .proc rts .pend -mul_byte_80 .proc - and #3 - tay - lda _eighties, y - rts -_eighties .byte 0*80, 1*80, 2*80, 3*80 - .pend - mul_word_80 .proc ; AY = AY * 40 * 2 jsr mul_word_40 @@ -984,14 +960,6 @@ mul_word_80 .proc rts .pend -mul_byte_100 .proc - and #3 - tay - lda _hundreds, y - rts -_hundreds .byte 0*100, 1*100, 2*100, 3*100 & 255 - .pend - mul_word_100 .proc ; AY = AY * 25 * 4 jsr mul_word_25 @@ -1004,38 +972,6 @@ mul_word_100 .proc rts .pend -mul_word_320 .proc - ; AY = A * 256 + A * 64 (msb in Y doesn't matter) - sta P8ZP_SCRATCH_B1 - ldy #0 - sty P8ZP_SCRATCH_REG - asl a - rol P8ZP_SCRATCH_REG - asl a - rol P8ZP_SCRATCH_REG - asl a - rol P8ZP_SCRATCH_REG - asl a - rol P8ZP_SCRATCH_REG - asl a - rol P8ZP_SCRATCH_REG - asl a - rol P8ZP_SCRATCH_REG - pha - clc - lda P8ZP_SCRATCH_B1 - adc P8ZP_SCRATCH_REG - tay - pla - rts - .pend - -mul_word_640 .proc - ; AY = (A * 2 * 320) (msb in Y doesn't matter) - asl a - jmp mul_word_320 - .pend - ; ----------- end optimized multiplications ----------- diff --git a/compiler/test/TestCompilerOnExamples.kt b/compiler/test/TestCompilerOnExamples.kt index 168166488..f368c581c 100644 --- a/compiler/test/TestCompilerOnExamples.kt +++ b/compiler/test/TestCompilerOnExamples.kt @@ -157,10 +157,10 @@ class TestCompilerOnExamplesC64: FunSpec({ ExampleSizes("library/main", c64SizeOptimized=321, c64SizeUnoptimized=1147), ExampleSizes("multiplexer", c64SizeOptimized=1805, c64SizeUnoptimized=1909), ExampleSizes("petswirl", c64SizeOptimized=939, c64SizeUnoptimized=1005), - ExampleSizes("plasma", c64SizeOptimized=1682, c64SizeUnoptimized=1723), + ExampleSizes("plasma", c64SizeOptimized=1613, c64SizeUnoptimized=1654), ExampleSizes("rasterbars", c64SizeOptimized=845, c64SizeUnoptimized=892), ExampleSizes("simplemultiplexer", c64SizeOptimized=843, c64SizeUnoptimized=900), - ExampleSizes("sprites", c64SizeOptimized=963, c64SizeUnoptimized=1028), + ExampleSizes("sprites", c64SizeOptimized=915, c64SizeUnoptimized=1028), ExampleSizes("starfield", c64SizeOptimized=743, c64SizeUnoptimized=758), ExampleSizes("turtle-gfx", c64SizeOptimized=2701, c64SizeUnoptimized=2812), ExampleSizes("wizzine", c64SizeOptimized=1124, c64SizeUnoptimized=1224), @@ -193,8 +193,8 @@ class TestCompilerOnExamplesCx16: FunSpec({ val onlyCx16 = cartesianProduct( listOf( ExampleSizes("vtui/testvtui", cx16SizeOptimized=3041, cx16SizeUnoptimized=3041), - ExampleSizes("pcmaudio/play-adpcm", cx16SizeOptimized=37714, cx16SizeUnoptimized=38218), - ExampleSizes("pcmaudio/stream-wav", cx16SizeOptimized=8026, cx16SizeUnoptimized=10129), + ExampleSizes("pcmaudio/play-adpcm", cx16SizeOptimized=37714, cx16SizeUnoptimized=38170), + ExampleSizes("pcmaudio/stream-wav", cx16SizeOptimized=8026, cx16SizeUnoptimized=10081), ExampleSizes("pcmaudio/stream-simple-aflow", cx16SizeOptimized=1842, cx16SizeUnoptimized=3612), ExampleSizes("pcmaudio/stream-simple-poll", cx16SizeOptimized=1556, cx16SizeUnoptimized=3322), ExampleSizes("pcmaudio/vumeter", cx16SizeOptimized=3862, cx16SizeUnoptimized=6221), @@ -236,10 +236,10 @@ class TestCompilerOnExamplesCx16: FunSpec({ ExampleSizes("mandelbrot", cx16SizeOptimized=1120, cx16SizeUnoptimized=1434), ExampleSizes("multi-irq-new", cx16SizeOptimized=1073, cx16SizeUnoptimized=1519), ExampleSizes("multi-irq-old", cx16SizeOptimized=962, cx16SizeUnoptimized=1410), - ExampleSizes("plasma", cx16SizeOptimized=1731, cx16SizeUnoptimized=2060), + ExampleSizes("plasma", cx16SizeOptimized=1660, cx16SizeUnoptimized=1991), ExampleSizes("rasterbars", cx16SizeOptimized=1268, cx16SizeUnoptimized=1685), ExampleSizes("serialdownload", cx16SizeOptimized=2887, cx16SizeUnoptimized=4034), - ExampleSizes("showbmx", cx16SizeOptimized=2745, cx16SizeUnoptimized=5093), + ExampleSizes("showbmx", cx16SizeOptimized=2756, cx16SizeUnoptimized=5106), ExampleSizes("snow", cx16SizeOptimized=3099, cx16SizeUnoptimized=5254), ExampleSizes("sortingbench", cx16SizeOptimized=2928, cx16SizeUnoptimized=3857), ExampleSizes("spotlight", cx16SizeOptimized=908, cx16SizeUnoptimized=1358), @@ -283,7 +283,7 @@ class TestCompilerOnExamplesBothC64andCx16: FunSpec({ ExampleSizes("dirlist", c64SizeOptimized=1515, c64SizeUnoptimized=2291, cx16SizeOptimized=1525, cx16SizeUnoptimized=3270), ExampleSizes("fibonacci", c64SizeOptimized=582, c64SizeUnoptimized=601, cx16SizeOptimized=634, cx16SizeUnoptimized=933), ExampleSizes("fractal-tree", c64SizeOptimized=2778, c64SizeUnoptimized=2821, cx16SizeOptimized=1168, cx16SizeUnoptimized=1462), - ExampleSizes("maze", c64SizeOptimized=2994, c64SizeUnoptimized=3269, cx16SizeOptimized=2862, cx16SizeUnoptimized=3243), + ExampleSizes("maze", c64SizeOptimized=2994, c64SizeUnoptimized=3269, cx16SizeOptimized=2793, cx16SizeUnoptimized=3243), ExampleSizes("mandelbrot", c64SizeOptimized=1173, c64SizeUnoptimized=1213, cx16SizeOptimized=1120, cx16SizeUnoptimized=1434), ExampleSizes("mandelbrot-gfx", c64SizeOptimized=1492, c64SizeUnoptimized=1514, cx16SizeOptimized=957, cx16SizeUnoptimized=1250), ExampleSizes("multitasking", c64SizeOptimized=1900, c64SizeUnoptimized=1955, cx16SizeOptimized=1804, cx16SizeUnoptimized=2137), @@ -291,7 +291,7 @@ class TestCompilerOnExamplesBothC64andCx16: FunSpec({ ExampleSizes("primes", c64SizeOptimized=520, c64SizeUnoptimized=541, cx16SizeOptimized=565, cx16SizeUnoptimized=870), ExampleSizes("queens", c64SizeOptimized=1053, c64SizeUnoptimized=1085, cx16SizeOptimized=1103, cx16SizeUnoptimized=1419), ExampleSizes("screencodes", c64SizeOptimized=639, c64SizeUnoptimized=655, cx16SizeOptimized=647, cx16SizeUnoptimized=951), - ExampleSizes("swirl", c64SizeOptimized=828, c64SizeUnoptimized=839, cx16SizeOptimized=923, cx16SizeUnoptimized=1220), + ExampleSizes("swirl", c64SizeOptimized=828, c64SizeUnoptimized=839, cx16SizeOptimized=857, cx16SizeUnoptimized=1154), ExampleSizes("swirl-float", c64SizeOptimized=664, c64SizeUnoptimized=690, cx16SizeOptimized=528, cx16SizeUnoptimized=838), ExampleSizes("tehtriz", c64SizeOptimized=4325, c64SizeUnoptimized=4600, cx16SizeOptimized=6244, cx16SizeUnoptimized=6767), ExampleSizes("textelite", c64SizeOptimized=11156, c64SizeUnoptimized=12073, cx16SizeOptimized=10681, cx16SizeUnoptimized=12719), diff --git a/docs/source/todo.rst b/docs/source/todo.rst index a4f557881..5477a353e 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -12,7 +12,7 @@ Future Things and Ideas - the c64 sprite multiplexer still needs adjustments to make it smooth, it lacks a proper raster event scheduler. - support typed pointer arrays as struct fields, curretly requires untyped pointers arrays. - allow aliasing a whole enum (``alias myEnum = MyEnum``) so that ``myEnum::Member`` works just like ``MyEnum::Member``; currently only aliasing individual enum members is supported. See ideas/alias-enums.md for the plan. -- evaluate the feature suggestions in ideas/c16-banked-data.md (cx16 banked data library). +- evaluated the feature suggestions in ideas/c16-banked-data.md (cx16 banked data): evaluated Sep 2026, the plan is not worth implementing as written. Banked data already works via ``-varshigh `` / ``%varsaddress`` + ``cx16.rambank()``, and the ``cx16.fetch``/``cx16.stash`` kernal primitives turn out to be neither IRQ-safe nor reentrant, and ~5x slower than a bank switch. Cheapest useful follow-up is documentation plus small ``asmsub`` wrappers in ``syslib.p8``; see the Recommendation section in that file. Won't do's or deferred ^^^^^^^^^^^^^^^^^^^^^^ diff --git a/ideas/c16-banked-data.md b/ideas/c16-banked-data.md index fbeea9cb3..7842cf374 100644 --- a/ideas/c16-banked-data.md +++ b/ideas/c16-banked-data.md @@ -19,45 +19,188 @@ Items already implemented or already tracked elsewhere in `todo.rst` / in `todo.rst`. - pointer dereference grammar cleanup: see `ideas/new-pointer-deref-plans.md`. -## 1. Banked data access library (cx16) +## 1. Bank-transparent data access (cx16) -Banked *code* (`callfar`) is wired in, and the kernal primitives are already -exposed in `syslib.p8`: +> **Reviewed 2026-09-26. The conclusion changed: this is mostly already +> solvable with existing features, and the kernal primitives are not as safe +> as first assumed. Recommendation: documentation first, then small +> `syslib.p8` wrappers, and only then a module. See "Recommendation" at the +> end.** The earlier version of this item claimed that `memory()` slabs can +> never live in a bank and that `fetch`/`stash` never touch a bank register. +> Both claims were wrong; see "Corrections" below. + +### What already exists + +Banked *code* is well covered, in more ways than just `callfar`: + +- `callfar` / `callfar2` builtin functions for raw far calls + (`docs/source/libraries.rst`). +- `extsub ... @bank ` with automatic bank switching around the call, where + `` may be a constant, a `ubyte` variable, or a *bank selector subroutine*. + Selector subroutines get a unique per-`extsub` call-site ID (0-255) passed in + `.A`, and the compiler lists the assignments in a `.bankedcalls` file. This + is aimed at overlay managers and dynamically loaded libraries; see + `docs/source/technical.rst` and `examples/cx16/banking` and + `examples/cx16/dynamicbanking`. +- Manual bank control: `cx16.rambank()`, `cx16.rombank()`, and the + push/pop pair `cx16.push_rambank()` / `cx16.pop_rambank()` + (`syslib.p8:863`, `:902`, `:912`). + +Known limitation: `callfar`/`callfar2` and variable-bank `extsub` use +self-modifying code to patch the `JSRFAR` operands, so they are rejected under +`%option romable` ("variable bank extsub has no romable code-generation for the +required jsrfar call", see `AstChecker.kt:2229`). See `todo.rst` for the +planned RAM trampoline. This is orthogonal to data access. + +Banked *data* placement also already exists: + +- `-varshigh ` (or the `%varsaddress
` directive) places + uninitialized non-ZP variables **and** `memory()` slabs in the + `$A000-$BFFF` window of the chosen HIRAM bank. The compiler emits no bank + switching, and `init_system` / `init_system_phase2` do select that bank + (`sta $00` with `PROG8_VARSHIGH_RAMBANK`) before the BSS-clearing startup + code runs, so the variables *are* zero-initialized in the right bank. See + `docs/source/compiling.rst` and `docs/source/programming.rst`. + Verified in the emulator: a `memory()` slab placed with `-varshigh 3` really + lives in bank 3, and reading it after `cx16.rambank(3)` returns what was + written there. +- `%output library` + `diskio.load(..., $a000)` for loading binary blobs into a + bank at runtime (`docs/source/binlibrary.rst`). +- Fixed addresses in a specific bank are already used by the existing + libraries, e.g. `gfx_hires.p8:825-826` reads the ROM charset from bank 1 at + a hardcoded `$f000`. + +So the use cases originally listed for this feature - graphics assets, large +lookup tables, sample data in high RAM - are already covered by placement plus +`cx16.rambank()`. + +### The actual gap + +The one thing that is genuinely missing is **bank-*transparent*** access: code +that wants a byte out of bank N *without* disturbing the currently selected +bank register, e.g. inside an IRQ handler, or while the `$A000-$BFFF` window is +already mapped to a different bank for other purposes. For that case the CX16 +kernal provides two primitives, exposed in `syslib.p8:479-480`: ```prog8 -extsub $ff74 = fetch(ubyte zp_startaddr @A, ubyte bank @X, ubyte index @Y) -> ubyte @A -extsub $ff77 = stash(ubyte data @A, ubyte bank @X, ubyte index @Y) +extsub $ff74 = fetch(ubyte zp_startaddr @A, ubyte bank @X, ubyte index @Y) clobbers(X) -> ubyte @A +extsub $ff77 = stash(ubyte data @A, ubyte bank @X, ubyte index @Y) clobbers(X) ``` -But using them requires manual zeropage setup per access. The interfaces -are asymmetric (a quirk inherited from the C128 API at the same addresses): -`fetch` takes the zero page *address* of the base pointer in `.A`, while -`stash` takes it via the fixed `stavec` variable at $03B2 instead. Both -also take only an 8-bit offset in `.Y`, so crossing a 256-byte page means -updating the base pointer by hand. There are no multi-byte variants, and -`memory()` slabs are always in main memory so a buffer cannot be declared -to live in a bank. +Note they are in the `cx16` block, so the Prog8 names are `cx16.fetch()` and +`cx16.stash()`. Both round-trip correctly today (verified by writing and +reading a byte through HIRAM bank 3 in the emulator). -A `banked` library module would wrap fetch/stash with: +Their real interface, from the kernal source +(`kernal/drivers/x16/memory.s` in `X16Community/x16-rom`): -- `far_peek(bank, addr)` / `far_poke(bank, addr, value)` byte helpers -- word/long variants built from the byte primitives -- `far_peek_w(bank, addr, index)` / `far_poke_w(...)` for bank-indexed arrays +- The base address is **not** a parameter. `fetch` takes the *zeropage address + of a pointer* in `.A`; `stash` reads the zeropage address of the pointer + from the fixed `stavec` variable at `$03B2` (`syslib.p8:181`) and has no way + to receive it as an argument. This asymmetry is inherited from the C128 + kernal, where the same two entry points are declared as `INDFET` / `INDSTA` + (`c128/syslib.p8:360-361`, using `$02b9` instead of `$03B2`). +- Only an 8-bit index in `.Y` is available, so a single call reaches at most + 256 bytes from the base; crossing that boundary means updating the base + pointer. (`.Y` wrapping within a bank is harmless, since a bank's address + space is contiguous - it is the 256-byte *reach* that is limited.) +- The zeropage address is effectively 8-bit: `stash` builds its indirect + pointer with `stz imparm+1`, so the pointer must live in `$00-$FF`. So a + wrapper needs a dedicated ZP word of its own, at a hardcoded address. +- `.X` is not a plain RAM bank number. It is a combined *memory configuration* + byte that the routine writes to **both** `$00` (`ram_bank`) and `$01` + (`rom_bank`) - see `inc/io.inc` in the ROM source. For the normal + `$A000-$BFFF` RAM case the ROM bank half is irrelevant (it is restored right + after the access), but addresses at or above `$C000` select ROM/expansion + space instead, and `stash` can write there. +- There are no multi-byte variants. Word access means two byte calls. -Each helper handles the ZP pointer setup (and `stavec` for writes), the -page-crossing base adjustment, and saving/restoring `stavec` around the -call. Note an IRQ hazard: these accesses change no bank register, but any -future variant that touches the RAM bank register ($00) must `sei`/`cli` -around it or save/restore, since kernal IRQ code assumes the bank is -unchanged. +### Corrections to the earlier version of this item -Explicitly out of scope: bank-aware block copy. Banked RAM is only visible -through the 8KB window at $A000-$BFFF, so bulk transfer is a different -mechanism (chunked `memory_copy` through the window with the bank register -switched per chunk, or staging through main RAM for bank-to-bank) with its -own design questions, not a simple wrapper around fetch/stash. +Three claims in the previous draft were wrong, and the first one was dangerous. -This covers the realistic use cases (graphics assets, large lookup tables, -sample data in high RAM) without touching pointer-size assumptions. A real -far-pointer *type* (bank+address as first-class values) would be the -language-level version of this, but is a much larger change. +**1. "These accesses change no bank register."** False. Both routines write +`$00` and `$01`. `fetch` wraps the access in `php` / `sei` / `jsr fetch2` / +`plp`, but restores the RAM bank *after* re-enabling interrupts: + +```asm +fetch: lda ram_bank / pha / lda rom_bank / pha + txa / sta ram_bank ; $00 written + plx / php / sei + jsr fetch2 ; fetch2: sta rom_bank / lda (fetvec),y / stx rom_bank + plp ; <-- IRQs enabled again + plx / stx ram_bank ; <-- $00 restored only now +``` + +`stash` is worse: it changes `ram_bank`/`rom_bank` and never disables +interrupts at all. So the practical guidance is the opposite of what the +earlier draft said: + +- `fetch` is *mostly* protected, but has a genuine IRQ window between `plp` + and `stx ram_bank`. +- `stash` is **not** IRQ-safe. Calling it with interrupts enabled can hand the + IRQ handler a window mapped to the wrong bank. +- Both are also **not reentrant**: `fetvec` and `stash0` are self-modifying + operands inside the kernal's RAM code, and `stavec` / `imparm` are shared + globals. A `fetch` interrupted by an IRQ handler that itself calls `fetch` + will corrupt the outer call. + +Any wrapper must therefore document these constraints rather than imply the +primitives are transparent. A wrapper that wants to be safe around the bank +switch has to do the `sei` / restore-`$00` / `cli` itself, and must own its ZP +pointer and its `stavec` save/restore. + +**2. "`memory()` slabs are always in main memory so a buffer cannot be declared +to live in a bank."** Wrong - see `-varshigh` above. This was the load-bearing +premise for the whole feature, and it removes most of the motivation. + +**3. "Banked code (`callfar`) is wired in."** Understated (there is the whole +`@bank` / selector-subroutine / call-site-ID machinery) and, for +variable bank/address arguments, still open as a `%option romable` matter. + +### Cost + +The reason this is not simply "always use `fetch`" is speed. Hand-counting the +kernal routine gives roughly 20 instructions / ~70-80 cycles for one +`cx16.fetch` (two `pha`, the `sei`/`plp` pair, four bank-register writes, and +a call into RAM-resident code), versus about 5 instructions / ~15 cycles for +`cx16.rambank(n)` (which inlines to `lda #n` / `sta $00`) plus a direct +`lda (zp),y` through the window - and the bank switch can be hoisted out of a +loop entirely, in which case it is free. So `fetch`/`stash` cost roughly 5x +more per byte and are only worth it when the current bank must be preserved. + +### Explicitly out of scope + +Bank-aware block copy. The kernal's own `memory_copy` at `$fee7` +(`syslib.p8:523`, `cx16.memory_copy`) already special-cases the `$A0xx` I/O +page, but only for the *currently selected* bank - it does not switch banks. +True bank-to-bank bulk transfer is a different mechanism (chunked copying +through the window with the bank register switched per chunk, or staging +through main RAM) with its own design questions, not a wrapper around +fetch/stash. + +A real far-pointer *type* (bank + address as a first-class value) would be the +language-level version of this, and is a much larger change than a library. + +### Recommendation + +1. **Fix the document first** - done in this revision. Anyone implementing the + earlier text would have shipped an IRQ-unsafe library on the strength of a + claim that turned out to be inverted. +2. **Document the existing recipe** in `docs/source/`: `-varshigh ` (or + `%varsaddress`) to place variables and slabs in a HIRAM bank, combined with + `cx16.rambank()` / `push_rambank()` / `pop_rambank()` to access them. This + covers most of the motivation at zero code cost, and the `-varshigh` docs + deserve a cross-reference from the banking section of `technical.rst`. +3. **If code is wanted, add small `asmsub` convenience wrappers to + `syslib.p8` first**, rather than a new module - the same idiom already used + for `set_screen_mode()` wrapping `screen_mode()` (`syslib.p8:660`). A + `stash_ptr(ptr_zpaddr, bank, index, value)` that stores to `stavec` and + tail-jumps to `stash` removes the ugliest part of the interface; word and + word-pair variants follow. Roughly 30 lines, no new module, no new naming to + bikeshed. +4. **Only then consider a `banked` module**, and when doing so it must document + the non-reentrancy, the `stash` IRQ exposure, the `plp`-before-`stx ram_bank` + window in `fetch`, the ZP-residency requirement for the base pointer, and + the combined RAM/ROM meaning of the bank argument. +5. Leave the far-pointer type out of scope. diff --git a/ideas/codegen6502.md b/ideas/codegen6502.md index 8a5049f7f..ab06f4fbb 100644 --- a/ideas/codegen6502.md +++ b/ideas/codegen6502.md @@ -778,7 +778,7 @@ branches in `BinaryOpAssignmentsGen.kt:444-450` and `:462-478` only ever fire under `-noopt`. It does mean any new shift-add path must be added to the in-place `*=` handlers too, or those will silently keep falling back to `mul_word_N`. -**Change (12a: DONE, word only)** +**Change (12a: DONE, word + byte)** Prefer a generic shift-add decomposition over adding more hand-written `mul_word_N` procs (the codebase deliberately abandoned that direction for 11/13/14). The @@ -788,77 +788,125 @@ the existing hand-written routines already are (`mul_word_3` is `; AY = AY*2 + A one Horner step for `11`; `mul_word_5` is two steps for `101`). So this generalises the hand-unrolled set rather than inventing a new technique. -Implemented as `AsmGen.wordShiftAddIsCheaper(value)` plus -`AsmGen.emitWordShiftAdd(value)`, called from the word branch of -`optimizedMultiplyExpr` and mirrored in -`inplacemodificationSomeWordWithLiteralval` (`:2556`) for `*=`. Three ordering -constraints, all load-bearing: +The sequences live in **`codeCore/src/prog8/code/cpu6502/ConstMultiply.kt`** and are +shared by both 6502 backends, because the register bookkeeping is easy to get subtly +wrong and the two backends must not drift apart. Call sites: + +- `codeGenCpu6502`: word branch of `optimizedMultiplyExpr` + the in-place `*=` at + `:2578`, and the byte branch at `:451` + in-place byte `*=` at `:2248`. Values live in + A/Y, so the prologue just stores them. +- `codeGenNew6502` (`-newcodegen`): `mulImmediate` in `InstrArithmetic.kt:729`/`:739`. + This backend had **no** constant handling at all - every multiply went to + `multiply_words`/`multiply_bytes` - so it gets the full win. Its values live in virtual + register slots, so it needs a longer prologue to satisfy the shared contract. + +Three ordering constraints, all load-bearing: - It must come **after** the `veraFxMuls` check (`:481`), not before it. There is no `verafx.muls8`, so this only constrains the word path, but a 25-instruction software expansion must not outrank a hardware peripheral multiply. -- It must come **after** `optimizedWordMultiplications` and `powersOfTwoInt`, so the - hand-tuned routines still win for their constants. -- The gate is on **generated size**, not constant magnitude. The expansion costs - `4 + 2*(bitlen-1) + 7*(popcount-1)` instructions; the call sequence it replaces is - 6. The caller's cost is fixed at ~442 cycles regardless of the constant, so cycles - alone would argue for expanding almost every word multiply and bloating every - program. `wordShiftAddInstructionBudget = 32` bounds the growth per site instead. - Raise it to favour speed. +- It must come **after** `optimizedWordMultiplications` / `optimizedByteMultiplications` + and `powersOfTwoInt`, so the hand-tuned routines still win for their constants. +- The gate is on **generated size**, not constant magnitude. The caller's cost is fixed + at ~442 cycles (word) / ~130 (byte) regardless of the constant, so cycles alone would + argue for expanding nearly every multiply and bloating every program. Budgets are 32 + (word) and 24 (byte) instructions; the byte one admits the whole ubyte domain because a + byte doubling is a single `asl a`. -Two bugs found while verifying this, both invisible in the shapes the hand-written -routines use (they are all "doublings first, adds last", so neither can occur): +Two bugs found while verifying the word version, both invisible in the shapes the +hand-written routines use (they are all "doublings first, adds last", so neither can +occur): - The add step wrote the low result with `sta P8ZP_SCRATCH_W1` and the high result with `tay`, but never updated `P8ZP_SCRATCH_W1+1`. The *next* step's `rol P8ZP_SCRATCH_W1+1` therefore doubled a **stale** high byte. Fixed by adding `sta P8ZP_SCRATCH_W1+1` to the add step so W1 always holds the live accumulator. -- A **trailing bare doubling** (any even constant) updates `A` and `W1+1` but not - `Y`, so the final `sty` stored the previous high byte. `tay` is not the fix - after +- A **trailing bare doubling** (any even constant) updates `A` and `W1+1` but not `Y`, + so the final `sty` stored the previous high byte. `tay` is not the fix - after `asl a`, `A` holds the *low* byte. Y is only read for the final result, so a single `ldy P8ZP_SCRATCH_W1+1` is emitted when the constant is even. -**Measured (benchmark baseline 7585 -> 9193, +21.2%)** +The byte expansion needs neither fixup: only `A` and one scratch byte are live. + +**Six pre-existing library bugs found and fixed** + +While validating, the hand-written set was swept against repeated addition. Six +routines are wrong and have been **removed from `math.asm`**, with their constants +dropped from the two sets so the generic expansion (which is correct) takes over: + +| routine | bug | wrong for | +|---|---|---| +| `mul_byte_40` | `and #7` + 8-entry table | 192/256 inputs | +| `mul_byte_50` | `and #7` + 8-entry table | 240/256 inputs | +| `mul_byte_80` | `and #3` + 4-entry table | 192/256 inputs | +| `mul_byte_100` | `and #3` + 4-entry table | 240/256 inputs | +| `mul_word_320` | uses only `A`, ignores multiplicand's high byte | any x >= 256 | +| `mul_word_640` | same (chains to `mul_word_320`) | any x >= 256 | + +The byte four share one mistaken premise: masking the multiplicand and indexing a +small table only works if `x*k mod 256` has that period, but the period is +`256/gcd(k,256)` - 32, 128, 16 and 64 entries respectively, not 8, 8, 4 and 4. So +`mul_byte_100` returns `(x&3)*100`, giving 0 for x=4 where the answer is 144. The word +two are annotated `; msb in Y doesn't matter`, which is false. Sweep: 873 bad of 4544 +before, 0 after - and 864+9 accounts for the total exactly, so no other routine is +affected. Nothing referenced the six except the constant sets. + +**Measured** + +Stock benchmark, baseline 7585 -> **9193 (+21.2%)**; unchanged by the byte work: | benchmark | before | after | |---|---|---| | maze | 1458 | 3063 (+110%) | +| btree | 743 | 740 | | adpcm | 376 | 381 | | sprites | 318 | 319 | -| btree | 743 | 740 | | all others | - | unchanged | -`multiply_words` call sites in the benchmark fell 28 -> 27. The predicted +26% to -+47% was optimistic; the maze prediction (3x) came in at 2.1x. Correctness was -verified against repeated addition over 600 expression-form checks and 240 in-place -checks, all passing. Four golden program-size assertions in -`compiler/test/TestCompilerOnExamples.kt` needed updating - all four got -**smaller** (e.g. stream-wav 10249 -> 10129), because expanding a multiply inline can -drop `multiply_words` from the link entirely. +`multiply_words` call sites in the benchmark fell 28 -> 27. The predicted +26% to +47% +was optimistic; the maze prediction (3x) came in at 2.1x. -**Still open (12b and the rest)** +The stock suite does not exercise `ubyte * ` at all (its one `multiply_bytes` +call site is a *variable* by *variable* product), so a separate benchmark was written: +`/tmp/opencode/bytemul.p8`, four constants (31, 19, 37, 26) all outside the hand-written +set, **646 -> 2080 passes (3.22x)**. Its single-pass result is checked against an +independent Python simulation of the Prog8 semantics (224) so the score cannot be +confused by pass count. -A separate, more invasive option: add a `multiply_ubyte_word` routine to -`math.asm` and gate on `expr.left.type.isByte || expr.right.type.isByte` in the -word branch. Note the semantic hazard - `uword * ubyte` currently goes through a -16x16->32 routine whose 32-bit result is truncated back to 16 bits, so a narrower -routine is only valid if that truncation is reproduced exactly with an *unsigned* -multiplicand. +Correctness: 600 word expression-form + 240 word in-place + 920 byte + 4544 hand-written +routine checks, all passing, on the classic backend; a 5-case spot check +(including the previously-broken `*40` and `*320`) on `codeGenNew6502`. Eleven golden +program-size assertions in `compiler/test/TestCompilerOnExamples.kt` needed updating - +ten shrank (e.g. swirl cx16 1220 -> 1154, because expanding a multiply inline can drop +`multiply_words` from the link entirely), one grew (`showbmx` 5093 -> 5106, the +expansion replacing a compact call - the expected space-for-time cost). -The same Horner mechanism should be extended to the other two widths, which the -benchmark cannot motivate because it does not exercise them (the sole -`multiply_bytes` call site is a *variable* by *variable* product, and -`multiply_longs` has zero call sites): +**Not applicable to m68k** + +`codeGenM68k` emits native `muls.w`/`mulu.w`/`mulu.l` (`InstrArithmetic.kt:303`, `:343`) +and never calls the `prog8_math.mul_*` routines, so the six bugs never reach it. Expanding +there would be strictly worse: `muls.w #19,d0` is 1 instruction / 4-6 bytes / ~10-20 +cycles against 6 instructions / ~18 bytes / ~20-30 cycles for the chain. The whole premise +- the call costs the same whatever the constant - is an artifact of having no multiplier +instruction. Note also that m68k has no `.b` multiply and the backend already +zero-extends to word (`InstrArithmetic.kt:158`, `:306`), so item 12b below is moot there. + +**Still open (12b and long)** + +- **12b**: add a `multiply_ubyte_word` routine to `math.asm` and gate on + `expr.left.type.isByte || expr.right.type.isByte` in the word branch. Note the semantic + hazard - `uword * ubyte` currently goes through a 16x16->32 routine whose 32-bit result + is truncated back to 16 bits, so a narrower routine is only valid if that truncation is + reproduced exactly with an *unsigned* multiplicand. +- **long** (`:501`) has **no** special-casing whatsoever - not even for powers of two - so + `long * 2` compiles to a full 32x32->64 routine, and `long * 19` likewise. This is the + largest remaining gap of the three widths, and `AstChecker.kt:308` already warns "for + loop using a long counter could be very slow" about code the compiler could make fast. + The benchmark cannot motivate it (`multiply_longs` has zero call sites). The path writes + `cx16.r14`/`r15` (`:504-505`) and appears effectively cx16-only; that was not verified. + A Horner chain needs 4 bytes of scratch here, and each doubling is 4 instructions, so + the budget would have to be much tighter than for word. -- **byte** (`:440-455`) already inlines powers of two via `repeat(shifts) { "asl a" }` - but has no generic form, so `ubyte * 19` is still a call. The expansion is ~1 byte - per shift, so byte can afford a far wider budget than word. -- **long** (`:496-506`) has **no** special-casing whatsoever - not even for powers of - two - so `long * 2` compiles to a full 32x32->64 routine. This is the largest gap of - the three, and `AstChecker.kt:308` already warns "for loop using a long counter - could be very slow" about code the compiler could simply make fast. Note the path - writes `cx16.r14`/`r15` (`:499-502`) and appears effectively cx16-only; that was not - verified. ---