small refactor and spelling fixes

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

@@ -74,88 +74,87 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
         //       (you can use subroutine.shouldSaveX() and saveX()/restoreX() routines as a help for this)
 
         val sub = call.target.targetSubroutine(program) ?: throw AssemblyError("undefined subroutine ${call.target}")
+        val subAsmName = asmgen.asmSymbolName(call.target)
 
-        if(!isExpression && !sub.isAsmSubroutine) {
+        if(!isExpression && !sub.isAsmSubroutine)
-            throw AssemblyError("functioncall statments to non-asmsub should have been replaced by GoSub $call")
+            throw AssemblyError("functioncall statements to non-asmsub should have been replaced by GoSub $call")
-        }
 
-        if(call.args.isNotEmpty()) {
+        if(sub.isAsmSubroutine) {
-
+            argumentsViaRegisters(sub, call)
-            if(sub.asmParameterRegisters.isEmpty()) {
+            if (sub.inline && asmgen.options.optimize) {
-                // via variables
+                // inline the subroutine.
-                for(arg in sub.parameters.withIndex().zip(call.args)) {
+                // we do this by copying the subroutine's statements at the call site.
-                    argumentViaVariable(sub, arg.first, arg.second)
+                // NOTE: *if* there is a return statement, it will be the only one, and the very last statement of the subroutine
-                }
+                // (this condition has been enforced by an ast check earlier)
+                asmgen.out("  \t; inlined routine follows: ${sub.name}")
+                val assembly = sub.statements.single() as InlineAssembly
+                asmgen.translate(assembly)
+                asmgen.out("  \t; inlined routine end: ${sub.name}")
             } else {
-                require(sub.isAsmSubroutine)
+                asmgen.out("  jsr  $subAsmName")
-                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 {
-
-                    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
-                    }
-
-                    when {
-                        call.args.all {isNoClobberRisk(it)} -> {
-                            // There's no risk of clobbering for these simple argument types. Optimize the register loading directly from these values.
-                            // register assignment order: 1) cx16 virtual word registers, 2) actual CPU registers, 3) CPU Carry status flag.
-                            val argsInfo = sub.parameters.withIndex().zip(call.args).zip(sub.asmParameterRegisters)
-                            val (cx16virtualRegs, args2) = argsInfo.partition { it.second.registerOrPair in Cx16VirtualRegisters }
-                            val (cpuRegs, statusRegs) = args2.partition { it.second.registerOrPair!=null }
-                            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)
-                        }
-                    }
-                }
             }
         }
-
+        else {
-        val subName = asmgen.asmSymbolName(call.target)
+            if(sub.inline)
-        if(!sub.inline || !asmgen.options.optimize) {
-            asmgen.out("  jsr  $subName")
-        } else {
-            // inline the subroutine.
-            // we do this by copying the subroutine's statements at the call site.
-            // NOTE: *if* there is a return statement, it will be the only one, and the very last statement of the subroutine
-            // (this condition has been enforced by an ast check earlier)
-
-            // note: for now, this is only reliably supported for asmsubs.
-            if(!sub.isAsmSubroutine)
                 throw AssemblyError("can only reliably inline asmsub routines at this time")
 
-            asmgen.out("  \t; inlined routine follows: ${sub.name}")
+            argumentsViaVariables(sub, call)
-            val assembly = sub.statements.single() as InlineAssembly
+            asmgen.out("  jsr  $subAsmName")
-            asmgen.translate(assembly)
-            asmgen.out("  \t; inlined routine end: ${sub.name}")
         }
 
         // remember: dealing with the X register and/or dealing with return values is the responsibility of the caller
     }
 
