From 22a2f8cbc5d11119db221647b2628fb910c66ebd Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sat, 12 Sep 2026 00:07:02 +0200 Subject: [PATCH] IR: end-exclusive increment+compare for step +-1 for-loops; fix DECM/BSTPOS on VM --- .../prog8/codegen/intermediate/IRCodeGen.kt | 44 +- .../intermediate/IRPeepholeOptimizer.kt | 107 +++-- compiler/test/vm/TestVariableStepForLoops.kt | 383 ++++++++++++++++++ examples/test.p8 | 31 +- 4 files changed, 508 insertions(+), 57 deletions(-) diff --git a/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt b/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt index 1eb9911d3..fa50283eb 100644 --- a/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt +++ b/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt @@ -871,9 +871,44 @@ class IRCodeGen( } addInstr(result, precheckInstruction, null) - addInstr(result, IRInstruction(Opcode.STOREM, loopvarDtIr, reg1=fromTr.resultReg, labelSymbol=loopvarSymbol), null) + // For step +-1 on scalar integer types, transform to an increment-then-compare + // against an end-exclusive value computed once in a typed register. This mirrors + // the 6502 backend optimization and eliminates the trailing JUMP and the extra + // LOADM per iteration. Pointer loop variables keep the original shape. + val useEndExclusive = (step==1 || step==-1) && loopvarDtIr != IRDataType.POINTER + if(useEndExclusive) { + // Emit the end-bump and the initial store each in their own labeled chunk. + // This prevents chunk joining from merging them, which would break M4's + // single-instruction store pattern. + val endBumpLabel = createLabelName() + result += IRCodeChunk(endBumpLabel, null).also { + it += if(step==1) + IRInstruction(Opcode.INC, loopvarDtIr, reg1=toTr.resultReg) + else + IRInstruction(Opcode.DEC, loopvarDtIr, reg1=toTr.resultReg) + } + val storeLabel = createLabelName() + result += IRCodeChunk(storeLabel, null).also { + it += IRInstruction(Opcode.STOREM, loopvarDtIr, reg1=fromTr.resultReg, labelSymbol=loopvarSymbol) + } + } else { + addInstr(result, IRInstruction(Opcode.STOREM, loopvarDtIr, reg1=fromTr.resultReg, labelSymbol=loopvarSymbol), null) + } result += labelFirstChunk(translateNode(forLoop.statements), loopLabel) - if(step==1 || step==-1) { + if(useEndExclusive) { + // Keep the increment and compare in separate labeled/unlabeled chunks so M4 can + // recognize the new tail shape after chunk joining. + val incLabel = createLabelName() + result += IRCodeChunk(incLabel, null).also { + it.instructions += addConstMem(loopvarDtIr, null, loopvarSymbol, step).instructions + } + val cmpLabel = createLabelName() + result += IRCodeChunk(cmpLabel, null).also { + it += IRInstruction(Opcode.LOADM, loopvarDtIr, reg1 = fromTr.resultReg, labelSymbol = loopvarSymbol) + it += IRInstruction(Opcode.CMP, loopvarDtIr, reg1=toTr.resultReg, reg2=fromTr.resultReg) + it += IRInstruction(Opcode.BSTNE, labelSymbol = loopLabel) + } + } else if(step==1 || step==-1) { // if endvalue == loopvar, stop loop, else iterate result += IRCodeChunk(null, null).also { it += IRInstruction(Opcode.LOADM, loopvarDtIr, reg1 = fromTr.resultReg, labelSymbol = loopvarSymbol) @@ -1050,7 +1085,10 @@ class IRCodeGen( val chunk2 = addConstMem(loopvarDtIr, null, loopvarSymbol, iterable.step) if(loopvarDtIr==IRDataType.BYTE && iterable.step==-1 && iterable.last==0) { // downto 0 optimization (byte) - if(loopvarDt.isSignedByte || iterable.first<=127) { + // Only rely on DECM setting status bits on targets where it actually does (6502 hardware, + // or targets that honor the strict multi-byte status-bits contract). On the VM DECM + // leaves flags untouched, so BSTPOS would read stale state and loop forever. + if((loopvarDt.isSignedByte || iterable.first<=127) && (options.compTarget.cpu.is6502 || options.compTarget.cpu.statusBitsOnMultiByteOps)) { chunk2 += IRInstruction(Opcode.BSTPOS, labelSymbol = loopLabel) } else { chunk2 += IRInstruction(Opcode.LOADM, loopvarDtIr, reg1 = indexReg, labelSymbol = loopvarSymbol) diff --git a/codeGenIntermediate/src/prog8/codegen/intermediate/IRPeepholeOptimizer.kt b/codeGenIntermediate/src/prog8/codegen/intermediate/IRPeepholeOptimizer.kt index 011bec42a..62b93fd4c 100644 --- a/codeGenIntermediate/src/prog8/codegen/intermediate/IRPeepholeOptimizer.kt +++ b/codeGenIntermediate/src/prog8/codegen/intermediate/IRPeepholeOptimizer.kt @@ -1458,9 +1458,10 @@ jump p8_label_gen_2 private fun optimizeLoopCounters(sub: IRSubroutine): Boolean { // M4: keep loop counter in register instead of round-tripping through memory. - // Looks for the non-constant range for-loop pattern emitted by IRCodeGen.translateForInNonConstantRange - // with step 1: STORM loopvar, loopLabel: body..., LOADM loopvar + CMP + BSTEQ, INCM loopvar + JUMP loopLabel. - // If the body does not write to loopvar, replace the memory round-trip with register operations. + // Recognizes two shapes emitted by IRCodeGen.translateForInNonConstantRange for step +-1: + // Old: STOREM loopvar; loopLabel: body...; [LOADM loopvar + CMP + BSTEQ after]; INCM loopvar; JUMP loopLabel. + // New: STOREM loopvar; loopLabel: body...; INCM/DECM loopvar; LOADM loopvar + CMP + BSTNE loopLabel. + // If the body does not write to loopvar and loopvar is not live-out, replace the memory round-trip with register operations. var changed = false var idx = 0 while (idx < sub.chunks.size) { @@ -1477,23 +1478,46 @@ jump p8_label_gen_2 if (bodyStart.label == null) { idx++; continue } val loopLabel = bodyStart.label!! - // find inc chunk: contains INCM loopvar and JUMP loopLabel (possibly same chunk or inc then jump) - var incIdx = -1 + // Find the tail: either the old shape (INCM loopvar + JUMP loopLabel) or the new + // increment-then-compare shape (INCM/DECM loopvar; LOADM loopvar + CMP + BSTNE loopLabel). + data class Tail(val incIdx: Int, val cmpIdx: Int?, val jumpSeparate: Boolean) + var tail: Tail? = null for (k in idx + 1 until sub.chunks.size) { val c = sub.chunks[k] if (c !is IRCodeChunk) continue - val hasInc = c.instructions.any { it.opcode == Opcode.INCM && it.labelSymbol == loopvar } - if (!hasInc) continue + val hasIncDec = c.instructions.any { it.opcode in setOf(Opcode.INCM, Opcode.DECM) && it.labelSymbol == loopvar } + if (!hasIncDec) continue + + // old shape: INCM then (same chunk or next chunk) JUMP loopLabel val hasJumpInSame = c.instructions.any { it.opcode == Opcode.JUMP && it.labelSymbol == loopLabel } - if (hasJumpInSame) { incIdx = k; break } + if (hasJumpInSame) { tail = Tail(k, null, false); break } if (k + 1 < sub.chunks.size) { val nxt = sub.chunks[k + 1] if (nxt is IRCodeChunk && nxt.instructions.size == 1 && nxt.instructions[0].opcode == Opcode.JUMP && nxt.instructions[0].labelSymbol == loopLabel) { - incIdx = k; break + tail = Tail(k, null, true); break + } + } + + // new shape: next chunk is LOADM loopvar + CMP + BSTNE loopLabel + if (k + 1 < sub.chunks.size) { + val nxt = sub.chunks[k + 1] + if (nxt is IRCodeChunk && nxt.instructions.size == 3) { + val i1 = nxt.instructions[0] + val i2 = nxt.instructions[1] + val i3 = nxt.instructions[2] + if (i1.opcode == Opcode.LOADM && i1.labelSymbol == loopvar && i1.reg1 != null && + i2.opcode == Opcode.CMP && (i2.reg1 == i1.reg1 || i2.reg2 == i1.reg1) && + i3.opcode == Opcode.BSTNE && i3.labelSymbol == loopLabel) { + tail = Tail(k, k + 1, false) + break + } } } } - if (incIdx == -1) { idx++; continue } + if (tail == null) { idx++; continue } + var incIdx = tail.incIdx + val cmpIdx = tail.cmpIdx + val jumpSeparate = tail.jumpSeparate // body range is idx+1 until incIdx exclusive var bodyWrites = false @@ -1503,7 +1527,7 @@ jump p8_label_gen_2 for (ins in ch.instructions) { // any store/inc/dec that writes to loopvar memory if (ins.labelSymbol == loopvar && ins.opcode in setOf(Opcode.STOREM, Opcode.STOREIM, Opcode.STOREZM, Opcode.STOREX, Opcode.STOREZX, Opcode.INCM, Opcode.DECM, Opcode.ADDIM, Opcode.SUBIM)) { - // exclude the initial STORM itself (not in body) and the inc chunk (not in body) + // exclude the initial STOREM itself (not in body) and the inc chunk (not in body) bodyWrites = true; break } // also check generic memory write via address? Loopvar is always via labelSymbol, so covered @@ -1513,42 +1537,27 @@ jump p8_label_gen_2 if (bodyWrites) { idx++; continue } // Check live-out: if loopvar is read after the loop, don't optimize (needs spill handling) - var labelAfter: String? = null - for (b in idx + 1 until incIdx) { - val ch = sub.chunks[b] as? IRCodeChunk ?: continue - for (ins in ch.instructions) { - if (ins.opcode == Opcode.BSTEQ && ins.labelSymbol != null) { - // This is likely the loop's exit branch (LOADM+CMP+BSTEQ pattern) - // Verify it follows a CMP that uses loopvar (heuristic: preceding LOADM loopvar) - labelAfter = ins.labelSymbol - break - } - } - if (labelAfter != null) break - } - var exitIdx = -1 - if (labelAfter != null) { - for (k in incIdx + 1 until sub.chunks.size) { - if (sub.chunks[k].label == labelAfter) { exitIdx = k; break } - } + val tailEnd = when { + cmpIdx != null -> cmpIdx + jumpSeparate -> incIdx + 1 + else -> incIdx } var liveOut = false - if (exitIdx != -1) { - for (k in exitIdx until sub.chunks.size) { - val ch = sub.chunks[k] as? IRCodeChunk ?: continue - for (ins in ch.instructions) { - if (ins.opcode == Opcode.LOADM && ins.labelSymbol == loopvar) { liveOut = true; break } - } - if (liveOut) break + for (k in tailEnd + 1 until sub.chunks.size) { + val ch = sub.chunks[k] as? IRCodeChunk ?: continue + for (ins in ch.instructions) { + if (ins.opcode == Opcode.LOADM && ins.labelSymbol == loopvar) { liveOut = true; break } } + if (liveOut) break } if (liveOut) { idx++; continue } // Found a candidate loop; perform rewrite - // 1. Remove STORM chunk + // 1. Remove STOREM chunk sub.chunks.removeAt(idx) // incIdx shifts by -1 after removal incIdx-- + val newCmpIdx = cmpIdx?.minus(1) // 2. In body chunks, replace LOADM loopvar with LOADR loopReg (keep type) for (b in idx until incIdx) { val ch = sub.chunks[b] as? IRCodeChunk ?: continue @@ -1566,15 +1575,33 @@ jump p8_label_gen_2 } } } - // 3. In inc chunk, replace INCM loopvar with INC loopReg + // 3. In the inc chunk, replace INCM/DECM loopvar with INC/DEC loopReg val incChunk = sub.chunks[incIdx] as IRCodeChunk for (i in incChunk.instructions.indices) { val ins = incChunk.instructions[i] - if (ins.opcode == Opcode.INCM && ins.labelSymbol == loopvar) { - incChunk.instructions[i] = IRInstruction(Opcode.INC, loopType, reg1 = loopReg) + if (ins.labelSymbol == loopvar && ins.opcode in setOf(Opcode.INCM, Opcode.DECM)) { + val newOpcode = if (ins.opcode == Opcode.INCM) Opcode.INC else Opcode.DEC + incChunk.instructions[i] = IRInstruction(newOpcode, loopType, reg1 = loopReg) break } } + // 4. New shape: also rewrite the compare chunk's LOADM loopvar + if (newCmpIdx != null) { + val cmpChunk = sub.chunks[newCmpIdx] as IRCodeChunk + for (i in cmpChunk.instructions.indices) { + val ins = cmpChunk.instructions[i] + if (ins.opcode == Opcode.LOADM && ins.labelSymbol == loopvar) { + val dest = ins.reg1!! + if (dest == loopReg) { + cmpChunk.instructions[i] = IRInstruction(Opcode.NOP) + } else { + val tp = ins.type ?: loopType + cmpChunk.instructions[i] = IRInstruction(Opcode.LOADR, tp, reg1 = dest, reg2 = loopReg) + } + break + } + } + } changed = true // Do not increment idx, re-evaluate at same position (now next loop) continue diff --git a/compiler/test/vm/TestVariableStepForLoops.kt b/compiler/test/vm/TestVariableStepForLoops.kt index 9bd5e7ef2..ac54bfc6b 100644 --- a/compiler/test/vm/TestVariableStepForLoops.kt +++ b/compiler/test/vm/TestVariableStepForLoops.kt @@ -448,4 +448,387 @@ class TestVariableStepForLoops: FunSpec({ memory.getUW(allocations["main.result"]!!) shouldBe 321u } } + + test("constant unsigned byte downto 0") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + ubyte @shared count + ubyte @shared result + sub start() { + ubyte i + for i in 5 downto 0 { + count++ + result += i + } + } + } + """) { memory, allocations -> + memory.getUB(allocations["main.count"]!!) shouldBe 6u + memory.getUB(allocations["main.result"]!!) shouldBe 15u + } + } + + test("constant unsigned byte 0 downto 0") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + ubyte @shared count + ubyte @shared result + sub start() { + ubyte i + for i in 0 downto 0 { + count++ + result += i + } + } + } + """) { memory, allocations -> + memory.getUB(allocations["main.count"]!!) shouldBe 1u + memory.getUB(allocations["main.result"]!!) shouldBe 0u + } + } + + test("constant signed byte downto 0") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + byte @shared count + byte @shared result + sub start() { + byte i + for i in 2 downto 0 { + count++ + result += i + } + } + } + """) { memory, allocations -> + memory.getSB(allocations["main.count"]!!) shouldBe 3 + memory.getSB(allocations["main.result"]!!) shouldBe 3 + } + } + + test("non-constant uword ascending boundary 65535") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + uword @shared count + uword @shared first + uword @shared last + sub start() { + uword @shared a = 65534 + uword @shared b = 65535 + uword i + for i in a to b { + if count == 0 + first = i + last = i + count++ + } + } + } + """) { memory, allocations -> + memory.getUW(allocations["main.count"]!!) shouldBe 2u + memory.getUW(allocations["main.first"]!!) shouldBe 65534u + memory.getUW(allocations["main.last"]!!) shouldBe 65535u + } + } + + test("non-constant uword descending boundary 0") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + uword @shared count + uword @shared first + uword @shared last + sub start() { + uword @shared a = 1 + uword @shared b = 0 + uword i + for i in a downto b { + if count == 0 + first = i + last = i + count++ + } + } + } + """) { memory, allocations -> + memory.getUW(allocations["main.count"]!!) shouldBe 2u + memory.getUW(allocations["main.first"]!!) shouldBe 1u + memory.getUW(allocations["main.last"]!!) shouldBe 0u + } + } + + test("non-constant word ascending signed") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + word @shared count + word @shared sum + sub start() { + word @shared a = -2 + word @shared b = 2 + word i + for i in a to b { + count++ + sum += i + } + } + } + """) { memory, allocations -> + memory.getSW(allocations["main.count"]!!) shouldBe 5 + memory.getSW(allocations["main.sum"]!!) shouldBe 0 + } + } + + test("non-constant word descending signed boundary") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + word @shared count + word @shared first + word @shared last + sub start() { + word @shared a = -32767 + word @shared b = -32768 + word i + for i in a downto b { + if count == 0 + first = i + last = i + count++ + } + } + } + """) { memory, allocations -> + memory.getSW(allocations["main.count"]!!) shouldBe 2 + memory.getSW(allocations["main.first"]!!) shouldBe -32767 + memory.getSW(allocations["main.last"]!!) shouldBe -32768 + } + } + + test("non-constant ubyte ascending boundary") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + ubyte @shared count + ubyte @shared first + ubyte @shared last + sub start() { + ubyte @shared a = 254 + ubyte @shared b = 255 + ubyte i + for i in a to b { + if count == 0 + first = i + last = i + count++ + } + } + } + """) { memory, allocations -> + memory.getUB(allocations["main.count"]!!) shouldBe 2u + memory.getUB(allocations["main.first"]!!) shouldBe 254u + memory.getUB(allocations["main.last"]!!) shouldBe 255u + } + } + + test("non-constant ubyte descending boundary") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + ubyte @shared count + ubyte @shared first + ubyte @shared last + sub start() { + ubyte @shared a = 1 + ubyte @shared b = 0 + ubyte i + for i in a downto b { + if count == 0 + first = i + last = i + count++ + } + } + } + """) { memory, allocations -> + memory.getUB(allocations["main.count"]!!) shouldBe 2u + memory.getUB(allocations["main.first"]!!) shouldBe 1u + memory.getUB(allocations["main.last"]!!) shouldBe 0u + } + } + + test("non-constant byte descending signed") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + byte @shared count + byte @shared sum + sub start() { + byte @shared a = 1 + byte @shared b = -1 + byte i + for i in a downto b { + count++ + sum += i + } + } + } + """) { memory, allocations -> + memory.getSB(allocations["main.count"]!!) shouldBe 3 + memory.getSB(allocations["main.sum"]!!) shouldBe 0 + } + } + + test("non-constant from equals to") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + ubyte @shared count + ubyte @shared value + sub start() { + ubyte @shared a = 7 + ubyte @shared b = 7 + ubyte i + for i in a to b { + count++ + value = i + } + } + } + """) { memory, allocations -> + memory.getUB(allocations["main.count"]!!) shouldBe 1u + memory.getUB(allocations["main.value"]!!) shouldBe 7u + } + } + + test("non-constant empty range ascending") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + ubyte @shared count + sub start() { + ubyte @shared a = 10 + ubyte @shared b = 0 + ubyte i + for i in a to b { + count++ + } + } + } + """) { memory, allocations -> + memory.getUB(allocations["main.count"]!!) shouldBe 0u + } + } + + test("non-constant empty range descending") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + ubyte @shared count + sub start() { + ubyte @shared a = 0 + ubyte @shared b = 10 + ubyte i + for i in a downto b { + count++ + } + } + } + """) { memory, allocations -> + memory.getUB(allocations["main.count"]!!) shouldBe 0u + } + } + + test("non-constant long ascending boundary") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + long @shared count + long @shared first + long @shared last + sub start() { + long @shared a = 2147483646 + long @shared b = 2147483647 + long i + for i in a to b { + if count == 0 + first = i + last = i + count++ + } + } + } + """) { memory, allocations -> + memory.getSL(allocations["main.count"]!!) shouldBe 2 + memory.getSL(allocations["main.first"]!!) shouldBe 2147483646 + memory.getSL(allocations["main.last"]!!) shouldBe 2147483647 + } + } + + test("non-constant long descending boundary") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + long @shared count + long @shared first + long @shared last + sub start() { + long @shared a = -2147483647 + long @shared b = -2147483648 + long i + for i in a downto b { + if count == 0 + first = i + last = i + count++ + } + } + } + """) { memory, allocations -> + memory.getSL(allocations["main.count"]!!) shouldBe 2 + memory.getSL(allocations["main.first"]!!) shouldBe -2147483647 + memory.getSL(allocations["main.last"]!!) shouldBe -2147483648 + } + } + + test("non-constant pointer step 1 keeps working") { + runVm(""" + %zeropage basicsafe + %option no_sysinit + main { + pointer @shared result + sub start() { + pointer @shared a = 0 + pointer @shared b = 4 + pointer i + pointer sum = 0 + for i in a to b { + sum += i + } + result = sum + } + } + """) { memory, allocations -> + memory.getUW(allocations["main.result"]!!) shouldBe 10u + } + } }) diff --git a/examples/test.p8 b/examples/test.p8 index 4a73c639b..10664775b 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,20 +1,23 @@ +%import textio +%zeropage basicsafe + main { sub start() { - cx16.r1 = memory.foo() - cx16.r2 = mkword.foo() - } -} + long total + cx16.r1 = 2000 + cx16.r2 = 1000 -memory { - sub foo() -> uword { - cx16.r0++ - return cx16.r0 - } -} + for cx16.r0 in 1000 to cx16.r1 { + total += cx16.r0 + } + txt.print_l(total) + txt.nl() -mkword { - sub foo() -> uword { - cx16.r0++ - return cx16.r0 + + for cx16.r0 in 2000 downto cx16.r2 { + total += cx16.r0 + } + txt.print_l(total) + txt.nl() } }