refactor fuction arguments codegen a bit

codeGeneration/src/prog8/compiler/target/C64Target.kt

@@ -5,11 +5,14 @@ import prog8.ast.base.ByteDatatypes
 import prog8.ast.base.DataType
 import prog8.ast.base.PassByReferenceDatatypes
 import prog8.ast.base.WordDatatypes
+import prog8.ast.expressions.Expression
+import prog8.ast.statements.Subroutine
 import prog8.compiler.target.c64.C64MachineDefinition
 import prog8.compiler.target.cbm.Petscii
+import prog8.compiler.target.cpu6502.codegen.asmsub6502ArgsEvalOrder
+import prog8.compiler.target.cpu6502.codegen.asmsub6502ArgsHaveRegisterClobberRisk
 import prog8.compilerinterface.ICompilationTarget
 
-
 object C64Target: ICompilationTarget {
     override val name = "c64"
     override val machine = C64MachineDefinition
@@ -23,6 +26,9 @@ object C64Target: ICompilationTarget {
     override fun decodeString(bytes: List<UByte>, altEncoding: Boolean) =
         if (altEncoding) Petscii.decodeScreencode(bytes, true) else Petscii.decodePetscii(bytes, true)
 
+    override fun asmsubArgsEvalOrder(sub: Subroutine): List<Int> = asmsub6502ArgsEvalOrder(sub)
+    override fun asmsubArgsHaveRegisterClobberRisk(args: List<Expression>): Boolean  = asmsub6502ArgsHaveRegisterClobberRisk(args)
+
     override fun memorySize(dt: DataType): Int {
         return when(dt) {
             in ByteDatatypes -> 1

codeGeneration/src/prog8/compiler/target/Cx16Target.kt

@@ -5,7 +5,11 @@ import prog8.ast.base.ByteDatatypes
 import prog8.ast.base.DataType
 import prog8.ast.base.PassByReferenceDatatypes
 import prog8.ast.base.WordDatatypes
+import prog8.ast.expressions.Expression
+import prog8.ast.statements.Subroutine
 import prog8.compiler.target.cbm.Petscii
+import prog8.compiler.target.cpu6502.codegen.asmsub6502ArgsEvalOrder
+import prog8.compiler.target.cpu6502.codegen.asmsub6502ArgsHaveRegisterClobberRisk
 import prog8.compiler.target.cx16.CX16MachineDefinition
 import prog8.compilerinterface.ICompilationTarget
 
@@ -23,6 +27,9 @@ object Cx16Target: ICompilationTarget {
     override fun decodeString(bytes: List<UByte>, altEncoding: Boolean) =
         if (altEncoding) Petscii.decodeScreencode(bytes, true) else Petscii.decodePetscii(bytes, true)
 
+    override fun asmsubArgsEvalOrder(sub: Subroutine): List<Int> = asmsub6502ArgsEvalOrder(sub)
+    override fun asmsubArgsHaveRegisterClobberRisk(args: List<Expression>): Boolean  = asmsub6502ArgsHaveRegisterClobberRisk(args)
+
     override fun memorySize(dt: DataType): Int {
         return when(dt) {
             in ByteDatatypes -> 1

codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt

@@ -104,6 +104,9 @@ 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<Expression>) = compTarget.asmsubArgsHaveRegisterClobberRisk(args)
+
     private fun header() {
         val ourName = this.javaClass.name
         val cpu = when(compTarget.machine.cpu) {

codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmsubHelpers.kt

@@ -0,0 +1,37 @@
+package prog8.compiler.target.cpu6502.codegen
+
+import prog8.ast.base.Cx16VirtualRegisters
+import prog8.ast.expressions.*
+import prog8.ast.statements.Subroutine
+
+
+internal fun asmsub6502ArgsEvalOrder(sub: Subroutine): List<Int> {
+    val order = mutableListOf<Int>()
+    // order is: 1) cx16 virtual word registers, 2) actual CPU registers, 3) CPU Carry status flag
+    val args = sub.parameters.zip(sub.asmParameterRegisters).withIndex()
+    val (cx16regs, args2) = args.partition { it.value.second.registerOrPair in Cx16VirtualRegisters }
+    val (regs, rest) = args2.partition { it.value.second.registerOrPair != null }
+    cx16regs.forEach { order += it.index }
+    regs.forEach { order += it.index }
+    rest.forEach { order += it.index }
+    require(order.size==sub.parameters.size)
+    return order
+}
+
+internal fun asmsub6502ArgsHaveRegisterClobberRisk(args: List<Expression>): 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])
+        }
+
+        return true
+    }
+
+    return args.size>1 && args.any { isClobberRisk(it) }
+}

codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt

@@ -111,45 +111,16 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
     }
 
     private fun argumentsViaRegisters(sub: Subroutine, call: IFunctionCall) {
-        fun isNoClobberRisk(expr: Expression): Boolean {
-            if(expr is AddressOf ||
-                expr is NumericLiteralValue ||
-                expr is StringLiteralValue ||
-                expr is ArrayLiteralValue ||
-                expr is IdentifierReference)
-                return true
-
-            if(expr is FunctionCall) {
-                if(expr.target.nameInSource==listOf("lsb") || expr.target.nameInSource==listOf("msb"))
-                    return isNoClobberRisk(expr.args[0])
-                if(expr.target.nameInSource==listOf("mkword"))
-                    return isNoClobberRisk(expr.args[0]) && isNoClobberRisk(expr.args[1])
-            }
-
-            return false
-        }
-
         if(sub.parameters.size==1) {
-            // just a single parameter, no risk of clobbering registers
             argumentViaRegister(sub, IndexedValue(0, sub.parameters.single()), call.args[0])
         } else {
-            when {
+            if(asmgen.asmsubArgsHaveRegisterClobberRisk(call.args)) {
-                call.args.all {isNoClobberRisk(it)} -> {
+                registerArgsViaStackEvaluation(call, sub)
-                    // There's no risk of clobbering for these simple argument types. Optimize the register loading directly from these values.
+            } else {
-                    // register assignment order: 1) cx16 virtual word registers, 2) actual CPU registers, 3) CPU Carry status flag.
+                asmgen.asmsubArgsEvalOrder(sub).forEach {
-                    val argsInfo = sub.parameters.withIndex().zip(call.args).zip(sub.asmParameterRegisters)
+                    val param = sub.parameters[it]
-                    val (cx16virtualRegs, args2) = argsInfo.partition { it.second.registerOrPair in Cx16VirtualRegisters }
+                    val arg = call.args[it]
-                    val (cpuRegs, statusRegs) = args2.partition { it.second.registerOrPair!=null }
+                    argumentViaRegister(sub, IndexedValue(it, param), arg)
-                    for(arg in cx16virtualRegs)
-                        argumentViaRegister(sub, arg.first.first, arg.first.second)
-                    for(arg in cpuRegs)
-                        argumentViaRegister(sub, arg.first.first, arg.first.second)
-                    for(arg in statusRegs)
-                        argumentViaRegister(sub, arg.first.first, arg.first.second)
-                }
-                else -> {
-                    // Risk of clobbering due to complex expression args. Evaluate first, then assign registers.
-                    registerArgsViaStackEvaluation(call, sub)
                 }
             }
         }

compiler/src/prog8/compiler/Compiler.kt

@@ -265,7 +265,7 @@ private fun processAst(program: Program, errors: IErrorReporter, compilerOptions
     program.charLiteralsToUByteLiterals(compilerOptions.compTarget)
     program.constantFold(errors, compilerOptions.compTarget)
     errors.report()
-    program.reorderStatements(errors)
+    program.reorderStatements(errors, compilerOptions.compTarget)
     errors.report()
     program.addTypecasts(errors)
     errors.report()

compiler/src/prog8/compiler/astprocessing/AstExtensions.kt

@@ -11,6 +11,7 @@ import prog8.ast.walk.AstWalker
 import prog8.ast.walk.IAstModification
 import prog8.compiler.BeforeAsmGenerationAstChanger
 import prog8.compilerinterface.CompilationOptions
+import prog8.compilerinterface.ICompilationTarget
 import prog8.compilerinterface.IErrorReporter
 import prog8.compilerinterface.IStringEncoding
 
@@ -28,8 +29,8 @@ internal fun Program.processAstBeforeAsmGeneration(compilerOptions: CompilationO
     }
 }
 
-internal fun Program.reorderStatements(errors: IErrorReporter) {
+internal fun Program.reorderStatements(errors: IErrorReporter, target: ICompilationTarget) {
-    val reorder = StatementReorderer(this, errors)
+    val reorder = StatementReorderer(this, errors, target)
     reorder.visit(this)
     if(errors.noErrors()) {
         reorder.applyModifications()

compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt

@@ -7,10 +7,11 @@ import prog8.ast.statements.*
 import prog8.ast.walk.AstWalker
 import prog8.ast.walk.IAstModification
 import prog8.compilerinterface.BuiltinFunctions
+import prog8.compilerinterface.ICompilationTarget
 import prog8.compilerinterface.IErrorReporter
 
 
-internal class StatementReorderer(val program: Program, val errors: IErrorReporter) : AstWalker() {
+internal class StatementReorderer(val program: Program, val errors: IErrorReporter, private val compTarget: ICompilationTarget) : AstWalker() {
     // Reorders the statements in a way the compiler needs.
     // - 'main' block must be the very first statement UNLESS it has an address set.
     // - library blocks are put last.
@@ -396,9 +397,30 @@ internal class StatementReorderer(val program: Program, val errors: IErrorReport
         if(function.parameters.isEmpty()) {
             // 0 params -> just GoSub
             return listOf(IAstModification.ReplaceNode(call, GoSub(null, call.target, null, call.position), parent))
+        } else if(!compTarget.asmsubArgsHaveRegisterClobberRisk(call.args)) {
+            // no register clobber risk, let the asmgen assign values to registers directly.
+            return noModifications
         } else {
-            // TODO new logic for passing arguments to asmsub:  >0 params -> not sure yet how to do this....
+            // TODO new logic for passing arguments to asmsub with clobber risk...
+            val argEvalOrder = compTarget.asmsubArgsEvalOrder(function)
+            println("ARGS ORDER OF $call:  ${argEvalOrder.toList()}")
+
+//            function.asmsubArgsEvalOrder().forEach {
+//                val arg = call.args[it]
+//                val param = function.parameters[it]
+//                val paramReg = function.asmParameterRegisters[it]
+//                when(param.type) {
+//                    DataType.UBYTE -> TODO()
+//                    DataType.BYTE -> TODO()
+//                    DataType.UWORD -> TODO()
+//                    DataType.WORD -> TODO()
+//                    else -> throw FatalAstException("invalidt dt for asmsub param")
+//                }
+//            }
+//
             return noModifications
         }
     }
+
+
 }

compiler/test/ZeropageTests.kt

@@ -6,9 +6,10 @@ import io.kotest.core.spec.style.FunSpec
 import io.kotest.matchers.collections.shouldBeIn
 import io.kotest.matchers.collections.shouldNotBeIn
 import io.kotest.matchers.comparables.shouldBeGreaterThan
-import io.kotest.matchers.ints.shouldBeGreaterThan
 import io.kotest.matchers.shouldBe
 import prog8.ast.base.DataType
+import prog8.ast.expressions.Expression
+import prog8.ast.statements.Subroutine
 import prog8.compiler.target.C64Target
 import prog8.compiler.target.Cx16Target
 import prog8.compiler.target.c64.C64MachineDefinition.C64Zeropage
@@ -32,6 +33,14 @@ class TestAbstractZeropage: FunSpec({
             throw NotImplementedError("dummy")
         }
 
+        override fun asmsubArgsEvalOrder(sub: Subroutine): List<Int> {
+            throw NotImplementedError("dummy")
+        }
+
+        override fun asmsubArgsHaveRegisterClobberRisk(args: List<Expression>): Boolean {
+            throw NotImplementedError("dummy")
+        }
+
         override fun memorySize(dt: DataType): Int {
             throw NotImplementedError("dummy")
         }

compilerInterfaces/src/prog8/compilerinterface/ICompilationTarget.kt

@@ -1,8 +1,14 @@
 package prog8.compilerinterface
 
+import prog8.ast.expressions.Expression
+import prog8.ast.statements.Subroutine
+
 interface ICompilationTarget: IStringEncoding, IMemSizer {
     val name: String
     val machine: IMachineDefinition
     override fun encodeString(str: String, altEncoding: Boolean): List<UByte>
     override fun decodeString(bytes: List<UByte>, altEncoding: Boolean): String
+
+    fun asmsubArgsEvalOrder(sub: Subroutine): List<Int>
+    fun asmsubArgsHaveRegisterClobberRisk(args: List<Expression>): Boolean
 }

examples/test.p8

@@ -5,20 +5,22 @@ main {
     sub start() {
         test_stack.test()
 
-        sys.push(-22 as ubyte)
+        uword @shared uw
-        sys.push(44)
-        sys.pushw(-11234 as uword)
-        sys.pushw(12345)
-        sys.push(1)
-        sys.push(2)
-        ubyte @shared ub = sys.pop()
-        byte @shared bb = sys.pop() as byte
-        uword @shared uw = sys.popw()
-        word @shared ww = sys.popw() as word
-        void sys.pop()
-        void sys.pop()
 
-        ; routine2(uw+1, true)
+;        sys.push(-22 as ubyte)
+;        sys.push(44)
+;        sys.pushw(-11234 as uword)
+;        sys.pushw(12345)
+;        sys.push(1)
+;        sys.push(2)
+;        ubyte @shared ub = sys.pop()
+;        byte @shared bb = sys.pop() as byte
+;        uw = sys.popw()
+;        word @shared ww = sys.popw() as word
+;        void sys.pop()
+;        void sys.pop()
+
+        routine2(uw, 11,22, true, 33)
 
         test_stack.test()
 
@@ -27,7 +29,7 @@ main {
 
     }
 
-    asmsub routine2(uword num @AY, ubyte switch @X) {
+    asmsub routine2(uword num @AY, ubyte a1 @R1, ubyte a2 @R2, ubyte switch @Pc, ubyte a3 @X) {
         %asm {{
             adc #20
             rts