tweak subroutine order

compiler/examples/test.p8

@@ -4,15 +4,32 @@
 
 ~ main {
 
-asmsub	MOVFM		(mflpt: word @ AY) -> clobbers(A,Y) -> ()	= $bba2		; load mflpt value from memory  in A/Y into fac1
-asmsub	FREADMEM	() -> clobbers(A,Y) -> ()			= $bba6		; load mflpt value from memory  in $22/$23 into fac1
-asmsub	CONUPK		(mflpt: word @ AY) -> clobbers(A,Y) -> ()	= $ba8c		; load mflpt value from memory  in A/Y into fac2
-asmsub	FAREADMEM	() -> clobbers(A,Y) -> ()			= $ba90		; load mflpt value from memory  in $22/$23 into fac2
-asmsub	MOVFA		() -> clobbers(A,X) -> ()			= $bbfc		; copy fac2 to fac1
-asmsub	MOVAF		() -> clobbers(A,X) -> ()			= $bc0c		; copy fac1 to fac2  (rounded)
-asmsub	MOVEF		() -> clobbers(A,X) -> ()			= $bc0f		; copy fac1 to fac2
-asmsub	FTOMEMXY	(mflpt: word @ XY) -> clobbers(A,Y) -> ()	= $bbd4		; store fac1 to memory  X/Y as 5-byte mflpt
 
+sub sub1() -> byte {
+    return 11
+}
+
+
+sub sub2() -> byte {
+    return 22
+}
+
+    A=55
+
+sub sub3() -> byte {
+    return 33
+}
+
+    X=34
+
+sub sub4() -> byte {
+    return 44
+}
+
+    A=sub1()
+    A=sub2()
+    A=sub3()
+    A=sub4()
 
 sub start() {
 
@@ -22,20 +39,38 @@ sub start() {
 
     bvar = 1
     bvar = 2.0
-    ;bvar = 2.w     ; @todo don't crash
+    bvar = 2.w
-    wvar = 1        ; @todo optimize byte literal to word literal
+    bvar = 255.w
+    wvar = 1
     wvar = 2.w
     wvar = 2.0
     wvar = bvar
-    fvar = 1        ; @todo optimize byte literal to float literal
+    fvar = 1
-    fvar = 2.w      ; @todo optimize word literal to float literal
+    fvar = 2.w
     fvar = 22.33
     fvar = bvar
     fvar = wvar
 
-    MOVAF()
-
     return
 
 }
+
+    A=sub1()
+    A=sub2()
+    A=sub3()
+    A=sub4()
+    sub4()
+
+
+}
+
+
+~ test {
+
+    sub test() -> byte {
+        return 44
+    }
+    sub test2() -> byte {
+        return 43
+    }
 }

compiler/src/prog8/CompilerMain.kt

@@ -55,6 +55,7 @@ fun main(args: Array<String>) {
         val heap = HeapValues()
         moduleAst.checkIdentifiers()
         moduleAst.constantFold(namespace, heap)
+        StatementReorderer().process(moduleAst)     // reorder statements to please the compiler later
         moduleAst.checkValid(namespace, compilerOptions, heap)          // check if tree is valid
 
         // optimize the parse tree
@@ -68,7 +69,6 @@ fun main(args: Array<String>) {
                 break
         }
 
-        StatementReorderer().process(moduleAst)     // reorder statements to please the compiler later
         namespace = moduleAst.definingScope()       // create it again, it could have changed in the meantime
         moduleAst.checkValid(namespace, compilerOptions, heap)          // check if final tree is valid
         moduleAst.checkRecursion(namespace)         // check if there are recursive subroutine calls

compiler/src/prog8/ast/AstChecker.kt

@@ -171,7 +171,6 @@ class AstChecker(private val namespace: INameScope,
             checkResult.add(SyntaxError("block memory address must be valid integer 0..\$ffff", block.position))
         }
 
-        checkSubroutinesPrecededByReturnOrJumpAndFollowedByLabelOrSub(block.statements)
         return super.process(block)
     }
 
@@ -200,7 +199,6 @@ class AstChecker(private val namespace: INameScope,
             err("parameter names must be unique")
 
         super.process(subroutine)
-        checkSubroutinesPrecededByReturnOrJumpAndFollowedByLabelOrSub(subroutine.statements)
 
         // subroutine must contain at least one 'return' or 'goto'
         // (or if it has an asm block, that must contain a 'rts' or 'jmp')
@@ -230,30 +228,6 @@ class AstChecker(private val namespace: INameScope,
         return subroutine
     }
 
-    private fun checkSubroutinesPrecededByReturnOrJumpAndFollowedByLabelOrSub(statements: MutableList<IStatement>) {
-        // @todo hmm, or move all the subroutines at the end of the block? (no fall-through execution)
-        var preceding: IStatement = BuiltinFunctionStatementPlaceholder("dummy", Position("<<dummy>>", 0, 0,0 ))
-        var checkNext = false
-        for (stmt in statements) {
-            if(checkNext) {
-                if(stmt !is Label && stmt !is Subroutine)
-                    checkResult.add(SyntaxError("preceding subroutine definition at line ${preceding.position.line} must be followed here by a label, another subroutine statement, or nothing", stmt.position))
-                return
-            }
-            if(stmt is Subroutine) {
-                if(preceding !is Return
-                        && preceding !is Jump
-                        && preceding !is Subroutine
-                        && preceding !is VarDecl
-                        && preceding !is BuiltinFunctionStatementPlaceholder) {
-                    checkResult.add(SyntaxError("subroutine definition must be preceded by a return, jump, vardecl, or another subroutine statement", stmt.position))
-                }
-                checkNext=true
-            }
-            preceding = stmt
-        }
-    }
-
     /**
      * Assignment target must be register, or a variable name
      * Also check data type compatibility
@@ -336,7 +310,7 @@ class AstChecker(private val namespace: INameScope,
             err("recursive var declaration")
         }
 
-        // for now, variables can only be declared in a block or subroutine (not in a loop statement block)  @todo fix this
+        // for now, variables can only be declared in a block or subroutine (not in a loop statement block)  @todo fix this (anonymous namescope)
         if(decl.parent !is Block && decl.parent !is Subroutine) {
             err ("variables must be declared at block or subroutine level")
         }

compiler/src/prog8/ast/StmtReorderer.kt

@@ -7,7 +7,9 @@ class StatementReorderer: IAstProcessor {
     //      -- the directives '%output', '%launcher', '%zeropage', '%address' and '%option' will come first.
     //      -- all vardecls then follow.
     //      -- the remaining statements then follow in their original order.
+    //
     // - the 'start' subroutine in the 'main' block will be moved to the top immediately following the directives.
+    // - all other subroutines will be moved to the end of their block.
 
     private val directivesToMove = setOf("%output", "%launcher", "%zeropage", "%address", "%option")
 
@@ -25,11 +27,42 @@ class StatementReorderer: IAstProcessor {
     }
 
     override fun process(block: Block): IStatement {
-        val startSub = block.statements.singleOrNull {it is Subroutine && it.name=="start"}
+        val subroutines = block.statements.asSequence().filter { it is Subroutine }.map { it as Subroutine }.toList()
-        if(startSub!=null) {
+        var numSubroutinesAtEnd = 0
-            block.statements.remove(startSub)
+        // move all subroutines to the end of the block
-            block.statements.add(0, startSub)
+        for (subroutine in subroutines) {
+            if(subroutine.name!="start" || block.name!="main") {
+                block.statements.remove(subroutine)
+                block.statements.add(subroutine)
+            }
+            numSubroutinesAtEnd++
         }
+        // move the "start" subroutine to the top
+        if(block.name=="main") {
+            block.statements.singleOrNull { it is Subroutine && it.name == "start" } ?.let {
+                block.statements.remove(it)
+                block.statements.add(0, it)
+                numSubroutinesAtEnd--
+            }
+        }
+
+        // make sure there is a 'return' in front of the first subroutine
+        // (if it isn't the first statement in the block itself, and isn't the program's entrypoint)
+        if(block.statements.size > numSubroutinesAtEnd) {
+            val firstSub = block.statements[block.statements.size - numSubroutinesAtEnd] as Subroutine
+            if(firstSub.name != "start" && block.name != "main") {
+                val stmtBeforeFirstSub = block.statements[block.statements.size - numSubroutinesAtEnd - 1]
+                if (stmtBeforeFirstSub !is Return
+                        && stmtBeforeFirstSub !is Jump
+                        && stmtBeforeFirstSub !is Subroutine
+                        && stmtBeforeFirstSub !is BuiltinFunctionStatementPlaceholder) {
+                    val ret = Return(emptyList(), stmtBeforeFirstSub.position)
+                    ret.linkParents(stmtBeforeFirstSub.parent)
+                    block.statements.add(block.statements.size - numSubroutinesAtEnd, ret)
+                }
+            }
+        }
+
         val varDecls = block.statements.filter { it is VarDecl }
         block.statements.removeAll(varDecls)
         block.statements.addAll(0, varDecls)

compiler/src/prog8/compiler/Compiler.kt

@@ -537,7 +537,7 @@ private class StatementTranslator(private val stackvmProg: StackVmProgram,
                     translateFunctionCall(funcname, expr.arglist)
                 } else {
                     when(target) {
-                        is Subroutine -> translateSubroutineCall(target, expr.arglist)
+                        is Subroutine -> translateSubroutineCall(target, expr.arglist, expr.position)
                         else -> TODO("non-builtin-function call to $target")
                     }
                 }
@@ -608,7 +608,7 @@ private class StatementTranslator(private val stackvmProg: StackVmProgram,
             is Label ->
                 stackvmProg.instr(Opcode.CALL, callLabel = targetStmt.scopedname)
             is Subroutine -> {
-                translateSubroutineCall(targetStmt, stmt.arglist)
+                translateSubroutineCall(targetStmt, stmt.arglist, stmt.position)
                 // make sure we clean up the unused result values from the stack.
                 for(rv in targetStmt.returnvalues) {
                     val opcode=opcodeDiscard(rv)
@@ -649,8 +649,9 @@ private class StatementTranslator(private val stackvmProg: StackVmProgram,
         }
     }
 
-    private fun translateSubroutineCall(subroutine: Subroutine, arguments: List<IExpression>) {
+    private fun translateSubroutineCall(subroutine: Subroutine, arguments: List<IExpression>, callPosition: Position) {
         // evaluate the arguments and assign them into the subroutine's argument variables.
+        stackvmProg.line(callPosition)
         for(arg in arguments.zip(subroutine.parameters)) {
             translate(arg.first)
             val opcode=opcodePopvar(arg.second.type)

compiler/src/prog8/stackvm/Program.kt

@@ -85,6 +85,8 @@ class Program (val name: String,
 
             while(true) {
                 val (lineNr, line) = lines.next()
+                if(line.isEmpty())
+                    continue
                 if(line=="%end_instructions")
                     return Pair(instructions, labels)
                 if(!line.startsWith(' ') && line.endsWith(':')) {
@@ -219,6 +221,7 @@ class Program (val name: String,
     val program: List<Instruction>
 
     init {
+        prog.add(LabelInstr("____program_end"))
         prog.add(Instruction(Opcode.TERMINATE))
         prog.add(Instruction(Opcode.NOP))
         program = prog

compiler/src/prog8/stackvm/StackVm.kt

@@ -231,7 +231,7 @@ open class Instruction(val opcode: Opcode,
 
 class LabelInstr(val name: String) : Instruction(opcode = Opcode.NOP) {
     override fun toString(): String {
-        return "$name:"
+        return "\n$name:"
     }
 }