+    private fun argumentsViaVariables(sub: Subroutine, call: IFunctionCall) {
+        for(arg in sub.parameters.withIndex().zip(call.args))
+            argumentViaVariable(sub, arg.first, arg.second)
+    }
+
+    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 {
+                call.args.all {isNoClobberRisk(it)} -> {
+                    // There's no risk of clobbering for these simple argument types. Optimize the register loading directly from these values.
+                    // register assignment order: 1) cx16 virtual word registers, 2) actual CPU registers, 3) CPU Carry status flag.
+                    val argsInfo = sub.parameters.withIndex().zip(call.args).zip(sub.asmParameterRegisters)
+                    val (cx16virtualRegs, args2) = argsInfo.partition { it.second.registerOrPair in Cx16VirtualRegisters }
+                    val (cpuRegs, statusRegs) = args2.partition { it.second.registerOrPair!=null }
+                    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)
+                }
+            }
+        }
+    }
+
     private fun registerArgsViaStackEvaluation(stmt: IFunctionCall, sub: Subroutine) {
         // this is called when one or more of the arguments are 'complex' and
         // cannot be assigned to a register easily or risk clobbering other registers.

codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AsmAssignment.kt

@@ -65,7 +65,7 @@ internal class AsmAssignTarget(val kind: TargetStorageKind,
                 identifier != null -> {
                     val parameter = identifier!!.targetVarDecl(program)?.subroutineParameter
                     if(parameter!=null && parameter.definingSubroutine!!.isAsmSubroutine) {
-                        TODO("ASSIGN ASMPARAM $parameter ::  $assign")
+                        TODO("ASSIGNTARGET ASMPARAM $parameter ::  $assign")
                     }
                     AsmAssignTarget(TargetStorageKind.VARIABLE, program, asmgen, dt, assign.definingSubroutine, variableAsmName = asmgen.asmVariableName(identifier!!), origAstTarget =  this)
                 }
@@ -141,7 +141,7 @@ internal class AsmAssignSource(val kind: SourceStorageKind,
                 is IdentifierReference -> {
                     val parameter = value.targetVarDecl(program)?.subroutineParameter
                     if(parameter!=null && parameter.definingSubroutine!!.isAsmSubroutine) {
-                        TODO("ASSIGN SOURCE FROM ASMPARAM $parameter ::  $value")
+                        TODO("ASSIGNSOURCE ASMPARAM $parameter ::  $value")
                     }
                     val dt = value.inferType(program).getOr(DataType.UNDEFINED)
                     val varName=asmgen.asmVariableName(value)

codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt

@@ -236,9 +236,9 @@ internal class AugmentableAssignmentAsmGen(private val program: Program,
                         }
                     }
                     else -> {
-                        // TODO OTHER EVALUATION HERE, don't use the estack
+                        // TODO OTHER EVALUATION HERE, don't use the estack to transfer the address to read/write from
                         asmgen.assignExpressionTo(memory.addressExpression, AsmAssignTarget(TargetStorageKind.STACK, program, asmgen, DataType.UWORD, memory.definingSubroutine))
-                        asmgen.out("  jsr  prog8_lib.read_byte_from_address_on_stack |  sta  P8ZP_SCRATCH_B1")      // TODO don't use estack to transfer the address to read from
+                        asmgen.out("  jsr  prog8_lib.read_byte_from_address_on_stack |  sta  P8ZP_SCRATCH_B1")
                         when {
                             valueLv != null -> inplaceModification_byte_litval_to_variable("P8ZP_SCRATCH_B1", DataType.UBYTE, operator, valueLv.toInt())
                             ident != null -> inplaceModification_byte_variable_to_variable("P8ZP_SCRATCH_B1", DataType.UBYTE, operator, ident)
@@ -249,7 +249,7 @@ internal class AugmentableAssignmentAsmGen(private val program: Program,
                             }
                             else -> inplaceModification_byte_value_to_variable("P8ZP_SCRATCH_B1", DataType.UBYTE, operator, value)
                         }
-                        asmgen.out("  lda  P8ZP_SCRATCH_B1 |  jsr  prog8_lib.write_byte_to_address_on_stack | inx")  // TODO don't use estack to transfer the address to read from
+                        asmgen.out("  lda  P8ZP_SCRATCH_B1 |  jsr  prog8_lib.write_byte_to_address_on_stack | inx")
                     }
                 }
             }
@@ -1898,11 +1898,10 @@ internal class AugmentableAssignmentAsmGen(private val program: Program,
         +                           ldx  #1
         +""")
                             }
-                            in Cx16VirtualRegisters -> TODO()
+                            in Cx16VirtualRegisters -> throw AssemblyError("cx16 virtual regs should be variables, not real registers")
                             else -> throw AssemblyError("invalid reg dt for word not")
                         }
                     }
-                    TargetStorageKind.MEMORY -> TODO("no asm gen for uword-memory not")
                     TargetStorageKind.STACK -> TODO("no asm gen for word stack not")
                     else -> throw AssemblyError("no asm gen for in-place not of uword for ${target.kind}")
                 }
@@ -1974,11 +1973,10 @@ internal class AugmentableAssignmentAsmGen(private val program: Program,
                             RegisterOrPair.AX -> asmgen.out("  pha |  txa |  eor  #255 |  tax |  pla |  eor  #255")
                             RegisterOrPair.AY -> asmgen.out("  pha |  tya |  eor  #255 |  tay |  pla |  eor  #255")
                             RegisterOrPair.XY -> asmgen.out("  txa |  eor  #255 |  tax |  tya |  eor  #255 |  tay")
-                            in Cx16VirtualRegisters -> TODO("no asm gen for cx16 word register invert")
+                            in Cx16VirtualRegisters -> throw AssemblyError("cx16 virtual regs should be variables, not real registers")
                             else -> throw AssemblyError("invalid reg dt for word invert")
                         }
                     }
-                    TargetStorageKind.MEMORY -> TODO("no asm gen for uword-memory invert")
                     TargetStorageKind.STACK -> TODO("no asm gen for word stack invert")
                     else -> throw AssemblyError("no asm gen for in-place invert uword for ${target.kind}")
                 }
@@ -2006,9 +2004,9 @@ internal class AugmentableAssignmentAsmGen(private val program: Program,
                             else -> throw AssemblyError("invalid reg dt for byte negate")
                         }
                     }
-                    TargetStorageKind.MEMORY -> TODO("can't in-place negate memory ubyte")
+                    TargetStorageKind.MEMORY -> throw AssemblyError("memory is ubyte, can't in-place negate")
                     TargetStorageKind.STACK -> TODO("no asm gen for byte stack negate")
-                    else -> throw AssemblyError("no asm gen for in-place negate byte array")
+                    else -> throw AssemblyError("no asm gen for in-place negate byte")
                 }
             }
             DataType.WORD -> {
@@ -2063,13 +2061,12 @@ internal class AugmentableAssignmentAsmGen(private val program: Program,
                                     sbc  P8ZP_SCRATCH_REG+1
                                     tay""")
                             }
