From af4de6d2fc873934d7849ed9c4040a84b528f801 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 23 Mar 2021 23:44:14 +0100 Subject: [PATCH] replacing complex array indexer expressions moved to BeforeAsmGeneration + use cx16 virtualregister instead of adding temp variables for this --- .../compiler/BeforeAsmGenerationAstChanger.kt | 69 +++++++++++++------ .../codegen/assignment/AssignmentAsmGen.kt | 12 +++- .../src/prog8/ast/statements/AstStatements.kt | 3 - examples/test.p8 | 10 +++ 4 files changed, 68 insertions(+), 26 deletions(-) diff --git a/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt b/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt index 853b8cf7a..3696b832d 100644 --- a/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt +++ b/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 { + + 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 { + + class Searcher : IAstVisitor { + val complexArrayIndexedExpressions = mutableListOf() + 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 { val modifications = mutableListOf() - 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 } diff --git a/compiler/src/prog8/compiler/target/cpu6502/codegen/assignment/AssignmentAsmGen.kt b/compiler/src/prog8/compiler/target/cpu6502/codegen/assignment/AssignmentAsmGen.kt index f905e4d34..efdeeb496 100644 --- a/compiler/src/prog8/compiler/target/cpu6502/codegen/assignment/AssignmentAsmGen.kt +++ b/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) - require(target.datatype in ByteDatatypes) + // 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 -> { diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index c4d532747..2336bf608 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/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() 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, diff --git a/examples/test.p8 b/examples/test.p8 index dbc862141..872943e95 100644 --- a/examples/test.p8 +++ b/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 {