more tweaks

codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt

@@ -874,7 +874,7 @@ $repeatLabel    lda  $counterVar
             "%asminclude" -> {
                 val includedName = stmt.args[0].str!!
                 if(stmt.definingModule.source is SourceCode.Generated)
-                    TODO("%asminclude inside non-library, non-filesystem module")
+                    throw AssemblyError("%asminclude inside non-library/non-filesystem module not yet supported")
                 loadAsmIncludeFile(includedName, stmt.definingModule.source).fold(
                     success = { assemblyLines.add(it.trimEnd().trimStart('\n')) },
                     failure = { errors.err(it.toString(), stmt.position) }
@@ -885,7 +885,7 @@ $repeatLabel    lda  $counterVar
                 val offset = if(stmt.args.size>1) ", ${stmt.args[1].int}" else ""
                 val length = if(stmt.args.size>2) ", ${stmt.args[2].int}" else ""
                 if(stmt.definingModule.source is SourceCode.Generated)
-                    TODO("%asmbinary inside non-library, non-filesystem module")
+                    throw AssemblyError("%asmbinary inside non-library/non-filesystem module not yet supported")
                 val sourcePath = Path(stmt.definingModule.source.origin)
                 val includedPath = sourcePath.resolveSibling(includedName)
                 val pathForAssembler = options.outputDir // #54: 64tass needs the path *relative to the .asm file*

codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt

@@ -120,7 +120,9 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
                         asmgen.out("  pla")
                 }
             } else {
-                argumentsViaVariables(sub, call)
+                // arguments via variables
+                for(arg in sub.parameters.withIndex().zip(call.args))
+                    argumentViaVariable(sub, arg.first.value, arg.second)
             }
             asmgen.out("  jsr  $subAsmName")
         }
@@ -186,11 +188,6 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
         }
     }
 
-    private fun argumentsViaVariables(sub: Subroutine, call: IFunctionCall) {
-        for(arg in sub.parameters.withIndex().zip(call.args))
-            argumentViaVariable(sub, arg.first.value, arg.second)
-    }
-
     private fun argumentsViaRegisters(sub: Subroutine, call: IFunctionCall) {
         if(sub.parameters.size==1) {
             argumentViaRegister(sub, IndexedValue(0, sub.parameters.single()), call.args[0])
@@ -207,16 +204,18 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
         }
     }
 
