v2.4

compiler/res/version.txt

@@ -1 +1 @@
-2.3
+2.4

compiler/src/prog8/ast/base/Extensions.kt

@@ -25,12 +25,6 @@ internal fun Program.reorderStatements() {
     reorder.applyModifications()
 }
 
-internal fun Program.inlineSubroutines(): Int {
-    val reorder = SubroutineInliner(this)
-    reorder.visit(this)
-    return reorder.applyModifications()
-}
-
 internal fun Program.addTypecasts(errors: ErrorReporter) {
     val caster = TypecastsAdder(this, errors)
     caster.visit(this)

compiler/src/prog8/ast/processing/SubroutineInliner.kt

@@ -1,39 +0,0 @@
-package prog8.ast.processing
-
-import prog8.ast.Node
-import prog8.ast.Program
-import prog8.ast.statements.*
-import prog8.optimizer.CallGraph
-
-
-internal class SubroutineInliner(private val program: Program) : AstWalker() {
-    private val noModifications = emptyList<IAstModification>()
-    private val callgraph = CallGraph(program)
-
-    override fun after(subroutine: Subroutine, parent: Node): Iterable<IAstModification> {
-
-        if(!subroutine.isAsmSubroutine && callgraph.calledBy[subroutine]!=null && subroutine.containsCodeOrVars()) {
-
-            // TODO for now, inlined subroutines  can't have parameters or local variables - improve this
-            if(subroutine.parameters.isEmpty() && subroutine.containsNoVars()) {
-                if (subroutine.countStatements() <= 5) {
-                    if (callgraph.calledBy.getValue(subroutine).size == 1 || !subroutine.statements.any { it.expensiveToInline })
-                        return inline(subroutine)
-                }
-            }
-        }
-        return noModifications
-    }
-
-    private fun inline(subroutine: Subroutine): Iterable<IAstModification> {
-        val calls = callgraph.calledBy.getValue(subroutine)
-        return calls.map {
-            call -> IAstModification.ReplaceNode(
-                call,
-                AnonymousScope(subroutine.statements, call.position),
-                call.parent
-            )
-        }.plus(IAstModification.Remove(subroutine, subroutine.parent))
-    }
-
-}

compiler/src/prog8/ast/statements/AstStatements.kt

@@ -29,8 +29,6 @@ sealed class Statement : Node {
         return scope.joinToString(".")
     }
 
-    abstract val expensiveToInline: Boolean
-
     fun definingBlock(): Block {
         if(this is Block)
             return this
@@ -48,7 +46,6 @@ class BuiltinFunctionStatementPlaceholder(val name: String, override val positio
     override fun replaceChildNode(node: Node, replacement: Node) {
         replacement.parent = this
     }
-    override val expensiveToInline = false
 }
 
 data class RegisterOrStatusflag(val registerOrPair: RegisterOrPair?, val statusflag: Statusflag?, val stack: Boolean)
@@ -59,8 +56,6 @@ class Block(override val name: String,
             val isInLibrary: Boolean,
             override val position: Position) : Statement(), INameScope {
     override lateinit var parent: Node
-    override val expensiveToInline
-        get() = statements.any { it.expensiveToInline }
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -86,7 +81,6 @@ class Block(override val name: String,
 
 data class Directive(val directive: String, val args: List<DirectiveArg>, override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = false
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -109,7 +103,6 @@ data class DirectiveArg(val str: String?, val name: String?, val int: Int?, over
 
 data class Label(val name: String, override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = false
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -126,7 +119,6 @@ data class Label(val name: String, override val position: Position) : Statement(
 
 open class Return(var value: Expression?, override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = value!=null && value !is NumericLiteralValue
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -158,7 +150,6 @@ class ReturnFromIrq(override val position: Position) : Return(null, position) {
 
 class Continue(override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = false
 
     override fun linkParents(parent: Node) {
         this.parent=parent
@@ -171,7 +162,6 @@ class Continue(override val position: Position) : Statement() {
 
 class Break(override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = false
 
     override fun linkParents(parent: Node) {
         this.parent=parent
@@ -207,9 +197,6 @@ open class VarDecl(val type: VarDeclType,
     var structHasBeenFlattened = false      // set later
         private set
 
-    override val expensiveToInline
-            get() = value!=null && value !is NumericLiteralValue
-
     // prefix for literal values that are turned into a variable on the heap
 
     companion object {
@@ -339,8 +326,6 @@ class ArrayIndex(var index: Expression, override val position: Position) : Node
 
 open class Assignment(var target: AssignTarget, var aug_op : String?, var value: Expression, override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline
-            get() = value is BinaryExpression
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -509,7 +494,6 @@ data class AssignTarget(val register: Register?,
 
 class PostIncrDecr(var target: AssignTarget, val operator: String, override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = false
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -535,7 +519,6 @@ class Jump(val address: Int?,
            val generatedLabel: String?,             // used in code generation scenarios
            override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = false
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -556,8 +539,6 @@ class FunctionCallStatement(override var target: IdentifierReference,
                             val void: Boolean,
                             override val position: Position) : Statement(), IFunctionCall {
     override lateinit var parent: Node
-    override val expensiveToInline
-            get() = args.any { it !is NumericLiteralValue }
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -585,7 +566,6 @@ class FunctionCallStatement(override var target: IdentifierReference,
 
 class InlineAssembly(val assembly: String, override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = true
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -600,8 +580,6 @@ class AnonymousScope(override var statements: MutableList<Statement>,
                      override val position: Position) : INameScope, Statement() {
     override val name: String
     override lateinit var parent: Node
-    override val expensiveToInline
-        get() = statements.any { it.expensiveToInline }
 
     companion object {
         private var sequenceNumber = 1
@@ -630,7 +608,6 @@ class AnonymousScope(override var statements: MutableList<Statement>,
 
 class NopStatement(override val position: Position): Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = false
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -664,11 +641,7 @@ class Subroutine(override val name: String,
                  override val position: Position) : Statement(), INameScope {
 
     var keepAlways: Boolean = false
-    override val expensiveToInline
-            get() = statements.any { it.expensiveToInline }
-
     override lateinit var parent: Node
-
     val scopedname: String by lazy { makeScopedName(name) }
 
     override fun linkParents(parent: Node) {
@@ -746,8 +719,6 @@ class IfStatement(var condition: Expression,
                   var elsepart: AnonymousScope,
                   override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline: Boolean
-        get() = truepart.expensiveToInline || elsepart.expensiveToInline
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -776,8 +747,6 @@ class BranchStatement(var condition: BranchCondition,
                       var elsepart: AnonymousScope,
                       override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline: Boolean
-        get() = truepart.expensiveToInline || elsepart.expensiveToInline
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -805,7 +774,6 @@ class ForLoop(val loopRegister: Register?,
               var body: AnonymousScope,
               override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = true
 
     override fun linkParents(parent: Node) {
         this.parent=parent
@@ -842,7 +810,6 @@ class WhileLoop(var condition: Expression,
                 var body: AnonymousScope,
                 override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = true
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -865,7 +832,6 @@ class WhileLoop(var condition: Expression,
 
 class ForeverLoop(var body: AnonymousScope, override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = true
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -886,7 +852,6 @@ class RepeatLoop(var body: AnonymousScope,
                  var untilCondition: Expression,
                  override val position: Position) : Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline = true
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -911,7 +876,6 @@ class WhenStatement(var condition: Expression,
                     var choices: MutableList<WhenChoice>,
                     override val position: Position): Statement() {
     override lateinit var parent: Node
-    override val expensiveToInline: Boolean = true
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -981,7 +945,6 @@ class StructDecl(override val name: String,
                  override val position: Position): Statement(), INameScope {
 
     override lateinit var parent: Node
-    override val expensiveToInline: Boolean = true
 
     override fun linkParents(parent: Node) {
         this.parent = parent

compiler/src/prog8/compiler/Main.kt

@@ -163,10 +163,9 @@ private fun optimizeAst(programAst: Program, errors: ErrorReporter) {
         // keep optimizing expressions and statements until no more steps remain
         val optsDone1 = programAst.simplifyExpressions()
         val optsDone2 = programAst.optimizeStatements(errors)
-        val optsDone3 = programAst.inlineSubroutines()
         programAst.constantFold(errors) // because simplified statements and expressions could give rise to more constants that can be folded away:
         errors.handle()
-        if (optsDone1 + optsDone2 + optsDone3 == 0)
+        if (optsDone1 + optsDone2 == 0)
             break
     }
 

compiler/src/prog8/compiler/target/c64/codegen/ExpressionsAsmGen.kt

@@ -178,8 +178,8 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge
     private fun translateExpression(expr: RegisterExpr) {
         when(expr.register) {
             Register.A -> asmgen.out(" sta  $ESTACK_LO_HEX,x | dex")
-            Register.X -> asmgen.out(" txa |  sta  $ESTACK_LO_HEX,x | dex")
-            Register.Y -> asmgen.out(" tya |  sta  $ESTACK_LO_HEX,x | dex")
+            Register.X -> asmgen.out(" pha |  txa |  sta  $ESTACK_LO_HEX,x |  dex |  pla")
+            Register.Y -> asmgen.out(" pha |  tya |  sta  $ESTACK_LO_HEX,x |  dex |  pla")
         }
     }
 

compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt

@@ -25,8 +25,45 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
 
         val subName = asmgen.asmIdentifierName(stmt.target)
         if(stmt.args.isNotEmpty()) {
+            if(sub.asmParameterRegisters.isEmpty()) {
+                // via variables
                 for(arg in sub.parameters.withIndex().zip(stmt.args)) {
-                translateFuncArguments(arg.first, arg.second, sub)
+                    argumentViaVariable(sub, arg.first, arg.second)
+                }
+            } else {
+                // via registers
+                if(sub.parameters.size==1) {
+                    // just a single parameter, no risk of clobbering registers
+                    argumentViaRegister(sub, sub.parameters.withIndex().single(), stmt.args[0])
+                } else {
+                    // multiple register arguments, risk of register clobbering.
+                    // evaluate arguments onto the stack, and load the registers from the evaluated values on the stack.
+                    when {
+                        stmt.args.all {it is AddressOf ||
+                                it is NumericLiteralValue ||
+                                it is StructLiteralValue ||
+                                it is StringLiteralValue ||
+                                it is ArrayLiteralValue ||
+                                it is IdentifierReference} -> {
+                            // no risk of clobbering for these simple argument types. Optimize the register loading.
+                            for(arg in sub.parameters.withIndex().zip(stmt.args)) {
+                                argumentViaRegister(sub, arg.first, arg.second)
+                            }
+                        }
+                        stmt.args.all {it is RegisterExpr} -> {
+                            val argRegisters = stmt.args.map {(it as RegisterExpr).register.toString()}
+                            val paramRegisters = sub.asmParameterRegisters.map { it.registerOrPair?.toString() }
+                            if(argRegisters != paramRegisters) {
+                                // all args are registers but differ from the function params. Can't pass directly, work via stack.
+                                argsViaStackEvaluation(stmt, sub)
+                            }
+                        }
+                        else -> {
+                            // Risk of clobbering due to complex expression args. Work via the stack.
+                            argsViaStackEvaluation(stmt, sub)
+                        }
+                    }
+                }
             }
         }
         asmgen.out("  jsr  $subName")
@@ -35,15 +72,47 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
             asmgen.out("  ldx  c64.SCRATCH_ZPREGX")        // restore X again
     }
 
-    private fun translateFuncArguments(parameter: IndexedValue<SubroutineParameter>, value: Expression, sub: Subroutine) {
-        val sourceIDt = value.inferType(program)
-        if(!sourceIDt.isKnown)
-            throw AssemblyError("arg type unknown")
-        val sourceDt = sourceIDt.typeOrElse(DataType.STRUCT)
-        if(!argumentTypeCompatible(sourceDt, parameter.value.type))
-            throw AssemblyError("argument type incompatible")
-        if(sub.asmParameterRegisters.isEmpty()) {
+    private fun argsViaStackEvaluation(stmt: IFunctionCall, sub: Subroutine) {
+        for (arg in stmt.args.reversed())
+            asmgen.translateExpression(arg)
+        for (regparam in sub.asmParameterRegisters) {
+            when (regparam.registerOrPair) {
+                RegisterOrPair.A -> asmgen.out(" inx |  lda  $ESTACK_LO_HEX,x")
+                RegisterOrPair.X -> throw AssemblyError("can't pop into X register - use a variable instead")
+                RegisterOrPair.Y -> asmgen.out(" inx |  ldy  $ESTACK_LO_HEX,x")
+                RegisterOrPair.AX -> throw AssemblyError("can't pop into X register - use a variable instead")
+                RegisterOrPair.AY -> asmgen.out(" inx |  lda  $ESTACK_LO_HEX,x |  ldy  $ESTACK_HI_HEX,x")
+                RegisterOrPair.XY -> throw AssemblyError("can't pop into X register - use a variable instead")
+                null -> {
+                }
+            }
+            when (regparam.statusflag) {
+                Statusflag.Pc -> asmgen.out("""
+                        inx
+                        pha
+                        lda  $ESTACK_LO_HEX,x
+                        beq  +
+                        sec  
+                        bcs  ++
+            +           clc
+            +           pla
+            """)
+                null -> {
+                }
+                else -> throw AssemblyError("can only use Carry as status flag parameter")
+            }
+        }
+    }
+
+    private fun argumentViaVariable(sub: Subroutine, parameter: IndexedValue<SubroutineParameter>, value: Expression) {
         // pass parameter via a regular variable (not via registers)
+        val valueIDt = value.inferType(program)
+        if(!valueIDt.isKnown)
+            throw AssemblyError("arg type unknown")
+        val valueDt = valueIDt.typeOrElse(DataType.STRUCT)
+        if(!argumentTypeCompatible(valueDt, parameter.value.type))
+            throw AssemblyError("argument type incompatible")
+
         val paramVar = parameter.value
         val scopedParamVar = (sub.scopedname+"."+paramVar.name).split(".")
         val target = AssignTarget(null, IdentifierReference(scopedParamVar, sub.position), null, null, sub.position)
@@ -93,8 +162,17 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
                 asmgen.assignFromEvalResult(target)
             }
         }
-        } else {
-            // pass parameter via a register parameter
+    }
+
+    private fun argumentViaRegister(sub: Subroutine, parameter: IndexedValue<SubroutineParameter>, value: Expression) {
+        // pass argument via a register parameter
+        val valueIDt = value.inferType(program)
+        if(!valueIDt.isKnown)
+            throw AssemblyError("arg type unknown")
+        val valueDt = valueIDt.typeOrElse(DataType.STRUCT)
+        if(!argumentTypeCompatible(valueDt, parameter.value.type))
+            throw AssemblyError("argument type incompatible")
+
         val paramRegister = sub.asmParameterRegisters[parameter.index]
         val statusflag = paramRegister.statusflag
         val register = paramRegister.registerOrPair
@@ -143,12 +221,13 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
                             asmgen.translateExpression(value)
                             asmgen.out("""
             inx
+            pha
             lda  $ESTACK_LO_HEX,x
             beq  +
             sec  
             bcs  ++
 +           clc
-+
++           pla
 """)
                         }
                     }
@@ -203,7 +282,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
                     }
                     is IdentifierReference -> {
                         val sourceName = asmgen.asmIdentifierName(value)
-                            if(sourceDt in PassByReferenceDatatypes) {
+                        if(valueDt in PassByReferenceDatatypes) {
                             when (register) {
                                 RegisterOrPair.AX -> asmgen.out("  lda  #<$sourceName  |  ldx  #>$sourceName")
                                 RegisterOrPair.AY -> asmgen.out("  lda  #<$sourceName  |  ldy  #>$sourceName")
@@ -230,7 +309,6 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
             }
         }
     }
-    }
 
     private fun argumentTypeCompatible(argType: DataType, paramType: DataType): Boolean {
         if(argType isAssignableTo paramType)

docs/source/todo.rst

@@ -2,9 +2,6 @@
 TODO
 ====
 
-- BUG FIX: fix register argument clobbering when calling asmsubs. (see fixme_argclobber.p8)
-
-
 - finalize (most) of the still missing "new" assignment asm code generation
 - aliases for imported symbols for example perhaps '%alias print = c64scr.print'
 - option to load library files from a directory instead of the embedded ones (easier library development/debugging)
@@ -20,13 +17,13 @@ Add more compiler optimizations to the existing ones.
 - more targeted optimizations for assigment asm code, such as the following:
 - subroutine calling convention? like: 1 byte arg -> pass in A, 2 bytes -> pass in A+Y, return value likewise.
 - remove unreachable code after an exit(), return or goto
-- working subroutine inlining (start with trivial routines, grow to taking care of vars and identifier refs to them)
 - add a compiler option to not include variable initialization code (useful if the program is expected to run only once, such as a game)
   the program will then rely solely on the values as they are in memory at the time of program startup.
 - Also some library routines and code patterns could perhaps be optimized further
 - can the parameter passing to subroutines be optimized to avoid copying?
 - more optimizations on the language AST level
 - more optimizations on the final assembly source level
+- note: abandoned subroutine inlining because of problems referencing non-local stuff. Can't move everything around.
 
 
 Eval stack redesign? (lot of work)

examples/fixme_argclobber.p8

@@ -1,47 +0,0 @@
-%import c64lib
-%import c64utils
-%import c64flt
-%zeropage basicsafe
-%option enable_floats
-
-
-; TODO: fix register argument clobbering when calling asmsubs.
-; for instance if the first arg goes into Y, and the second in A,
-; but when calculating the second argument clobbers Y, the first argument gets destroyed.
-
-main {
-
-    sub start() {
-        function(20, calculate())
-        asmfunction(20, calculate())
-
-        c64.CHROUT('\n')
-
-        if @($0400)==@($0402) and @($0401) == @($0403) {
-            c64scr.print("ok: results are same\n")
-        } else {
-            c64scr.print("error: result differ; arg got clobbered\n")
-        }
-    }
-
-    sub function(ubyte a1, ubyte a2) {
-        ; non-asm function passes via stack, this is ok
-        @($0400) = a1
-        @($0401) = a2
-    }
-
-    asmsub asmfunction(ubyte a1 @ Y, ubyte a2 @ A) {
-        ; asm-function passes via registers, risk of clobbering
-        %asm {{
-            sty  $0402
-            sta  $0403
-        }}
-    }
-
-    sub calculate() -> ubyte {
-        Y = 99
-        return Y
-    }
-}
-
-

examples/tehtriz.p8

@@ -8,10 +8,6 @@
 ;   some simple sound effects
 
 
-
-;  TODO fix noCollision() at bottom when compiled without optimizations (codegen issue).
-
-
 main {
 
     const ubyte boardOffsetX = 14
@@ -549,7 +545,6 @@ blocklogic {
     sub noCollision(ubyte xpos, ubyte ypos) -> ubyte {
         ubyte i
         for i in 15 downto 0 {
-            ; TODO FIX THIS when compiling without optimizations (codegen problem: clobbering register arguments, see fixme_argclobber):
             if currentBlock[i] and c64scr.getchr(xpos + (i&3), ypos+i/4)!=32
                 return false
         }

examples/test.p8

@@ -5,43 +5,8 @@
 %option enable_floats
 
 
-; TODO: fix register argument clobbering when calling asmsubs.
-; for instance if the first arg goes into Y, and the second in A,
-; but when calculating the second argument clobbers Y, the first argument gets destroyed.
-
 main {
 
     sub start() {
-        function(20, calculate())
-        asmfunction(20, calculate())
-
-        c64.CHROUT('\n')
-
-        if @($0400)==@($0402) and @($0401) == @($0403) {
-            c64scr.print("ok: results are same\n")
-        } else {
-            c64scr.print("error: result differ; arg got clobbered\n")
     }
 }
-
-    sub function(ubyte a1, ubyte a2) {
-        ; non-asm function passes via stack, this is ok
-        @($0400) = a1
-        @($0401) = a2
-    }
-
-    asmsub asmfunction(ubyte a1 @ Y, ubyte a2 @ A) {
-        ; asm-function passes via registers, risk of clobbering
-        %asm {{
-            sty  $0402
-            sta  $0403
-        }}
-    }
-
-    sub calculate() -> ubyte {
-        Y = 99
-        return Y
-    }
-}
-
-

examples/turtle-gfx.p8

@@ -43,26 +43,32 @@ turtle {
         c64.SPENA = 1
         c64.SP0COL = 5
 
-        turtlepos()
+        update_turtle_sprite()
     }
 
-    sub turtlepos() {
+    sub update_turtle_sprite() {
         uword xx = xpos as uword
         c64.SPXY[0] = lsb(xx) + 12
-        if msb(xx)
-            c64.MSIGX = 1
-        else
-            c64.MSIGX = 0
+        c64.MSIGX = msb(xx) > 0
         c64.SPXY[1] = lsb(ypos) + 40
     }
 
+    sub pos(float x, float y) {
+        if pendown {
+            graphics.line(xpos as uword, ypos as ubyte, x as uword, y as ubyte)
+        }
+        xpos = x
+        ypos = y
+        update_turtle_sprite()
+    }
+
     sub fd(uword length) {
         float flen = length as float
         float sx = xpos
         float sy = ypos
         xpos += flen * sin(angle)
         ypos -= flen * cos(angle)
-        turtlepos()
+        update_turtle_sprite()
         if pendown {
             graphics.line(sx as uword, lsb(sy), xpos as uword, lsb(ypos))
         }