replacing complex array indexer expressions moved to BeforeAsmGeneration + use cx16 virtualregister instead of adding temp variables for this

compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt

@@ -8,6 +8,7 @@ import prog8.ast.expressions.*
 import prog8.ast.statements.*
 import prog8.ast.walk.AstWalker
 import prog8.ast.walk.IAstModification
+import prog8.ast.walk.IAstVisitor
 import prog8.compiler.target.ICompilationTarget
 
 
@@ -230,6 +231,14 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, val errors: I
     }
 
     override fun after(arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable<IAstModification> {
+
+        val containingStatement = getContainingStatement(arrayIndexedExpression)
+        if(getComplexArrayIndexedExpressions(containingStatement).size > 1) {
+            errors.err("it's not possible to use more than one complex array indexing expression in a single statement; break it up via a temporary variable for instance", containingStatement.position)
+            return noModifications
+        }
+
+
         val index = arrayIndexedExpression.indexer.indexExpr
         if(index !is NumericLiteralValue && index !is IdentifierReference) {
             // replace complex indexing expression with a temp variable to hold the computed index first
@@ -239,33 +248,51 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, val errors: I
         return noModifications
     }
 
+    private fun getComplexArrayIndexedExpressions(stmt: Statement): List<ArrayIndexedExpression> {
+
+        class Searcher : IAstVisitor {
+            val complexArrayIndexedExpressions = mutableListOf<ArrayIndexedExpression>()
+            override fun visit(arrayIndexedExpression: ArrayIndexedExpression) {
+                val ix = arrayIndexedExpression.indexer.indexExpr
+                if(ix !is NumericLiteralValue && ix !is IdentifierReference)
+                    complexArrayIndexedExpressions.add(arrayIndexedExpression)
+            }
+
+            override fun visit(branchStatement: BranchStatement) {}
+
+            override fun visit(forLoop: ForLoop) {}
+
+            override fun visit(ifStatement: IfStatement) {
+                ifStatement.condition.accept(this)
+            }
+
+            override fun visit(untilLoop: UntilLoop) {
+                untilLoop.condition.accept(this)
+            }
+        }
+
+        val searcher = Searcher()
+        stmt.accept(searcher)
+        return searcher.complexArrayIndexedExpressions
+    }
+
+    private fun getContainingStatement(expression: Expression): Statement {
+        var node: Node = expression
+        while(node !is Statement)
+            node = node.parent
+
+        return node
+    }
+
     private fun getAutoIndexerVarFor(expr: ArrayIndexedExpression): MutableList<IAstModification> {
         val modifications = mutableListOf<IAstModification>()
-        val subroutine = expr.definingSubroutine()!!
         val statement = expr.containingStatement()
-        val indexerVarPrefix = "prog8_autovar_index_"
-        val repo = subroutine.asmGenInfo.usedAutoArrayIndexerForStatements
-
-        // TODO [codegen] make this a bit smarter so it can reuse indexer variables. BUT BEWARE of scoping+initialization problems then
-        //  for instance when dealing with multiple complex array indices in a single statement...
-        //   ... use cx16's virtual registers for this instead of adding new variables?
-
-        // add another loop index var to be used for this expression
-        val indexerVarName = "$indexerVarPrefix${expr.indexer.hashCode()}"
-        val indexerVar = AsmGenInfo.ArrayIndexerInfo(indexerVarName, expr.indexer)
-        repo.add(indexerVar)
-        // create the indexer var at block level scope
-        val vardecl = VarDecl(VarDeclType.VAR, DataType.UBYTE, ZeropageWish.PREFER_ZEROPAGE,
-            null, indexerVarName, null, null, isArray = false, autogeneratedDontRemove = true, position = expr.position)
-        modifications.add(IAstModification.InsertFirst(vardecl, subroutine))
-
-        // replace the indexer with just the variable
+        // replace the indexer with just the variable (simply use a cx16 virtual register r9, that we HOPE is not used for other things in the expression...)
         // assign the indexing expression to the helper variable, but only if that hasn't been done already
-        val target = AssignTarget(IdentifierReference(listOf(indexerVar.name), expr.indexer.position), null, null, expr.indexer.position)
+        val target = AssignTarget(IdentifierReference(listOf("cx16", "r9"), expr.indexer.position), null, null, expr.indexer.position)
         val assign = Assignment(target, expr.indexer.indexExpr, expr.indexer.position)
         modifications.add(IAstModification.InsertBefore(statement, assign, statement.definingScope()))
-        modifications.add(IAstModification.ReplaceNode(expr.indexer.indexExpr, target.identifier!!.copy(), expr.indexer))       // TODO is this ok now?
-
+        modifications.add(IAstModification.ReplaceNode(expr.indexer.indexExpr, target.identifier!!.copy(), expr.indexer))
         return modifications
     }
 

compiler/src/prog8/compiler/target/cpu6502/codegen/assignment/AssignmentAsmGen.kt

@@ -1341,8 +1341,16 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen
     }
 
     internal fun assignRegisterByte(target: AsmAssignTarget, register: CpuRegister) {
-        if(target.register !in Cx16VirtualRegisters)
+        // we make an exception in the type check for assigning something to a cx16 virtual register
+        if(target.register !in Cx16VirtualRegisters) {
+            if(target.kind==TargetStorageKind.VARIABLE) {
+                val parts = target.asmVarname.split('.')
+                if (parts.size != 2 || parts[0] != "cx16")
                     require(target.datatype in ByteDatatypes)
+            } else {
+                require(target.datatype in ByteDatatypes)
+            }
+        }
 
         when(target.kind) {
             TargetStorageKind.VARIABLE -> {

compilerAst/src/prog8/ast/statements/AstStatements.kt

@@ -606,14 +606,11 @@ class AsmGenInfo {
     // Conceptually it should be part of any INameScope.
     // But because the resulting code only creates "real" scopes on a subroutine level,
     // it's more consistent to only define these attributes on a Subroutine node.
-    var usedAutoArrayIndexerForStatements = mutableListOf<ArrayIndexerInfo>()
     var usedRegsaveA = false
     var usedRegsaveX = false
     var usedRegsaveY = false
     var usedFloatEvalResultVar1 = false
     var usedFloatEvalResultVar2 = false
-
-    class ArrayIndexerInfo(val name: String, val replaces: ArrayIndex)
 }
 
 // the subroutine class covers both the normal user-defined subroutines,

examples/test.p8

@@ -7,6 +7,16 @@ main {
     sub start() {
         txt.print("hello")
 
+        ubyte[] array = [1,2,3,4]
+        ubyte ix
+
+        ubyte a = array[1] + array[ix]
+        a = array[ix] + array[ix]
+        a = array[ix+1] + array[ix]
+        uword multiple=0
+        a = array[lsb(multiple)] + array[ix]
+
+
 ;        str filename = "titlescreen.bin"
 ;        ubyte success = cx16.vload(filename, 8, 0, $0000)
 ;        if success {