big compiler speedup due to optimized scope lookups

README.md

@@ -31,13 +31,13 @@ which aims to provide many conveniences over raw assembly code (even when using
 
 Rapid edit-compile-run-debug cycle:
 
-- use modern PC to work on
-- quick compilation times (seconds)
-- option to automatically run the program in the Vice emulator
+- use a modern PC to do the work on
+- very quick compilation times
+- can automatically run the program in the Vice emulator after succesful compilation
 - breakpoints, that let the Vice emulator drop into the monitor if execution hits them
 - source code labels automatically loaded in Vice emulator so it can show them in disassembly
 
-It is mainly targeted at the Commodore-64 machine at this time.
+Prog8 is mainly targeted at the Commodore-64 machine at this time.
 Contributions to add support for other 8-bit (or other?!) machines are welcome.
 
 Documentation/manual

compiler/build.gradle

@@ -1,11 +1,11 @@
 buildscript {
     dependencies {
-        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.3.70"
+        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.3.72"
     }
 }
 
 plugins {
-    // id "org.jetbrains.kotlin.jvm" version "1.3.70"
+    // id "org.jetbrains.kotlin.jvm" version "1.3.72"
     id 'application'
     id 'org.jetbrains.dokka' version "0.9.18"
     id 'com.github.johnrengelman.shadow' version '5.2.0'

compiler/src/prog8/ast/AstToplevel.kt

@@ -53,33 +53,31 @@ interface INameScope {
 
     fun linkParents(parent: Node)
 
-    fun subScopes(): Map<String, INameScope> {
-        // TODO PERFORMANCE: this is called very often and is relatively expensive. Optimize this.
-        val subscopes = mutableMapOf<String, INameScope>()
+    fun subScope(name: String): INameScope? {
         for(stmt in statements) {
             when(stmt) {
                 // NOTE: if other nodes are introduced that are a scope, or contain subscopes, they must be added here!
-                is ForLoop -> subscopes[stmt.body.name] = stmt.body
-                is RepeatLoop -> subscopes[stmt.body.name] = stmt.body
-                is WhileLoop -> subscopes[stmt.body.name] = stmt.body
+                is ForLoop -> if(stmt.body.name==name) return stmt.body
+                is RepeatLoop -> if(stmt.body.name==name) return stmt.body
+                is WhileLoop -> if(stmt.body.name==name) return stmt.body
                 is BranchStatement -> {
-                    subscopes[stmt.truepart.name] = stmt.truepart
-                    if(stmt.elsepart.containsCodeOrVars())
-                        subscopes[stmt.elsepart.name] = stmt.elsepart
+                    if(stmt.truepart.name==name) return stmt.truepart
+                    if(stmt.elsepart.containsCodeOrVars() && stmt.elsepart.name==name) return stmt.elsepart
                 }
                 is IfStatement -> {
-                    subscopes[stmt.truepart.name] = stmt.truepart
-                    if(stmt.elsepart.containsCodeOrVars())
-                        subscopes[stmt.elsepart.name] = stmt.elsepart
+                    if(stmt.truepart.name==name) return stmt.truepart
+                    if(stmt.elsepart.containsCodeOrVars() && stmt.elsepart.name==name) return stmt.elsepart
                 }
                 is WhenStatement -> {
-                    stmt.choices.forEach { subscopes[it.statements.name] = it.statements }
+                    val scope = stmt.choices.firstOrNull { it.statements.name==name }
+                    if(scope!=null)
+                        return scope.statements
                 }
-                is INameScope -> subscopes[stmt.name] = stmt
+                is INameScope -> if(stmt.name==name) return stmt
                 else -> {}
             }
         }
-        return subscopes
+        return null
     }
 
     fun getLabelOrVariable(name: String): Statement? {
@@ -127,7 +125,7 @@ interface INameScope {
             for(module in localContext.definingModule().program.modules) {
                 var scope: INameScope? = module
                 for(name in scopedName.dropLast(1)) {
-                    scope = scope?.subScopes()?.get(name)           // TODO PERFORMANCE: EXPENSIVE! (creates new map every call) OPTIMIZE.
+                    scope = scope?.subScope(name)
                     if(scope==null)
                         break
                 }
@@ -135,7 +133,7 @@ interface INameScope {
                     val result = scope.getLabelOrVariable(scopedName.last())
                     if(result!=null)
                         return result
-                    return scope.subScopes()[scopedName.last()] as Statement?
+                    return scope.subScope(scopedName.last()) as Statement?
                 }
             }
             return null
@@ -147,7 +145,7 @@ interface INameScope {
                 val result = localScope.getLabelOrVariable(scopedName[0])
                 if (result != null)
                     return result
-                val subscope = localScope.subScopes()[scopedName[0]] as Statement?
+                val subscope = localScope.subScope(scopedName[0]) as Statement?
                 if (subscope != null)
                     return subscope
                 // not found in this scope, look one higher up
@@ -213,7 +211,7 @@ class Program(val name: String, val modules: MutableList<Module>): Node {
         return if(mainBlocks.isEmpty()) {
             null
         } else {
-            mainBlocks[0].subScopes()["start"] as Subroutine?
+            mainBlocks[0].subScope("start") as Subroutine?
         }
     }
 

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

@@ -25,7 +25,7 @@ internal class AstChecker(private val program: Program,
             errors.err("there is no 'main' block", program.modules.firstOrNull()?.position ?: program.position)
 
         for(mainBlock in mainBlocks) {
-            val startSub = mainBlock.subScopes()["start"] as? Subroutine
+            val startSub = mainBlock.subScope("start") as? Subroutine
             if (startSub == null) {
                 errors.err("missing program entrypoint ('start' subroutine in 'main' block)", mainBlock.position)
             } else {
@@ -58,7 +58,7 @@ internal class AstChecker(private val program: Program,
         if(irqBlocks.size>1)
             errors.err("more than one 'irq' block", irqBlocks[0].position)
         for(irqBlock in irqBlocks) {
-            val irqSub = irqBlock.subScopes()["irq"] as? Subroutine
+            val irqSub = irqBlock.subScope("irq") as? Subroutine
             if (irqSub != null) {
                 if (irqSub.parameters.isNotEmpty() || irqSub.returntypes.isNotEmpty())
                     errors.err("irq entrypoint subroutine can't have parameters and/or return values", irqSub.position)

compiler/src/prog8/compiler/Main.kt

@@ -5,6 +5,7 @@ import prog8.ast.Program
 import prog8.ast.base.*
 import prog8.ast.statements.Directive
 import prog8.compiler.target.CompilationTarget
+import prog8.optimizer.UnusedCodeRemover
 import prog8.optimizer.constantFold
 import prog8.optimizer.optimizeStatements
 import prog8.optimizer.simplifyExpressions
@@ -170,6 +171,9 @@ private fun optimizeAst(programAst: Program, errors: ErrorReporter) {
     // because simplified statements and expressions could give rise to more constants that can be folded away:
     programAst.constantFold(errors)
     errors.handle()
+
+    val remover = UnusedCodeRemover()
+    remover.visit(programAst)
 }
 
 private fun postprocessAst(programAst: Program, errors: ErrorReporter, compilerOptions: CompilationOptions) {

compiler/src/prog8/optimizer/StatementOptimizer.kt

@@ -1,7 +1,6 @@
 package prog8.optimizer
 
 import prog8.ast.INameScope
-import prog8.ast.Module
 import prog8.ast.Program
 import prog8.ast.base.*
 import prog8.ast.expressions.*
@@ -24,11 +23,10 @@ internal class StatementOptimizer(private val program: Program,
         private set
 
     private val pureBuiltinFunctions = BuiltinFunctions.filter { it.value.pure }
-    private val callgraph = CallGraph(program)      // TODO PERFORMANCE: it is expensive to create this every round
+    private val callgraph = CallGraph(program)
     private val vardeclsToRemove = mutableListOf<VarDecl>()
 
     override fun visit(program: Program) {
-        removeUnusedCode(callgraph)
         super.visit(program)
 
         for(decl in vardeclsToRemove) {
@@ -36,46 +34,6 @@ internal class StatementOptimizer(private val program: Program,
         }
     }
 
-    private fun removeUnusedCode(callgraph: CallGraph) {
-        // TODO PERFORMANCE: expensive code (because of callgraph) OPTIMIZE THIS: only run once separately ?
-        // remove all subroutines that aren't called, or are empty
-        val removeSubroutines = mutableSetOf<Subroutine>()
-        val entrypoint = program.entrypoint()
-        program.modules.forEach {
-            callgraph.forAllSubroutines(it) { sub ->
-                if (sub !== entrypoint && !sub.keepAlways && (sub.calledBy.isEmpty() || (sub.containsNoCodeNorVars() && !sub.isAsmSubroutine)))
-                    removeSubroutines.add(sub)
-            }
-        }
-
-        if (removeSubroutines.isNotEmpty()) {
-            removeSubroutines.forEach {
-                it.definingScope().remove(it)
-            }
-        }
-
-        val removeBlocks = mutableSetOf<Block>()
-        program.modules.flatMap { it.statements }.filterIsInstance<Block>().forEach { block ->
-            if (block.containsNoCodeNorVars() && "force_output" !in block.options())
-                removeBlocks.add(block)
-        }
-
-        if (removeBlocks.isNotEmpty()) {
-            removeBlocks.forEach { it.definingScope().remove(it) }
-        }
-
-        // remove modules that are not imported, or are empty (unless it's a library modules)
-        val removeModules = mutableSetOf<Module>()
-        program.modules.forEach {
-            if (!it.isLibraryModule && (it.importedBy.isEmpty() || it.containsNoCodeNorVars()))
-                removeModules.add(it)
-        }
-
-        if (removeModules.isNotEmpty()) {
-            program.modules.removeAll(removeModules)
-        }
-    }
-
     override fun visit(block: Block): Statement {
         if("force_output" !in block.options()) {
             if (block.containsNoCodeNorVars()) {

compiler/src/prog8/optimizer/UnusedCodeRemover.kt

@@ -0,0 +1,55 @@
+package prog8.optimizer
+
+import prog8.ast.Module
+import prog8.ast.Program
+import prog8.ast.processing.IAstModifyingVisitor
+import prog8.ast.statements.Block
+import prog8.ast.statements.Subroutine
+
+
+internal class UnusedCodeRemover: IAstModifyingVisitor {
+
+
+    override fun visit(program: Program) {
+        val callgraph = CallGraph(program)
+
+        // remove all subroutines that aren't called, or are empty
+        val removeSubroutines = mutableSetOf<Subroutine>()
+        val entrypoint = program.entrypoint()
+        program.modules.forEach {
+            callgraph.forAllSubroutines(it) { sub ->
+                if (sub !== entrypoint && !sub.keepAlways && (sub.calledBy.isEmpty() || (sub.containsNoCodeNorVars() && !sub.isAsmSubroutine)))
+                    removeSubroutines.add(sub)
+            }
+        }
+
+        if (removeSubroutines.isNotEmpty()) {
+            removeSubroutines.forEach {
+                it.definingScope().remove(it)
+            }
+        }
+
+        val removeBlocks = mutableSetOf<Block>()
+        program.modules.flatMap { it.statements }.filterIsInstance<Block>().forEach { block ->
+            if (block.containsNoCodeNorVars() && "force_output" !in block.options())
+                removeBlocks.add(block)
+        }
+
+        if (removeBlocks.isNotEmpty()) {
+            removeBlocks.forEach { it.definingScope().remove(it) }
+        }
+
+        // remove modules that are not imported, or are empty (unless it's a library modules)
+        val removeModules = mutableSetOf<Module>()
+        program.modules.forEach {
+            if (!it.isLibraryModule && (it.importedBy.isEmpty() || it.containsNoCodeNorVars()))
+                removeModules.add(it)
+        }
+
+        if (removeModules.isNotEmpty()) {
+            program.modules.removeAll(removeModules)
+        }
+
+        super.visit(program)
+    }
+}

docs/source/todo.rst

@@ -31,13 +31,13 @@ Eval stack redesign? (lot of work)
 
 The eval stack is now a split lsb/msb stack using X as the stackpointer.
 Is it easier/faster to just use a single page unsplit stack?
-It could then even be moved into the zeropage to greatly reduce code size and slowness.
+It could then even be moved into the zeropage to reduce code size and slowness.
 
 Or just move the LSB portion into a slab of the zeropage.
 
 Allocate a fixed word in ZP that is the Top Of Stack value so we can always operate on TOS directly
 without having to index with X into the eval stack all the time?
-This could GREATLY improvde code size and speed for operatios that work on just a single value.
+This could GREATLY improve code size and speed for operations that work on just a single value.
 
 
 Bug Fixing

examples/tehtriz.p8

@@ -6,8 +6,6 @@
 ;   shows next piece
 ;   staged speed increase
 ;   some simple sound effects
-;
-; @todo show ghost?
 
 main {