-    private fun registerArgsViaStackEvaluation(stmt: IFunctionCall, sub: Subroutine) {
+    private fun registerArgsViaStackEvaluation(call: IFunctionCall, callee: 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.
-        // TODO find another way to prepare the arguments, without using the eval stack: use a few temporary variables instead, or use push()/pop() like replaceCallAsmSubStatementWithGosub() in the statement reorderer
+        // TODO find another way to prepare the arguments, without using the eval stack: use a few temporary variables instead,
+        //       or use cpu hardware stack like makeGosubWithArgsViaCpuStack() in the statement reorderer
+        //       we can't reuse tryReplaceCallWithGosub() from the StatementReorderer because we can't rewrite an expression node into a gosub *statement*...
 
-        if(sub.parameters.isEmpty())
+        if(callee.parameters.isEmpty())
             return
 
         // load all arguments reversed onto the stack: first arg goes last (is on top).
-        for (arg in stmt.args.reversed())
+        for (arg in call.args.reversed())
             asmgen.translateExpression(arg)
 
         var argForCarry: IndexedValue<Pair<Expression, RegisterOrStatusflag>>? = null
@@ -225,7 +224,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
 
         asmgen.out("  inx")     // align estack pointer
 
-        for(argi in stmt.args.zip(sub.asmParameterRegisters).withIndex()) {
+        for(argi in call.args.zip(callee.asmParameterRegisters).withIndex()) {
             val plusIdxStr = if(argi.index==0) "" else "+${argi.index}"
             when {
                 argi.value.second.statusflag == Statusflag.Pc -> {
@@ -246,7 +245,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
                 }
                 argi.value.second.registerOrPair in Cx16VirtualRegisters -> {
                     // immediately output code to load the virtual register, to avoid clobbering the A register later
-                    when (sub.parameters[argi.index].type) {
+                    when (callee.parameters[argi.index].type) {
                         in ByteDatatypes -> {
                             // only load the lsb of the virtual register
                             asmgen.out(
@@ -315,7 +314,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
             if(argForAregister!=null)
                 asmgen.out("  pla")
         } else {
-            repeat(sub.parameters.size - 1) { asmgen.out("  inx") }       // unwind stack
+            repeat(callee.parameters.size - 1) { asmgen.out("  inx") }       // unwind stack
         }
 
         if(argForCarry!=null)

codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt

@@ -125,8 +125,9 @@ class StatementOptimizer(private val program: Program,
                 return listOf(IAstModification.Remove(functionCallStatement, parent as IStatementContainer))
         }
 
-        // see if we can optimize any complex arguments
+        // see if we can optimize any complex argument expressions to be just a simple variable
         // TODO for now, only works for single-argument functions because we use just 1 temp var: R9
+        // TODO is this still useful at all, when functioncallstatement gets replaced by GoSub?
         if(functionCallStatement.target.nameInSource !in listOf(listOf("pop"), listOf("popw")) && functionCallStatement.args.size==1) {
             val arg = functionCallStatement.args[0]
             if(!arg.isSimple && arg !is TypecastExpression && arg !is IFunctionCall) {

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

@@ -10,7 +10,10 @@ import prog8.ast.statements.Directive
 import prog8.ast.statements.VarDeclOrigin
 import prog8.ast.walk.AstWalker
 import prog8.ast.walk.IAstModification
-import prog8.compilerinterface.*
+import prog8.compilerinterface.CompilationOptions
+import prog8.compilerinterface.ICompilationTarget
+import prog8.compilerinterface.IErrorReporter
+import prog8.compilerinterface.IVariablesAndConsts
 
 
 internal fun Program.checkValid(errors: IErrorReporter, compilerOptions: CompilationOptions) {

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

@@ -8,6 +8,7 @@ import prog8.ast.walk.AstWalker
 import prog8.ast.walk.IAstModification
 import prog8.compilerinterface.BuiltinFunctions
 import prog8.compilerinterface.CompilationOptions
+import prog8.compilerinterface.ICompilationTarget
 import prog8.compilerinterface.IErrorReporter
 
 internal class StatementReorderer(val program: Program,
@@ -409,52 +410,52 @@ internal class StatementReorderer(val program: Program,
     override fun after(functionCallStatement: FunctionCallStatement, parent: Node): Iterable<IAstModification> {
         val function = functionCallStatement.target.targetStatement(program)!!
         checkUnusedReturnValues(functionCallStatement, function, program, errors)
-        return replaceCallByGosub(functionCallStatement, parent, program, options)
+        return tryReplaceCallWithGosub(functionCallStatement, parent, program, options)
     }
 }
 
 
-internal fun replaceCallByGosub(functionCallStatement: FunctionCallStatement,
+internal fun tryReplaceCallWithGosub(functionCallStatement: FunctionCallStatement,
-                                parent: Node,
+                                     parent: Node,
-                                program: Program,
+                                     program: Program,
-                                options: CompilationOptions): Iterable<IAstModification> {
+                                     options: CompilationOptions): Iterable<IAstModification> {
-    val function = functionCallStatement.target.targetStatement(program)!!
+    val callee = functionCallStatement.target.targetStatement(program)!!
-    if(function is Subroutine) {
+    if(callee is Subroutine) {
-        if(function.inline)
+        if(callee.inline)
             return emptyList()
-        return if(function.isAsmSubroutine)
+        return if(callee.isAsmSubroutine)
-            replaceCallAsmSubStatementWithGosub(function, functionCallStatement, parent, options)
+            tryReplaceCallAsmSubWithGosub(functionCallStatement, parent, callee, options.compTarget)
         else
-            replaceCallSubStatementWithGosub(function, functionCallStatement, parent, program)
+            tryReplaceCallNormalSubWithGosub(functionCallStatement, parent, callee, program)
     }
     return emptyList()
 }
 
-private fun replaceCallSubStatementWithGosub(function: Subroutine, call: FunctionCallStatement, parent: Node, program: Program): Iterable<IAstModification> {
+private fun tryReplaceCallNormalSubWithGosub(call: FunctionCallStatement, parent: Node, callee: Subroutine, program: Program): Iterable<IAstModification> {
     val noModifications = emptyList<IAstModification>()
 
-    if(function.parameters.isEmpty()) {
+    if(callee.parameters.isEmpty()) {
         // 0 params -> just GoSub
         return listOf(IAstModification.ReplaceNode(call, GoSub(null, call.target, null, call.position), parent))
     }
 
-    if(function.parameters.size==1) {
+    if(callee.parameters.size==1) {
-        if(function.parameters[0].type in IntegerDatatypes) {
+        if(callee.parameters[0].type in IntegerDatatypes) {
             // optimization: 1 integer param is passed via register(s) directly, not by assignment to param variable
             return noModifications
         }
     }
-    else if(function.parameters.size==2) {
+    else if(callee.parameters.size==2) {
-        if(function.parameters[0].type in ByteDatatypes && function.parameters[1].type in ByteDatatypes) {
+        if(callee.parameters[0].type in ByteDatatypes && callee.parameters[1].type in ByteDatatypes) {
             // optimization: 2 simple byte param is passed via 2 registers directly, not by assignment to param variables
             return noModifications
         }
     }
 
     val assignParams =
-        function.parameters.zip(call.args).map {
+        callee.parameters.zip(call.args).map {
             var argumentValue = it.second
-            val paramIdentifier = IdentifierReference(function.scopedName + it.first.name, argumentValue.position)
+            val paramIdentifier = IdentifierReference(callee.scopedName + it.first.name, argumentValue.position)
             val argDt = argumentValue.inferType(program).getOrElse { throw FatalAstException("invalid dt") }
             if(argDt in ArrayDatatypes) {
                 // pass the address of the array instead
@@ -468,51 +469,63 @@ private fun replaceCallSubStatementWithGosub(function: Subroutine, call: Functio
     return listOf(IAstModification.ReplaceNode(call, scope, parent))
 }
 
-private fun replaceCallAsmSubStatementWithGosub(function: Subroutine, call: FunctionCallStatement, parent: Node, options: CompilationOptions): Iterable<IAstModification> {
+private fun tryReplaceCallAsmSubWithGosub(call: FunctionCallStatement,
+                                          parent: Node,
+                                          callee: Subroutine,
+                                          compTarget: ICompilationTarget): Iterable<IAstModification> {
     val noModifications = emptyList<IAstModification>()
 
-    if(function.parameters.isEmpty()) {
+    if(callee.parameters.isEmpty()) {
         // 0 params -> just GoSub
         val scope = AnonymousScope(mutableListOf(), call.position)
-        if(function.shouldSaveX()) {
+        if(callee.shouldSaveX()) {
             scope.statements += FunctionCallStatement(IdentifierReference(listOf("rsavex"), call.position), mutableListOf(), true, call.position)
         }
         scope.statements += GoSub(null, call.target, null, call.position)
-        if(function.shouldSaveX()) {
+        if(callee.shouldSaveX()) {
             scope.statements += FunctionCallStatement(IdentifierReference(listOf("rrestorex"), call.position), mutableListOf(), true, call.position)
         }
         return listOf(IAstModification.ReplaceNode(call, scope, parent))
-    } else if(!options.compTarget.asmsubArgsHaveRegisterClobberRisk(call.args, function.asmParameterRegisters)) {
+    } else if(!compTarget.asmsubArgsHaveRegisterClobberRisk(call.args, callee.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.
         return noModifications
     } else {
         // clobber risk; evaluate the arguments on the CPU stack first (in reverse order)...
-        val argOrder = options.compTarget.asmsubArgsEvalOrder(function)
+        return makeGosubWithArgsViaCpuStack(call, call.position, parent, callee, compTarget)
-        val scope = AnonymousScope(mutableListOf(), call.position)
-        if(function.shouldSaveX()) {
-            scope.statements += FunctionCallStatement(IdentifierReference(listOf("rsavex"), call.position), mutableListOf(), true, call.position)
-        }
-        argOrder.reversed().forEach {
-            val arg = call.args[it]
-            val param = function.parameters[it]
-            scope.statements += pushCall(arg, param.type, arg.position)
-        }
-        // ... and pop them off again into the registers.
-        argOrder.forEach {
-            val param = function.parameters[it]
-            val targetName = function.scopedName + param.name
-            scope.statements += popCall(targetName, param.type, call.position)
-        }
-        scope.statements += GoSub(null, call.target, null, call.position)
-        if(function.shouldSaveX()) {
-            scope.statements += FunctionCallStatement(IdentifierReference(listOf("rrestorex"), call.position), mutableListOf(), true, call.position)
-        }
-        return listOf(IAstModification.ReplaceNode(call, scope, parent))
     }
 }
 
+private fun makeGosubWithArgsViaCpuStack(call: IFunctionCall,
+                                         position: Position,
+                                         parent: Node,
+                                         callee: Subroutine,
+                                         compTarget: ICompilationTarget): Iterable<IAstModification> {
+
+    val argOrder = compTarget.asmsubArgsEvalOrder(callee)
+    val scope = AnonymousScope(mutableListOf(), position)
+    if(callee.shouldSaveX()) {
+        scope.statements += FunctionCallStatement(IdentifierReference(listOf("rsavex"), position), mutableListOf(), true, position)
+    }
+    argOrder.reversed().forEach {
+        val arg = call.args[it]
+        val param = callee.parameters[it]
+        scope.statements += pushCall(arg, param.type, arg.position)
+    }
+    // ... and pop them off again into the registers.
+    argOrder.forEach {
+        val param = callee.parameters[it]
+        val targetName = callee.scopedName + param.name
+        scope.statements += popCall(targetName, param.type, position)
+    }
+    scope.statements += GoSub(null, call.target, null, position)
+    if(callee.shouldSaveX()) {
+        scope.statements += FunctionCallStatement(IdentifierReference(listOf("rrestorex"), position), mutableListOf(), true, position)
+    }
+    return listOf(IAstModification.ReplaceNode(call as Node, scope, parent))
+}
+
 private fun popCall(targetName: List<String>, dt: DataType, position: Position): FunctionCallStatement {
     return FunctionCallStatement(
         IdentifierReference(listOf(if(dt in ByteDatatypes) "pop" else "popw"), position),

compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt

@@ -200,7 +200,7 @@ internal class VariousCleanups(val program: Program, val errors: IErrorReporter,
     }
 
     override fun after(functionCallStatement: FunctionCallStatement, parent: Node): Iterable<IAstModification> {
-        return replaceCallByGosub(functionCallStatement, parent, program, options)
+        return tryReplaceCallWithGosub(functionCallStatement, parent, program, options)
     }
 }
 

compilerInterfaces/src/prog8/compilerinterface/IVariablesAndConsts.kt

@@ -4,7 +4,10 @@ import prog8.ast.INameScope
 import prog8.ast.base.DataType
 import prog8.ast.base.Position
 import prog8.ast.expressions.Expression
-import prog8.ast.statements.*
+import prog8.ast.statements.Block
+import prog8.ast.statements.Subroutine
+import prog8.ast.statements.VarDecl
+import prog8.ast.statements.ZeropageWish
 
 /**
  * A more convenient way to pass variable (and constant values) definitions to the code generator,

docs/source/todo.rst

@@ -3,11 +3,16 @@ TODO
 
 For next release
 ^^^^^^^^^^^^^^^^
-- attempt to rework registerArgsViaStackEvaluation() to use tempvars or push()/pop() instead of evalstack based evaluation
+- attempt to rework registerArgsViaStackEvaluation() to use tempvars or cpu hardware stack instead of evalstack based evaluation
-  actually, all function call asmgen code should use the same routine to pass arguments (replaceCallAsmSubStatementWithGosub ?)
+- ... UNLESS the expression (functioncall) node is the topmost node of the expression tree ???
-- If all regular function calls (both expression + statement) are then replaced by a GoSub node,
+
-  the only reason the old FunctionCall[stmt/expression] nodes are still present is because they're for a builtin function call.
+- check if the optimization step for single arg func call statements to use R9 is still used/useful in after(functionCallStatement) in StatementOptimizer
-  -> at this time make those a new Node type for the codegenerator
+
+- all function call asmgen code should use the same routine to pass arguments...
+- Also calls to builtin functions are still a FunctionCall node -> make new Node type for these if they're a Statement?
+  So that we at least get rid of the FunctionCallStatement altogether in the codegen.
+
+- see if we can get rid of storing the  origAstTarget in AsmAssignTarget
 
 
 Need help with

examples/test.p8

@@ -1,12 +1,14 @@
-%import textio
 
 main {
     sub start() {
         ubyte xx = 10
         ubyte yy = 10
 
-        routine(xx+yy, yy+99, 99, true)
+        simple(xx+yy)
-
+        void routine(xx+yy, yy+99, 99, true)
+        uword @shared zz = mkword(xx+yy,yy+99)
+        zz = routine(xx+yy, yy+99, 99, true)
+        memory.mem()
     }
 
     uword @shared r_arg
@@ -14,7 +16,13 @@ main {
     ubyte @shared r_arg3
     ubyte @shared r_arg4
 
-    asmsub routine(uword arg @AY, ubyte arg2 @X, ubyte arg3 @R0, ubyte arg4 @Pc) {
+    asmsub simple(ubyte arg @A) {
+        %asm {{
+            rts
+        }}
+    }
+
+    asmsub routine(uword arg @AY, ubyte arg2 @X, ubyte arg3 @R0, ubyte arg4 @Pc) -> ubyte @A {
         %asm {{
             pha
             adc  #0
@@ -25,7 +33,16 @@ main {
             stx  r_arg2
             lda  cx16.r0
             sta  r_arg3
+            lda  #99
             rts
         }}
     }
 }
+
+memory {
+    sub mem() {
+        %asm {{
+            nop
+        }}
+    }
+}