reverted subroutine inlining, it was a mistake

compiler/res/version.txt

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

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
     }
 

docs/source/todo.rst

@@ -20,13 +20,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/test.p8

@@ -12,36 +12,25 @@
 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
+        turtle.pu()
+        turtle.pu()
+        turtle.pu()
+        turtle.pu()
+        turtle.pu()
+        turtle.pu()
+        turtle.pu()
+        turtle.pu()
+        turtle.pu()
+        turtle.pu()
     }
 }
 
 
+turtle {
+    ubyte pendown
+
+    sub pu() {
+        pendown = false
+    }
+
+}

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))
         }