IR: end-exclusive increment+compare for step +-1 for-loops; fix DECM/BSTPOS on VM

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)

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

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
+        }
+    }
 })

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()
     }
 }