vm: fix expression codegen for pointer indexing

codeGenVirtual/src/prog8/codegen/virtual/AssignmentGen.kt

@@ -159,6 +159,19 @@ internal class AssignmentGen(private val codeGen: CodeGen, private val expressio
             val variable = array.variable.targetName
             var variableAddr = codeGen.allocations.get(variable)
             val itemsize = codeGen.program.memsizer.memorySize(array.type)
+
+            if(array.variable.type==DataType.UWORD) {
+                // indexing a pointer var instead of a real array or string
+                if(itemsize!=1)
+                    throw AssemblyError("non-array var indexing requires bytes dt")
+                if(array.index.type!=DataType.UBYTE)
+                    throw AssemblyError("non-array var indexing requires bytes index")
+                val idxReg = codeGen.vmRegisters.nextFree()
+                code += expressionEval.translateExpression(array.index, idxReg, -1)
+                code += VmCodeInstruction(Opcode.LOADIX, vmDt, reg1=resultRegister, reg2=idxReg, value = variableAddr)
+                return code
+            }
+
             val fixedIndex = constIntValue(array.index)
             if(zero) {
                 if(fixedIndex!=null) {

codeGenVirtual/src/prog8/codegen/virtual/ExpressionGen.kt

@@ -154,6 +154,8 @@ internal class ExpressionGen(private val codeGen: CodeGen) {
             // indexing a pointer var instead of a real array or string
             if(eltSize!=1)
                 throw AssemblyError("non-array var indexing requires bytes dt")
+            if(arrayIx.index.type!=DataType.UBYTE)
+                throw AssemblyError("non-array var indexing requires bytes index")
             code += translateExpression(arrayIx.index, idxReg, -1)
             code += VmCodeInstruction(Opcode.LOADIX, vmDt, reg1=resultRegister, reg2=idxReg, value = arrayLocation)
             return code

compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt

@@ -45,17 +45,19 @@ internal class AstOnetimeTransforms(private val program: Program, private val op
             } else {
                 val fcall = parent as? IFunctionCall
                 if(fcall!=null) {
-                    if(fcall.target.nameInSource.size==1 && fcall.target.nameInSource[0] in InplaceModifyingBuiltinFunctions) {
+                    if(options.compTarget.name != VMTarget.NAME) {
-                        // TODO for now, swap() etc don't work on pointer var indexed args, so still replace this
+                        // TODO for now, 6502 codegen seems wrong when using pointer indexed args to any function.
+                        val memread = DirectMemoryRead(add, arrayIndexedExpression.position)
+                        return listOf(IAstModification.ReplaceNode(arrayIndexedExpression, memread, parent))
+                    } else if(fcall.target.nameInSource.size==1 && fcall.target.nameInSource[0] in InplaceModifyingBuiltinFunctions) {
+                        // TODO for now, vm codegen seems wrong when using pointer indexed args to an in-place modifying function.
                         val memread = DirectMemoryRead(add, arrayIndexedExpression.position)
                         return listOf(IAstModification.ReplaceNode(arrayIndexedExpression, memread, parent))
                     } else {
-                        println("PTR INDEX 1: $arrayIndexedExpression PARENT=${parent.javaClass}")       // TODO
                         return noModifications
                     }
                 }
                 else {
-                    println("PTR INDEX 2: $arrayIndexedExpression PARENT=${parent.javaClass}")       // TODO
                     return noModifications
                 }
             }

compiler/test/TestSubroutines.kt

@@ -3,13 +3,9 @@ package prog8tests
 import io.kotest.assertions.withClue
 import io.kotest.core.spec.style.FunSpec
 import io.kotest.matchers.shouldBe
-import io.kotest.matchers.shouldNotBe
 import io.kotest.matchers.string.shouldContain
 import io.kotest.matchers.types.instanceOf
-import prog8.ast.expressions.BinaryExpression
-import prog8.ast.expressions.DirectMemoryRead
 import prog8.ast.expressions.IdentifierReference
-import prog8.ast.expressions.NumericLiteral
 import prog8.ast.statements.*
 import prog8.code.core.DataType
 import prog8.code.target.C64Target
@@ -192,81 +188,6 @@ class TestSubroutines: FunSpec({
         errors.errors[1] shouldContain "pass-by-reference type can't be used"
     }
 
-    test("uword param and normal varindexed as array work as DirectMemoryRead") {
-        val text="""
-            main {
-              sub thing(uword rr) {
-                ubyte @shared xx = rr[1]    ; should still work as var initializer that will be rewritten
-                ubyte @shared yy
-                yy = rr[2]
-                uword @shared other
-                ubyte zz = other[3]
-              }
-            
-              sub start() {
-                ubyte[] array=[1,2,3]
-                thing(array)
-              }
-            }
-        """
-
-        val result = compileText(C64Target(), false, text, writeAssembly = true)!!
-        val module = result.program.toplevelModule
-        val block = module.statements.single() as Block
-        val thing = block.statements.filterIsInstance<Subroutine>().single {it.name=="thing"}
-        block.name shouldBe "main"
-        thing.statements.size shouldBe 10          // rr paramdecl, xx, xx assign, yy decl, yy assign, other, other assign 0, zz, zz assign, return
-        val xx = thing.statements[1] as VarDecl
-        withClue("vardecl init values must have been moved to separate assignments") {
-            xx.value shouldBe null
-        }
-        val assignXX = thing.statements[2] as Assignment
-        val assignYY = thing.statements[4] as Assignment
-        val assignZZ = thing.statements[8] as Assignment
-        assignXX.target.identifier!!.nameInSource shouldBe listOf("xx")
-        assignYY.target.identifier!!.nameInSource shouldBe listOf("yy")
-        assignZZ.target.identifier!!.nameInSource shouldBe listOf("zz")
-        val valueXXexpr = (assignXX.value as DirectMemoryRead).addressExpression as BinaryExpression
-        val valueYYexpr = (assignYY.value as DirectMemoryRead).addressExpression as BinaryExpression
-        val valueZZexpr = (assignZZ.value as DirectMemoryRead).addressExpression as BinaryExpression
-        (valueXXexpr.left as IdentifierReference).nameInSource shouldBe listOf("rr")
-        (valueYYexpr.left as IdentifierReference).nameInSource shouldBe listOf("rr")
-        (valueZZexpr.left as IdentifierReference).nameInSource shouldBe listOf("other")
-        (valueXXexpr.right as NumericLiteral).number.toInt() shouldBe 1
-        (valueYYexpr.right as NumericLiteral).number.toInt() shouldBe 2
-        (valueZZexpr.right as NumericLiteral).number.toInt() shouldBe 3
-    }
-
-    test("uword param and normal varindexed as array work as MemoryWrite") {
-        val text="""
-            main {
-              sub thing(uword rr) {
-                rr[10] = 42
-              }
-            
-              sub start() {
-                ubyte[] array=[1,2,3]
-                thing(array)
-              }
-            }
-        """
-
-        val result = compileText(C64Target(), false, text, writeAssembly = true)!!
-        val module = result.program.toplevelModule
-        val block = module.statements.single() as Block
-        val thing = block.statements.filterIsInstance<Subroutine>().single {it.name=="thing"}
-        block.name shouldBe "main"
-        thing.statements.size shouldBe 3   // "rr, rr assign, return void"
-        val assignRR = thing.statements[1] as Assignment
-        (assignRR.value as NumericLiteral).number.toInt() shouldBe 42
-        val memwrite = assignRR.target.memoryAddress
-        memwrite shouldNotBe null
-        val addressExpr = memwrite!!.addressExpression as BinaryExpression
-        (addressExpr.left as IdentifierReference).nameInSource shouldBe listOf("rr")
-        addressExpr.operator shouldBe "+"
-        (addressExpr.right as NumericLiteral).number.toInt() shouldBe 10
-    }
-
     test("invalid number of args check on normal subroutine") {
         val text="""
             main {

docs/source/todo.rst

@@ -3,18 +3,7 @@ TODO
 
 For next release
 ^^^^^^^^^^^^^^^^
-- fix pointervar indexing 6502 codegen (AssignmentAsmGen translateNormalAssignment) -> gridcommander worm doesn't move
+- BUG in 6502 codegen:   ubyte one = 1  |  value = one+one+one+one+one    is not 5
-- is this code still so much larger:
-                uword xx
-                for xx in 0 to size-1 {
-                    gfx2.next_pixel(bitmapbuf[xx])
-                }
-  than this loop:
-                uword srcptr = bitmapbuf
-                repeat size {
-                    gfx2.next_pixel(@(srcptr))
-                    srcptr++
-                }
 
 - optimize pointervar indexing codegen: writing (all sorts of things)
 - pipe operator: (targets other than 'Virtual'): allow non-unary function calls in the pipe that specify the other argument(s) in the calls.  Already working for VM target.

examples/test.p8

@@ -39,31 +39,34 @@ main {
         txt.print_ub(value)    ;; 33
         txt.nl()
 
-;        ; 11 22 33
+        ; 11 22 33
-;        txt.print_ub(bitmapbuf[0])
+        txt.print_ub(bitmapbuf[0])
-;        txt.spc()
+        txt.spc()
-;        txt.print_ub(bitmapbuf[1])
+        txt.print_ub(bitmapbuf[1])
-;        txt.spc()
+        txt.spc()
-;        txt.print_ub(bitmapbuf[2])
+        txt.print_ub(bitmapbuf[2])
-;        txt.nl()
+        txt.nl()
-;        rol(bitmapbuf[0])
+        rol(bitmapbuf[0])
-;        rol(bitmapbuf[0])
+        rol(bitmapbuf[0])
-;        txt.print_ub(bitmapbuf[0])  ; 44
+        txt.print_ub(bitmapbuf[0])  ; 44
-;        txt.spc()
+        txt.spc()
-;        ror(bitmapbuf[0])
+        ror(bitmapbuf[0])
-;        ror(bitmapbuf[0])
+        ror(bitmapbuf[0])
-;        txt.print_ub(bitmapbuf[0])  ; 11
+        txt.print_ub(bitmapbuf[0])  ; 11
-;        txt.nl()
+        txt.nl()
-;
+
-;        ; 22 44 66
+        ; 22 44 66
-;        txt.print_ub(bitmapbuf[0]*2)
+        txt.print_ub(bitmapbuf[0]*2)
-;        txt.spc()
+        txt.spc()
-;        txt.print_ub(bitmapbuf[1]*2)
+        txt.print_ub(bitmapbuf[1]*2)
-;        txt.spc()
+        txt.spc()
-;        txt.print_ub(bitmapbuf[2]*2)
+        txt.print_ub(bitmapbuf[2]*2)
-;        txt.nl()
+        txt.nl()
 
         ubyte one = 1
+        value = one+one+one+one+one
+        txt.print_ub(value)     ; 5
+        txt.nl()
 
         bitmapbuf[0] = one
         bitmapbuf[1] = one+one
@@ -73,7 +76,7 @@ main {
         bitmapbuf[2] -= 2
         swap(bitmapbuf[0], bitmapbuf[1])
 
-        ; 1 2 3
+        ; 2 1 3
         txt.print_ub(bitmapbuf[0])
         txt.spc()
         txt.print_ub(bitmapbuf[1])
@@ -87,24 +90,6 @@ main {
         }
         txt.nl()
 
-        ; TODO why is this loop much larger in code than the one below.
-        value = 0
-        ubyte xx
-        for xx in 0 to size-1 {
-            value += bitmapbuf[xx]
-        }
-        txt.print_ub(value)     ; 45
-        txt.nl()
-
-        ; TODO this one is much more compact
-        value = 0
-        uword srcptr = bitmapbuf
-        repeat size {
-            value += @(srcptr)
-            srcptr++
-        }
-        txt.print_ub(value)     ; 45
-        txt.nl()
 
 ;        ; a "pixelshader":
 ;        sys.gfx_enable(0)       ; enable lo res screen