diff --git a/codeGeneration/src/prog8/compiler/target/C64Target.kt b/codeGeneration/src/prog8/compiler/target/C64Target.kt index 9d30c52da..bd3c68da7 100644 --- a/codeGeneration/src/prog8/compiler/target/C64Target.kt +++ b/codeGeneration/src/prog8/compiler/target/C64Target.kt @@ -1,11 +1,9 @@ package prog8.compiler.target import com.github.michaelbull.result.fold -import prog8.ast.base.ByteDatatypes -import prog8.ast.base.DataType -import prog8.ast.base.PassByReferenceDatatypes -import prog8.ast.base.WordDatatypes +import prog8.ast.base.* import prog8.ast.expressions.Expression +import prog8.ast.statements.RegisterOrStatusflag import prog8.ast.statements.Subroutine import prog8.compiler.target.c64.C64MachineDefinition import prog8.compiler.target.cbm.Petscii @@ -26,8 +24,10 @@ object C64Target: ICompilationTarget { override fun decodeString(bytes: List, altEncoding: Boolean) = if (altEncoding) Petscii.decodeScreencode(bytes, true) else Petscii.decodePetscii(bytes, true) - override fun asmsubArgsEvalOrder(sub: Subroutine): List = asmsub6502ArgsEvalOrder(sub) - override fun asmsubArgsHaveRegisterClobberRisk(args: List) = asmsub6502ArgsHaveRegisterClobberRisk(args) + override fun asmsubArgsEvalOrder(sub: Subroutine): List = + asmsub6502ArgsEvalOrder(sub) + override fun asmsubArgsHaveRegisterClobberRisk(args: List, paramRegisters: List) = + asmsub6502ArgsHaveRegisterClobberRisk(args, paramRegisters) override fun memorySize(dt: DataType): Int { return when(dt) { diff --git a/codeGeneration/src/prog8/compiler/target/Cx16Target.kt b/codeGeneration/src/prog8/compiler/target/Cx16Target.kt index 7a634d60c..94458470e 100644 --- a/codeGeneration/src/prog8/compiler/target/Cx16Target.kt +++ b/codeGeneration/src/prog8/compiler/target/Cx16Target.kt @@ -6,6 +6,7 @@ import prog8.ast.base.DataType import prog8.ast.base.PassByReferenceDatatypes import prog8.ast.base.WordDatatypes import prog8.ast.expressions.Expression +import prog8.ast.statements.RegisterOrStatusflag import prog8.ast.statements.Subroutine import prog8.compiler.target.cbm.Petscii import prog8.compiler.target.cpu6502.codegen.asmsub6502ArgsEvalOrder @@ -27,8 +28,10 @@ object Cx16Target: ICompilationTarget { override fun decodeString(bytes: List, altEncoding: Boolean) = if (altEncoding) Petscii.decodeScreencode(bytes, true) else Petscii.decodePetscii(bytes, true) - override fun asmsubArgsEvalOrder(sub: Subroutine): List = asmsub6502ArgsEvalOrder(sub) - override fun asmsubArgsHaveRegisterClobberRisk(args: List) = asmsub6502ArgsHaveRegisterClobberRisk(args) + override fun asmsubArgsEvalOrder(sub: Subroutine): List = + asmsub6502ArgsEvalOrder(sub) + override fun asmsubArgsHaveRegisterClobberRisk(args: List, paramRegisters: List) = + asmsub6502ArgsHaveRegisterClobberRisk(args, paramRegisters) override fun memorySize(dt: DataType): Int { return when(dt) { diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt index 7390e297d..8fb311c65 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt @@ -104,8 +104,10 @@ class AsmGen(private val program: Program, internal fun isTargetCpu(cpu: CpuType) = compTarget.machine.cpu == cpu internal fun haveFPWR() = compTarget is Cx16Target - internal fun asmsubArgsEvalOrder(sub: Subroutine) = compTarget.asmsubArgsEvalOrder(sub) - internal fun asmsubArgsHaveRegisterClobberRisk(args: List) = compTarget.asmsubArgsHaveRegisterClobberRisk(args) + internal fun asmsubArgsEvalOrder(sub: Subroutine) = + compTarget.asmsubArgsEvalOrder(sub) + internal fun asmsubArgsHaveRegisterClobberRisk(args: List, paramRegisters: List) = + compTarget.asmsubArgsHaveRegisterClobberRisk(args, paramRegisters) private fun header() { val ourName = this.javaClass.name diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmsubHelpers.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmsubHelpers.kt index a5a1eff8f..08b68d099 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmsubHelpers.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmsubHelpers.kt @@ -3,6 +3,7 @@ package prog8.compiler.target.cpu6502.codegen import prog8.ast.base.Cx16VirtualRegisters import prog8.ast.base.RegisterOrPair import prog8.ast.expressions.* +import prog8.ast.statements.RegisterOrStatusflag import prog8.ast.statements.Subroutine @@ -34,19 +35,27 @@ internal fun asmsub6502ArgsEvalOrder(sub: Subroutine): List { return order } -internal fun asmsub6502ArgsHaveRegisterClobberRisk(args: List): Boolean { +internal fun asmsub6502ArgsHaveRegisterClobberRisk(args: List, + paramRegisters: List): Boolean { fun isClobberRisk(expr: Expression): Boolean { - if (expr.isSimple && expr !is PrefixExpression) - return false - - if (expr is FunctionCall) { - if (expr.target.nameInSource == listOf("lsb") || expr.target.nameInSource == listOf("msb")) - return isClobberRisk(expr.args[0]) - if (expr.target.nameInSource == listOf("mkword")) - return isClobberRisk(expr.args[0]) && isClobberRisk(expr.args[1]) + when (expr) { + is ArrayIndexedExpression -> { + return paramRegisters.any { + it.registerOrPair in listOf(RegisterOrPair.Y, RegisterOrPair.AY, RegisterOrPair.XY) + } + } + is PrefixExpression -> { + return true // TODO really, is prefixexpression problematic for register clobbering? + } + is FunctionCall -> { + if (expr.target.nameInSource == listOf("lsb") || expr.target.nameInSource == listOf("msb")) + return isClobberRisk(expr.args[0]) + if (expr.target.nameInSource == listOf("mkword")) + return isClobberRisk(expr.args[0]) && isClobberRisk(expr.args[1]) + return !expr.isSimple + } + else -> return !expr.isSimple } - - return true } return args.size>1 && args.any { isClobberRisk(it) } diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt index 08833ecf1..57a478495 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt @@ -114,7 +114,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg if(sub.parameters.size==1) { argumentViaRegister(sub, IndexedValue(0, sub.parameters.single()), call.args[0]) } else { - if(asmgen.asmsubArgsHaveRegisterClobberRisk(call.args)) { + if(asmgen.asmsubArgsHaveRegisterClobberRisk(call.args, sub.asmParameterRegisters)) { registerArgsViaStackEvaluation(call, sub) } else { asmgen.asmsubArgsEvalOrder(sub).forEach { diff --git a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt index 142f8a6f6..7d519df5b 100644 --- a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt +++ b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt @@ -411,7 +411,7 @@ internal class StatementReorderer(val program: Program, scope.statements += FunctionCallStatement(IdentifierReference(listOf("sys", "rrestorex"), call.position), mutableListOf(), true, call.position) } return listOf(IAstModification.ReplaceNode(call, scope, parent)) - } else if(!options.compTarget.asmsubArgsHaveRegisterClobberRisk(call.args)) { + } else if(!options.compTarget.asmsubArgsHaveRegisterClobberRisk(call.args, function.asmParameterRegisters)) { // No register clobber risk, let the asmgen assign values to the registers directly. // this is more efficient than first evaluating them to the stack. // As complex expressions will be flagged as a clobber-risk, these will be simplified below. diff --git a/compiler/test/ZeropageTests.kt b/compiler/test/ZeropageTests.kt index 5aa06a473..b4150f8af 100644 --- a/compiler/test/ZeropageTests.kt +++ b/compiler/test/ZeropageTests.kt @@ -9,6 +9,7 @@ import io.kotest.matchers.comparables.shouldBeGreaterThan import io.kotest.matchers.shouldBe import prog8.ast.base.DataType import prog8.ast.expressions.Expression +import prog8.ast.statements.RegisterOrStatusflag import prog8.ast.statements.Subroutine import prog8.compiler.target.C64Target import prog8.compiler.target.Cx16Target @@ -37,7 +38,8 @@ class TestAbstractZeropage: FunSpec({ throw NotImplementedError("dummy") } - override fun asmsubArgsHaveRegisterClobberRisk(args: List): Boolean { + override fun asmsubArgsHaveRegisterClobberRisk(args: List, + paramRegisters: List): Boolean { throw NotImplementedError("dummy") } diff --git a/compilerInterfaces/src/prog8/compilerinterface/ICompilationTarget.kt b/compilerInterfaces/src/prog8/compilerinterface/ICompilationTarget.kt index 4d85b5d6d..b28e3c2e9 100644 --- a/compilerInterfaces/src/prog8/compilerinterface/ICompilationTarget.kt +++ b/compilerInterfaces/src/prog8/compilerinterface/ICompilationTarget.kt @@ -1,6 +1,7 @@ package prog8.compilerinterface import prog8.ast.expressions.Expression +import prog8.ast.statements.RegisterOrStatusflag import prog8.ast.statements.Subroutine interface ICompilationTarget: IStringEncoding, IMemSizer { @@ -10,5 +11,6 @@ interface ICompilationTarget: IStringEncoding, IMemSizer { override fun decodeString(bytes: List, altEncoding: Boolean): String fun asmsubArgsEvalOrder(sub: Subroutine): List - fun asmsubArgsHaveRegisterClobberRisk(args: List): Boolean + fun asmsubArgsHaveRegisterClobberRisk(args: List, + paramRegisters: List): Boolean } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 13874a95e..e183318c6 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -4,6 +4,8 @@ TODO For next compiler release (7.4) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +BUG: sys.rsave/sys.rrestore can never work as subroutine because stack is used -> builtin funcs + BUG: balls example crashes / animates wrong! caused by c83882161521378f20dc0076c01e18e8556e363e 'refactor function arguments codegen a bit' on the lines that call txt.setclr(BX[lp], BY[lp], BC[lp]) - they work with regular vars as args diff --git a/examples/test.p8 b/examples/test.p8 index bd037bda3..9f8662379 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,28 +1,26 @@ %import textio %import test_stack +%zeropage dontuse main { sub start() { test_stack.test() - ubyte[50] xpos = 49 to 0 step -1 - ubyte[50] ypos = 49 to 0 step -1 + ubyte[20] xpos = 19 to 0 step -1 + ubyte[20] ypos = 19 to 0 step -1 ubyte ball for ball in 0 to len(xpos)-1 { - txt.print_ub(xpos[ball]) - txt.spc() - txt.print_ub(ypos[ball]) - txt.nl() + ubyte xx = xpos[ball] + 1 + ubyte yy = ypos[ball] + txt.setchr(xx,yy,87) ; correct codegen + txt.setclr(xx,yy,5) ; correct codegen + txt.setchr(xpos[ball], ypos[ball], 81) ; TODO WRONG CODEGEN WITH NOOPT + txt.setclr(xpos[ball], ypos[ball], 6) ; TODO WRONG CODEGEN WITH NOOPT } - ubyte @shared x1 = 10 - ubyte @shared x2 = 20 - ubyte @shared x3 = 30 - test_stack.test() - repeat { }