From 3050156325452cc9bcfe7164056c9ab5c56b4c3f Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sat, 4 Jul 2020 01:02:36 +0200 Subject: [PATCH] reverted subroutine inlining, it was a mistake --- compiler/res/version.txt | 2 +- compiler/src/prog8/ast/base/Extensions.kt | 6 --- .../prog8/ast/processing/SubroutineInliner.kt | 39 --------------- .../src/prog8/ast/statements/AstStatements.kt | 37 --------------- compiler/src/prog8/compiler/Main.kt | 3 +- docs/source/todo.rst | 2 +- examples/test.p8 | 47 +++++++------------ examples/turtle-gfx.p8 | 20 +++++--- 8 files changed, 34 insertions(+), 122 deletions(-) delete mode 100644 compiler/src/prog8/ast/processing/SubroutineInliner.kt diff --git a/compiler/res/version.txt b/compiler/res/version.txt index bb576dbde..f35288e3d 100644 --- a/compiler/res/version.txt +++ b/compiler/res/version.txt @@ -1 +1 @@ -2.3 +2.4-SNAPSHOT diff --git a/compiler/src/prog8/ast/base/Extensions.kt b/compiler/src/prog8/ast/base/Extensions.kt index 4a864e14c..2cce870e8 100644 --- a/compiler/src/prog8/ast/base/Extensions.kt +++ b/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) diff --git a/compiler/src/prog8/ast/processing/SubroutineInliner.kt b/compiler/src/prog8/ast/processing/SubroutineInliner.kt deleted file mode 100644 index ce6555338..000000000 --- a/compiler/src/prog8/ast/processing/SubroutineInliner.kt +++ /dev/null @@ -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() - private val callgraph = CallGraph(program) - - override fun after(subroutine: Subroutine, parent: Node): Iterable { - - 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 { - 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)) - } - -} diff --git a/compiler/src/prog8/ast/statements/AstStatements.kt b/compiler/src/prog8/ast/statements/AstStatements.kt index a662bb9b2..92a78f30c 100644 --- a/compiler/src/prog8/ast/statements/AstStatements.kt +++ b/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, 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, 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, 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, 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 diff --git a/compiler/src/prog8/compiler/Main.kt b/compiler/src/prog8/compiler/Main.kt index 510d7f513..f913bed10 100644 --- a/compiler/src/prog8/compiler/Main.kt +++ b/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 } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 11340b44a..db7cd9969 100644 --- a/docs/source/todo.rst +++ b/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) diff --git a/examples/test.p8 b/examples/test.p8 index e9fcfec45..8475b9136 100644 --- a/examples/test.p8 +++ b/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 + } + +} diff --git a/examples/turtle-gfx.p8 b/examples/turtle-gfx.p8 index a0c8e1290..06ea06c4f 100644 --- a/examples/turtle-gfx.p8 +++ b/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)) }