-                            in Cx16VirtualRegisters -> TODO("no asm gen for cx16 word register negate")
+                            in Cx16VirtualRegisters -> throw AssemblyError("cx16 virtual regs should be variables, not real registers")
                             else -> throw AssemblyError("invalid reg dt for word neg")
                         }
                     }
-                    TargetStorageKind.MEMORY -> TODO("no asm gen for word memory negate")
                     TargetStorageKind.STACK -> TODO("no asm gen for word stack negate")
-                    else -> throw AssemblyError("no asm gen for in-place negate word array")
+                    else -> throw AssemblyError("no asm gen for in-place negate word")
                 }
             }
             DataType.FLOAT -> {
@@ -2082,8 +2079,6 @@ internal class AugmentableAssignmentAsmGen(private val program: Program,
                             sta  ${target.asmVarname}+1
                         """)
                     }
-                    TargetStorageKind.REGISTER -> TODO("no asm gen for float reg negate")
-                    TargetStorageKind.MEMORY -> TODO("no asm gen for float memory negate")
                     TargetStorageKind.STACK -> TODO("no asm gen for stack float negate")
                     else -> throw AssemblyError("weird target kind for inplace negate float ${target.kind}")
                 }

compiler/res/prog8lib/cx16/syslib.p8

@@ -312,7 +312,7 @@ romsub $ff56 = joystick_get(ubyte joynr @A) -> ubyte @A, ubyte @X, ubyte @Y
 romsub $ff4d = clock_set_date_time(uword yearmonth @R0, uword dayhours @R1, uword minsecs @R2, ubyte jiffies @R3)  clobbers(A, X, Y)
 romsub $ff50 = clock_get_date_time()  clobbers(A, X, Y)  -> uword @R0, uword @R1, uword @R2, ubyte @R3   ; result registers see clock_set_date_time()
 
-; TODO specify the correct clobbers for alle these functions below, we now assume all 3 regs are clobbered
+; TODO specify the correct clobbers for all functions below, we now assume all 3 regs are clobbered
 
 ; high level graphics & fonts
 romsub $ff20 = GRAPH_init(uword vectors @R0)  clobbers(A,X,Y)

docs/source/todo.rst

@@ -6,8 +6,9 @@ For next compiler release (7.4)
 Use GoSub to call subroutines (statements):
     - [DONE] allow separate assigns to subroutine's parameter variables / registers
     - [DONE] turn a regular subroutine call into assignments to the parameters + GoSub (take code from gosub branch)
+    - also do this for asmsubs taking >0 parameters
 
-Function call expression:
+Optimize Function calls in expressions:
     - move args to assignments to params
     - add tempvar immediately in front of expression with the fuction call
     - replace the function call in the expression with the tempvar

examples/test.p8

@@ -1,23 +1,22 @@
-%import textio
-%import conv
 
 main {
 
     sub start() {
 
-        ubyte  @shared xx=20
+        &ubyte derp = $c000
-        ubyte  @shared yy=10
 
-        routine2()
+        @($c000) = not @($c000)
-        routine2()
+        @($c000) = ~ @($c000)
-        routine2()
 
+        ubyte uu
+        uu = abs(uu)
+        routine2(12345, true)
         repeat {
-            xx++
         }
+
     }
 
-    asmsub routine2() -> ubyte @A {
+    asmsub routine2(uword num @AY, ubyte switch @Pc) {
         %asm {{
             adc #20
             rts