From 870c6ea747476618e85a16d1f266867ef0a7f59c Mon Sep 17 00:00:00 2001
From: Irmen de Jong <irmen@razorvine.net>
Date: Mon, 14 Jan 2019 21:31:14 +0100
Subject: [PATCH] stuff

---
 compiler/src/prog8/ast/AST.kt                 | 24 +----
 compiler/src/prog8/ast/AstChecker.kt          |  6 +-
 .../src/prog8/ast/AstIdentifiersChecker.kt    | 15 +--
 compiler/src/prog8/compiler/Compiler.kt       | 22 +++--
 .../intermediate/IntermediateProgram.kt       |  7 +-
 .../src/prog8/compiler/intermediate/Opcode.kt |  1 +
 .../src/prog8/compiler/target/c64/AsmGen.kt   |  4 +-
 .../prog8/compiler/target/c64/Commodore64.kt  |  4 +
 docs/docs.iml                                 |  4 +-
 docs/source/conf.py                           |  1 -
 examples/assignments.p8                       |  6 +-
 examples/hello.p8                             |  2 +-
 examples/test.p8                              | 95 ++++++++++++-------
 13 files changed, 109 insertions(+), 82 deletions(-)

diff --git a/compiler/src/prog8/ast/AST.kt b/compiler/src/prog8/ast/AST.kt
index 88c04a838..4af598419 100644
--- a/compiler/src/prog8/ast/AST.kt
+++ b/compiler/src/prog8/ast/AST.kt
@@ -347,7 +347,7 @@ interface IFunctionCall {
 interface INameScope {
     val name: String
     val position: Position
-    var statements: MutableList<IStatement>
+    val statements: MutableList<IStatement>
     val parent: Node
 
     fun linkParents(parent: Node)
@@ -376,17 +376,16 @@ interface INameScope {
     }
 
     fun getLabelOrVariable(name: String): IStatement? {
+        // TODO this call is relatively slow.... cache it? make statement list non-mutable and update the cache when it is explicitly updated?
         for (stmt in statements) {
-            if (stmt is Label && stmt.name==name) return stmt
             if (stmt is VarDecl && stmt.name==name) return stmt
+            else if (stmt is Label && stmt.name==name) return stmt
         }
         return null
     }
 
-    fun allLabelsAndVariables(): Map<String, IStatement> {
-        val labelsAndVars = statements.filterIsInstance<Label>() + statements.filterIsInstance<VarDecl>()
-        return labelsAndVars.associate { ((it as? Label)?.name ?: (it as? VarDecl)?.name)!! to it }
-    }
+    fun allLabelsAndVariables(): Set<String> =
+            statements.filterIsInstance<Label>().map { it.name }.toSet() + statements.filterIsInstance<VarDecl>().map { it.name }.toSet()
 
     fun lookup(scopedName: List<String>, statement: Node) : IStatement? {
         if(scopedName.size>1) {
@@ -419,19 +418,6 @@ interface INameScope {
         }
     }
 
-    fun debugPrint() {
-        fun printNames(indent: Int, namespace: INameScope) {
-            println(" ".repeat(4*indent) + "${namespace.name}   ->  ${namespace::class.simpleName} at ${namespace.position}")
-            namespace.allLabelsAndVariables().forEach {
-                println(" ".repeat(4 * (1 + indent)) + "${it.key}   ->  ${it.value::class.simpleName} at ${it.value.position}")
-            }
-            namespace.statements.filterIsInstance<INameScope>().forEach {
-                printNames(indent+1, it)
-            }
-        }
-        printNames(0, this)
-    }
-
     fun isEmpty() = statements.isEmpty()
     fun isNotEmpty() = statements.isNotEmpty()
 
diff --git a/compiler/src/prog8/ast/AstChecker.kt b/compiler/src/prog8/ast/AstChecker.kt
index c1637d4aa..fb209344d 100644
--- a/compiler/src/prog8/ast/AstChecker.kt
+++ b/compiler/src/prog8/ast/AstChecker.kt
@@ -134,7 +134,7 @@ class AstChecker(private val namespace: INameScope,
             printWarning("for loop body is empty", forLoop.position)
 
         if(forLoop.iterable is LiteralValue)
-            checkResult.add(SyntaxError("currently not possible to loop over a literal value directly, use a variable instead", forLoop.position))      // todo loop over literals (by creating a generated variable)
+            checkResult.add(SyntaxError("currently not possible to loop over a literal value directly, define it as a variable instead", forLoop.position))      // todo loop over literals (by creating a generated variable)
 
         if(!forLoop.iterable.isIterable(namespace, heap)) {
             checkResult.add(ExpressionError("can only loop over an iterable type", forLoop.position))
@@ -330,7 +330,9 @@ class AstChecker(private val namespace: INameScope,
             if(subroutine.asmClobbers.intersect(regCounts.keys).isNotEmpty())
                 err("a return register is also in the clobber list")
         } else {
-            // TODO: currently, non-asm subroutines can only take numeric arguments
+            // TODO: currently, non-asm subroutines can only take numeric arguments (including floats)
+            // the way string params are treated is almost okay (their address is passed) but the receiving subroutine treats it as an integer rather than referring back to the original string.
+            // the way array params are treated is buggy; it thinks the subroutine needs a byte parameter in place of a byte[] ...
             if(!subroutine.parameters.all{it.type in NumericDatatypes}) {
                 err("non-asm subroutine can only take numerical parameters (no str/array types) for now")
             }
diff --git a/compiler/src/prog8/ast/AstIdentifiersChecker.kt b/compiler/src/prog8/ast/AstIdentifiersChecker.kt
index 33ab76a13..8f2acfb85 100644
--- a/compiler/src/prog8/ast/AstIdentifiersChecker.kt
+++ b/compiler/src/prog8/ast/AstIdentifiersChecker.kt
@@ -111,12 +111,13 @@ class AstIdentifiersChecker(val heap: HeapValues) : IAstProcessor {
             }
 
             // check that there are no local variables that redefine the subroutine's parameters
-            val definedNames = subroutine.allLabelsAndVariables()   // TODO this call is slow
-            val paramNames = subroutine.parameters.map { it.name }
-            val definedNamesCorrespondingToParameters = definedNames.filter { it.key in paramNames }
-            for(name in definedNamesCorrespondingToParameters) {
-                if(name.value.position != subroutine.position)
-                    nameError(name.key, name.value.position, subroutine)
+            val allDefinedNames = subroutine.allLabelsAndVariables()
+            val paramNames = subroutine.parameters.map { it.name }.toSet()
+            val paramsToCheck = paramNames.intersect(allDefinedNames)
+            for(name in paramsToCheck) {
+                val thing = subroutine.getLabelOrVariable(name)!!
+                if(thing.position != subroutine.position)
+                    nameError(name, thing.position, subroutine)
             }
 
             // inject subroutine params as local variables (if they're not there yet) (for non-kernel subroutines and non-asm parameters)
@@ -126,7 +127,7 @@ class AstIdentifiersChecker(val heap: HeapValues) : IAstProcessor {
             if(subroutine.asmAddress==null) {
                 if(subroutine.asmParameterRegisters.isEmpty()) {
                     subroutine.parameters
-                            .filter { !definedNames.containsKey(it.name) }
+                            .filter { it.name !in allDefinedNames }
                             .forEach {
                                 val vardecl = VarDecl(VarDeclType.VAR, it.type, null, it.name, null, subroutine.position)
                                 vardecl.linkParents(subroutine)
diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt
index 1b611aad2..75f393dc8 100644
--- a/compiler/src/prog8/compiler/Compiler.kt
+++ b/compiler/src/prog8/compiler/Compiler.kt
@@ -163,7 +163,7 @@ private class StatementTranslator(private val prog: IntermediateProgram,
 
     override fun process(block: Block): IStatement {
         prog.newBlock(block.scopedname, block.name, block.address, block.options())
-        processVariables(block)         // @todo optimize initializations with same value: load the value only once  (sort on initalization value, datatype   ?)
+        processVariables(block)
         prog.label("block."+block.scopedname, false)
         prog.line(block.position)
         translate(block.statements)
@@ -183,7 +183,10 @@ private class StatementTranslator(private val prog: IntermediateProgram,
             prog.instr(Opcode.START_PROCDEF)
             prog.line(subroutine.position)
             // note: the caller has already written the arguments into the subroutine's parameter variables.
-            translate(subroutine.statements)
+            val (varinits, others) = subroutine.statements.partition { it is VariableInitializationAssignment }
+            val varInits: List<VariableInitializationAssignment> = varinits as List<VariableInitializationAssignment>
+            translateVarInits(varInits)
+            translate(others)
             val r= super.process(subroutine)
             prog.instr(Opcode.END_PROCDEF)
             return r
@@ -197,12 +200,18 @@ private class StatementTranslator(private val prog: IntermediateProgram,
         }
     }
 
+    private fun translateVarInits(varinits: List<VariableInitializationAssignment>) {
+        // sort by datatype and value, so multiple initializations with the same value can be optimized (to load the value just once)
+        val sortedInits = varinits.sortedWith(compareBy({it.value.resultingDatatype(namespace, heap)}, {it.value.constValue(namespace, heap)?.asNumericValue?.toDouble()}))
+        for (vi in sortedInits)
+            translate(vi)
+    }
+
     private fun translate(statements: List<IStatement>) {
         for (stmt: IStatement in statements) {
             generatedLabelSequenceNumber++
             when (stmt) {
                 is Label -> translate(stmt)
-                is VariableInitializationAssignment -> translate(stmt)        // for initializing vars in a scope
                 is Assignment -> translate(stmt)        // normal and augmented assignments
                 is PostIncrDecr -> translate(stmt)
                 is Jump -> translate(stmt, null)
@@ -1409,13 +1418,6 @@ private class StatementTranslator(private val prog: IntermediateProgram,
         }
     }
 
-    private fun translate(stmt: VariableInitializationAssignment) {
-        // this is an assignment to initialize a variable's value in the scope.
-        // the compiler can perhaps optimize this phase.
-        // todo: optimize variable init by keeping track of the block of init values so it can be copied as a whole via memcopy instead of all separate load value instructions
-        translate(stmt as Assignment)
-    }
-
     private fun translate(stmt: Assignment) {
         prog.line(stmt.position)
         translate(stmt.value)
diff --git a/compiler/src/prog8/compiler/intermediate/IntermediateProgram.kt b/compiler/src/prog8/compiler/intermediate/IntermediateProgram.kt
index 6554c7005..13b7668da 100644
--- a/compiler/src/prog8/compiler/intermediate/IntermediateProgram.kt
+++ b/compiler/src/prog8/compiler/intermediate/IntermediateProgram.kt
@@ -44,7 +44,7 @@ class IntermediateProgram(val name: String, var loadAddress: Int, val heap: Heap
         optimizeMultipleSequentialLineInstrs()
         optimizeCallReturnIntoJump()
         optimizeRestoreXYSaveXYIntoRestoreXY()
-        // todo: optimize stackvm code more
+        // todo: add more optimizations to stackvm code
 
         optimizeRemoveNops()    //  must be done as the last step
         optimizeMultipleSequentialLineInstrs()      // once more
@@ -396,9 +396,8 @@ class IntermediateProgram(val name: String, var loadAddress: Int, val heap: Heap
     fun writeCode(out: PrintStream, embeddedLabels: Boolean=true) {
         out.println("; stackVM program code for '$name'")
         out.println("%memory")
-        if(memory.isNotEmpty()) {
-            TODO("output initial memory values")
-        }
+        if(memory.isNotEmpty())
+            TODO("add support for writing/reading initial memory values")
         out.println("%end_memory")
         out.println("%heap")
         heap.allEntries().forEach {
diff --git a/compiler/src/prog8/compiler/intermediate/Opcode.kt b/compiler/src/prog8/compiler/intermediate/Opcode.kt
index ea6d1270c..a78f2e75c 100644
--- a/compiler/src/prog8/compiler/intermediate/Opcode.kt
+++ b/compiler/src/prog8/compiler/intermediate/Opcode.kt
@@ -263,6 +263,7 @@ enum class Opcode {
     RRESTORE,   // restore all internal registers and status flags
     RRESTOREX,  // restore just X (the evaluation stack pointer)
     RRESTOREY,  // restore just Y (used in for loops for instance)
+
     NOP,        // do nothing
     BREAKPOINT, // breakpoint
     TERMINATE,  // end the program
diff --git a/compiler/src/prog8/compiler/target/c64/AsmGen.kt b/compiler/src/prog8/compiler/target/c64/AsmGen.kt
index 2a36ec4da..9413ed6f2 100644
--- a/compiler/src/prog8/compiler/target/c64/AsmGen.kt
+++ b/compiler/src/prog8/compiler/target/c64/AsmGen.kt
@@ -254,7 +254,7 @@ class AsmGen(val options: CompilationOptions, val program: IntermediateProgram,
         vardecls2asm(block)
         out("")
 
-        val instructionPatternWindowSize = 6
+        val instructionPatternWindowSize = 6        // increase once patterns occur longer than this.
         var processed = 0
 
         if(trace) println("BLOCK: ${block.scopedname}  ${block.address ?: ""}")
@@ -3205,7 +3205,7 @@ class AsmGen(val options: CompilationOptions, val program: IntermediateProgram,
             },
 
             // 16 bit addition avoiding excessive stack usage
-            // @todo optimize this even more with longer asmpatterns (avoid stack use altogether on most common operations)
+            // @todo optimize 8 and 16 bit adds and subs even more with longer asmpatterns (avoid stack use altogether on most common operations)
             AsmPattern(listOf(Opcode.PUSH_VAR_WORD, Opcode.ADD_UW),
                     listOf(Opcode.PUSH_VAR_WORD, Opcode.ADD_W)) { segment ->
                 """
diff --git a/compiler/src/prog8/compiler/target/c64/Commodore64.kt b/compiler/src/prog8/compiler/target/c64/Commodore64.kt
index 6ccb1586a..41087ed62 100644
--- a/compiler/src/prog8/compiler/target/c64/Commodore64.kt
+++ b/compiler/src/prog8/compiler/target/c64/Commodore64.kt
@@ -25,6 +25,10 @@ const val ESTACK_HI = 0xcf00        //  $cf00-$cfff inclusive
 
 class C64Zeropage(options: CompilationOptions) : Zeropage(options) {
 
+
+    // @todo: actually USE the zero page when allocating variables at code generation time
+    //        (ideally, the variables that are used 'most' / inside long loops are allocated in ZP first)
+
     companion object {
         const val SCRATCH_B1 = 0x02
         const val SCRATCH_REG = 0x03    // temp storage for a register
diff --git a/docs/docs.iml b/docs/docs.iml
index 88ad54124..28bcee67e 100644
--- a/docs/docs.iml
+++ b/docs/docs.iml
@@ -2,7 +2,9 @@
 <module type="PYTHON_MODULE" version="4">
   <component name="NewModuleRootManager" inherit-compiler-output="true">
     <exclude-output />
-    <content url="file://$MODULE_DIR$" />
+    <content url="file://$MODULE_DIR$">
+      <excludeFolder url="file://$MODULE_DIR$/build" />
+    </content>
     <orderEntry type="jdk" jdkName="Python 3.7" jdkType="Python SDK" />
     <orderEntry type="sourceFolder" forTests="false" />
   </component>
diff --git a/docs/source/conf.py b/docs/source/conf.py
index f65623a61..1f2c2e0f1 100644
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -171,5 +171,4 @@ texinfo_documents = [
 
 # -- Options for to do extension ----------------------------------------------
 
-# If true, `todo` and `todoList` produce output, else they produce nothing.
 todo_include_todos = True
diff --git a/examples/assignments.p8 b/examples/assignments.p8
index a3c241065..91adfbe2f 100644
--- a/examples/assignments.p8
+++ b/examples/assignments.p8
@@ -311,7 +311,7 @@ ubyte_assignment_to_ubytearray:
     string[mubyte2] = ubarr1[mbyte2]        ; via evaluation
     string[mubyte2] = ubarr1[mubyte2]       ; via evaluation
     string[mubyte2] = string[mubyte2]       ; via evaluation
-    ubarr1[ubarr2[A]] = ubarr2[ubarr1[Y]]   ; via evaluation-- todo check generated asm...
+    ubarr1[ubarr2[A]] = ubarr2[ubarr1[Y]]   ; via evaluation
 
 
 
@@ -366,7 +366,7 @@ byte_assignment_to_bytearray:
     barr2[mubyte2] = barr1[ub]        ; via evaluation
     barr2[mubyte2] = barr1[mbyte2]    ; via evaluation
     barr2[mubyte2] = barr1[mubyte2]   ; via evaluation
-    barr1[ubarr2[A]] = barr2[ubarr1[Y]]   ; via evaluation-- todo check generated asm...
+    barr1[ubarr2[A]] = barr2[ubarr1[Y]]   ; via evaluation
 
 
 byte_assignment_to_membytearray:
@@ -420,7 +420,7 @@ byte_assignment_to_membytearray:
     mbarr1[mubyte2] = barr1[ub]        ; via evaluation
     mbarr1[mubyte2] = barr1[mbyte2]    ; via evaluation
     mbarr1[mubyte2] = barr1[mubyte2]   ; via evaluation
-    mbarr1[ubarr2[A]] = barr2[ubarr1[Y]]   ; via evaluation-- todo check generated asm...
+    mbarr1[ubarr2[A]] = barr2[ubarr1[Y]]   ; via evaluation
 
 
 
diff --git a/examples/hello.p8 b/examples/hello.p8
index 76414acda..4e681b8bf 100644
--- a/examples/hello.p8
+++ b/examples/hello.p8
@@ -17,7 +17,7 @@
         str question = "How are you?\n"
 
         ; use iteration to write text
-        for ubyte char in question {            ; @todo fix iteration
+        for ubyte char in question {
             c64.CHROUT(char)
         }
 
diff --git a/examples/test.p8 b/examples/test.p8
index 9e19a6334..7232b153f 100644
--- a/examples/test.p8
+++ b/examples/test.p8
@@ -5,40 +5,71 @@
 
     sub start()  {
 
-        c64scr.print_ub(c64utils.str2ubyte("1"))
-        c64.CHROUT('\n')
-        c64scr.print_ub(c64utils.str2ubyte("12"))
-        c64.CHROUT('\n')
-        c64scr.print_ub(c64utils.str2ubyte("123"))
-        c64.CHROUT('\n')
-        c64scr.print_ub(c64utils.str2ubyte("1234"))
-        c64.CHROUT('\n')
-        c64scr.print_ub(c64utils.str2ubyte("12xyz"))
-        c64.CHROUT('\n')
-        c64.CHROUT('\n')
 
-        c64scr.print_ub(c64utils.str2byte("1"))
-        c64.CHROUT('\n')
-        c64scr.print_ub(c64utils.str2byte("12"))
-        c64.CHROUT('\n')
-        c64scr.print_ub(c64utils.str2byte("123"))
-        c64.CHROUT('\n')
-        c64scr.print_ub(c64utils.str2byte("1234"))
-        c64.CHROUT('\n')
-        c64scr.print_ub(c64utils.str2ubyte("12xyz"))
-        c64.CHROUT('\n')
-        c64.CHROUT('\n')
+        ubyte b1=42
+        ubyte b2=42
+        ubyte b3=99
+        ubyte b4=42
+        word  w1=42
+        word  w2=42
+        word  w3=99
+        word  w4=42
+        uword  uw1=42
+        uword  uw2=42
+        uword  uw3=99
+        uword  uw4=42
+        float f1 = 3.14
+        float f2 = 3.14
+        float f3 = 99.0
+        float f4 = 3.14
+        str  string1="hoi"
+        str  string2="hoi"
+        str  string3="hoi222"
+        str  string4="hoi"
+
+
+
+;
+;        c64scr.print_ub(c64utils.str2ubyte("1"))
+;        c64.CHROUT('\n')
+;        c64scr.print_ub(c64utils.str2ubyte("12"))
+;        c64.CHROUT('\n')
+;        c64scr.print_ub(c64utils.str2ubyte("123"))
+;        c64.CHROUT('\n')
+;        c64scr.print_ub(c64utils.str2ubyte("1234"))
+;        c64.CHROUT('\n')
+;        c64scr.print_ub(c64utils.str2ubyte("12xyz"))
+;        c64.CHROUT('\n')
+;        c64.CHROUT('\n')
+;
+;        c64scr.print_ub(c64utils.str2byte("1"))
+;        c64.CHROUT('\n')
+;        c64scr.print_ub(c64utils.str2byte("12"))
+;        c64.CHROUT('\n')
+;        c64scr.print_ub(c64utils.str2byte("123"))
+;        c64.CHROUT('\n')
+;        c64scr.print_ub(c64utils.str2byte("1234"))
+;        c64.CHROUT('\n')
+;        c64scr.print_ub(c64utils.str2ubyte("12xyz"))
+;        c64.CHROUT('\n')
+;        c64.CHROUT('\n')
+;
+;        c64scr.print_b(c64utils.str2byte("-1"))
+;        c64.CHROUT('\n')
+;        c64scr.print_b(c64utils.str2byte("-12"))
+;        c64.CHROUT('\n')
+;        c64scr.print_b(c64utils.str2byte("-123"))
+;        c64.CHROUT('\n')
+;        c64scr.print_b(c64utils.str2byte("-1111"))
+;        c64.CHROUT('\n')
+;        c64scr.print_b(c64utils.str2byte("-12xyz"))
+;        c64.CHROUT('\n')
 
-        c64scr.print_b(c64utils.str2byte("-1"))
-        c64.CHROUT('\n')
-        c64scr.print_b(c64utils.str2byte("-12"))
-        c64.CHROUT('\n')
-        c64scr.print_b(c64utils.str2byte("-123"))
-        c64.CHROUT('\n')
-        c64scr.print_b(c64utils.str2byte("-1234"))
-        c64.CHROUT('\n')
-        c64scr.print_b(c64utils.str2byte("-12xyz"))
-        c64.CHROUT('\n')
     }
+
+    sub foo(ubyte param1, ubyte param2) {
+        ubyte local1
+    }
+
 }