more rewrite variable allocation

codeGenCpu6502/src/prog8/codegen/cpu6502/ProgramGen.kt

@@ -218,8 +218,6 @@ internal class ProgramGen(
         }
 
         asmgen.out("")
-        asmgen.outputSourceLine(sub)
-
 
         if(sub.isAsmSubroutine) {
             if(sub.asmAddress!=null)

codeGenCpu6502/src/prog8/codegen/cpu6502/VariableAllocation.kt

@@ -7,6 +7,7 @@ import prog8.ast.base.DataType
 import prog8.ast.base.VarDeclType
 import prog8.ast.expressions.StringLiteralValue
 import prog8.ast.statements.VarDecl
+import prog8.ast.statements.VarDeclOrigin
 import prog8.ast.statements.ZeropageWish
 import prog8.compilerinterface.*
 
@@ -19,10 +20,12 @@ internal class VariableAllocation(val vars: IVariablesAndConsts, val errors: IEr
             return
 
         val zeropage = options.compTarget.machine.zeropage
+//        val allVariables = vars.blockVars.asSequence().flatMap { it.value }.map {it.origVar to it.origVar.scopedName} +
+//                           vars.subroutineVars.asSequence().flatMap { it.value }.map {it.origVar to it.origVar.scopedName}
         val allVariables = callGraph.allIdentifiers.asSequence()
             .map { it.value }
             .filterIsInstance<VarDecl>()
-            .filter { it.type== VarDeclType.VAR }
+            .filter { it.type== VarDeclType.VAR && it.origin!=VarDeclOrigin.SUBROUTINEPARAM }
             .toSet()
             .map { it to it.scopedName }
         val varsRequiringZp = allVariables.filter { it.first.zeropage== ZeropageWish.REQUIRE_ZEROPAGE }
@@ -30,6 +33,33 @@ internal class VariableAllocation(val vars: IVariablesAndConsts, val errors: IEr
             .filter { it.first.zeropage== ZeropageWish.PREFER_ZEROPAGE }
             .sortedBy { options.compTarget.memorySize(it.first.datatype) }      // allocate the smallest DT first
 
+
+        val allVarsFromCallgraph = allVariables.map { it.first.name to it.first.position }.toSet()
+        val altAllVars = (vars.blockVars.flatMap { it.value }.map {it.name to it.position} + vars.subroutineVars.flatMap { it.value }.map {it.name to it.position}).toSet()
+        val extraVarsInCallgraph = allVarsFromCallgraph - altAllVars
+        val extraVarsInNew = altAllVars - allVarsFromCallgraph
+
+        if(extraVarsInCallgraph.any() || extraVarsInNew.any()) {
+            println("EXTRA VARS IN CALLGRAPH: ${extraVarsInCallgraph.size}")
+            extraVarsInCallgraph.forEach {
+                println("  $it")
+            }
+            println("EXTRA VARS IN VARIABLESOBJ: ${extraVarsInNew.size}")
+            extraVarsInNew.forEach {
+                println("  $it")
+            }
+            // TODO("fix differences")
+        }
+
+        val altTotalNumVars = vars.blockVars.flatMap { it.value }.size + vars.subroutineVars.flatMap { it.value }.size
+        val altVarsRequiringZpBlocks = vars.blockVars.flatMap { it.value }.filter { it.zp==ZeropageWish.REQUIRE_ZEROPAGE }
+        val altVarsRequiringZpSubs = vars.subroutineVars.flatMap { it.value }.filter { it.zp==ZeropageWish.REQUIRE_ZEROPAGE }
+        val altTotalZpVars = altVarsRequiringZpBlocks.size + altVarsRequiringZpSubs.size
+        val altVarsPreferringZpBlocks = vars.blockVars.flatMap { it.value }.filter { it.zp==ZeropageWish.PREFER_ZEROPAGE }
+        val altVarsPreferringZpSubs = vars.subroutineVars.flatMap { it.value }.filter { it.zp==ZeropageWish.PREFER_ZEROPAGE }
+        val altTotalPrefZpVars = altVarsPreferringZpBlocks.size + altVarsPreferringZpSubs.size
+
+
         for ((vardecl, scopedname) in varsRequiringZp) {
             val numElements: Int? = when(vardecl.datatype) {
                 DataType.STR -> {

codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt

@@ -89,7 +89,11 @@ class StatementOptimizer(private val program: Program,
                             mutableListOf(NumericLiteralValue(DataType.UBYTE, firstCharEncoded.toDouble(), pos)),
                             functionCallStatement.void, pos
                         )
-                        return listOf(IAstModification.ReplaceNode(functionCallStatement, chrout, parent))
+                        val stringDecl = string.parent as VarDecl
+                        return listOf(
+                            IAstModification.ReplaceNode(functionCallStatement, chrout, parent),
+                            IAstModification.Remove(stringDecl, stringDecl.parent as IStatementContainer)
+                        )
                     } else if (string.value.length == 2) {
                         val firstTwoCharsEncoded = compTarget.encodeString(string.value.take(2), string.encoding)
                         val chrout1 = FunctionCallStatement(
@@ -102,9 +106,11 @@ class StatementOptimizer(private val program: Program,
                             mutableListOf(NumericLiteralValue(DataType.UBYTE, firstTwoCharsEncoded[1].toDouble(), pos)),
                             functionCallStatement.void, pos
                         )
+                        val stringDecl = string.parent as VarDecl
                         return listOf(
                             IAstModification.InsertBefore(functionCallStatement, chrout1, parent as IStatementContainer),
-                            IAstModification.ReplaceNode(functionCallStatement, chrout2, parent)
+                            IAstModification.ReplaceNode(functionCallStatement, chrout2, parent),
+                            IAstModification.Remove(stringDecl, stringDecl.parent as IStatementContainer)
                         )
                     }
                 }

codeOptimizers/src/prog8/optimizer/UnusedCodeRemover.kt

@@ -69,6 +69,7 @@ class UnusedCodeRemover(private val program: Program,
             if(callgraph.unused(block)) {
                 if(block.statements.any{ it !is VarDecl || it.type==VarDeclType.VAR})
                     errors.warn("removing unused block '${block.name}'", block.position)
+                program.removeInternedStringsFromRemovedBlock(block)
                 return listOf(IAstModification.Remove(block, parent as IStatementContainer))
             }
         }
@@ -92,6 +93,7 @@ class UnusedCodeRemover(private val program: Program,
                 }
                 if(!subroutine.definingModule.isLibrary)
                     errors.warn("removing unused subroutine '${subroutine.name}'", subroutine.position)
+                program.removeInternedStringsFromRemovedSubroutine(subroutine)
                 return listOf(IAstModification.Remove(subroutine, parent as IStatementContainer))
             }
         }
@@ -101,11 +103,13 @@ class UnusedCodeRemover(private val program: Program,
 
     override fun after(decl: VarDecl, parent: Node): Iterable<IAstModification> {
         if(decl.type==VarDeclType.VAR) {
-            val forceOutput = "force_output" in decl.definingBlock.options()
+            val block = decl.definingBlock
-            if (!forceOutput && decl.origin==VarDeclOrigin.USERCODE && !decl.sharedWithAsm && !decl.definingBlock.isInLibrary) {
+            val forceOutput = "force_output" in block.options()
+            if (!forceOutput && decl.origin==VarDeclOrigin.USERCODE && !decl.sharedWithAsm && !block.isInLibrary) {         // TODO remove check on block.isInLibrary, but this now breaks some programs
                 val usages = callgraph.usages(decl)
                 if (usages.isEmpty()) {
-                    errors.warn("removing unused variable '${decl.name}'", decl.position)
+                    if(!decl.definingModule.isLibrary)
+                        errors.warn("removing unused variable '${decl.name}'", decl.position)
                     return listOf(IAstModification.Remove(decl, parent as IStatementContainer))
                 }
                 else {
@@ -113,8 +117,9 @@ class UnusedCodeRemover(private val program: Program,
                         val singleUse = usages[0].parent
                         if(singleUse is AssignTarget) {
                             val assignment = singleUse.parent as Assignment
-                            if(assignment.value !is IFunctionCall) {
+                            if(assignment.origin==AssignmentOrigin.VARINIT) {
-                                errors.warn("removing unused variable '${decl.name}'", decl.position)
+                                if(!decl.definingModule.isLibrary)
+                                    errors.warn("removing unused variable '${decl.name}'", decl.position)
                                 return listOf(
                                     IAstModification.Remove(decl, parent as IStatementContainer),
                                     IAstModification.Remove(assignment, assignment.parent as IStatementContainer)

compiler/res/prog8lib/prog8_lib.p8

@@ -7,13 +7,13 @@ prog8_lib {
 	%asminclude "library:prog8_funcs.asm"
 
 	; to store intermediary expression results for return values:
-	; NOTE: these variables are used in the StatementReorderer and StatementOptimizer
+	; NOTE: these variables can be used in the StatementReorderer and StatementOptimizer
-	uword @zp retval_interm_uw
+	uword @zp @shared retval_interm_uw
-	word @zp retval_interm_w
+	word @zp @shared retval_interm_w
-	ubyte @zp retval_interm_ub
+	ubyte @zp @shared retval_interm_ub
-	byte @zp retval_interm_b
+	byte @zp @shared retval_interm_b
-	word retval_interm_w2
+	word @shared retval_interm_w2
-	byte retval_interm_b2
+	byte @shared retval_interm_b2
 
         ; prog8 "hooks" to be able to access the temporary scratch variables
         ; YOU SHOULD NOT USE THESE IN USER CODE - THESE ARE MEANT FOR INTERNAL COMPILER USE

compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt

@@ -71,19 +71,21 @@ internal class BeforeAsmAstChanger(val program: Program, private val options: Co
         val scope=decl.definingScope
         when (decl.type) {
             VarDeclType.VAR -> {
-                when(scope) {
+                if(decl.origin!=VarDeclOrigin.SUBROUTINEPARAM) {
-                    is Block -> {
+                    when (scope) {
-                        val decls = allBlockVars[scope] ?: mutableSetOf()
+                        is Block -> {
-                        decls.add(decl)
+                            val decls = allBlockVars[scope] ?: mutableSetOf()
-                        allBlockVars[scope] = decls
+                            decls.add(decl)
-                    }
+                            allBlockVars[scope] = decls
-                    is Subroutine -> {
+                        }
-                        val decls = allSubroutineVars[scope] ?: mutableSetOf()
+                        is Subroutine -> {
-                        decls.add(decl)
+                            val decls = allSubroutineVars[scope] ?: mutableSetOf()
-                        allSubroutineVars[scope] = decls
+                            decls.add(decl)
-                    }
+                            allSubroutineVars[scope] = decls
-                    else -> {
+                        }
-                        throw FatalAstException("var can only occur in subroutine or block scope")
+                        else -> {
+                            throw FatalAstException("var can only occur in subroutine or block scope")
+                        }
                     }
                 }
             }
@@ -469,7 +471,7 @@ internal class VariablesAndConsts (
         astBlockVars.forEach { (block, decls) ->
             val vars = bv.getValue(block)
             vars.addAll(decls.map {
-                IVariablesAndConsts.StaticBlockVariable(it.datatype, it.name, it.value, it.position, it)
+                IVariablesAndConsts.StaticBlockVariable(it.datatype, it.name, it.value, it.zeropage, it.position, it)
             })
         }
         astBlockConsts.forEach { (block, decls) ->
@@ -497,7 +499,7 @@ internal class VariablesAndConsts (
         astSubroutineVars.forEach { (sub, decls) ->
             val vars = sv.getValue(sub)
             vars.addAll(decls.map {
-                IVariablesAndConsts.StaticSubroutineVariable(it.datatype, it.name, it.position, it)
+                IVariablesAndConsts.StaticSubroutineVariable(it.datatype, it.name, it.zeropage, it.position, it)
             })
         }
         astSubroutineConsts.forEach { (sub, decls) ->

compiler/test/TestImportedModulesOrderAndOptions.kt

@@ -36,7 +36,7 @@ main {
         }
         withClue("module order in parse tree") {
             moduleNames.drop(1) shouldBe listOf(
-                "prog8_interned_strings",
+                internedStringsModuleName,
                 "textio",
                 "syslib",
                 "conv",

compiler/test/codegeneration/TestAsmGenSymbols.kt

@@ -79,13 +79,13 @@ class TestAsmGenSymbols: StringSpec({
             override val subroutineMemvars: Map<Subroutine, Set<IVariablesAndConsts.MemoryMappedVariable>>
             init {
                 blockVars = mutableMapOf()
-                blockVars[block] = mutableSetOf(IVariablesAndConsts.StaticBlockVariable(varInBlock.datatype, varInBlock.name, varInBlock.value, varInBlock.position, varInBlock))
+                blockVars[block] = mutableSetOf(IVariablesAndConsts.StaticBlockVariable(varInBlock.datatype, varInBlock.name, varInBlock.value, varInBlock.zeropage, varInBlock.position, varInBlock))
                 blockConsts = mutableMapOf()
                 blockMemvars = mutableMapOf()
                 subroutineVars = mutableMapOf()
                 subroutineVars[subroutine] = mutableSetOf(
-                    IVariablesAndConsts.StaticSubroutineVariable(varInSub.datatype, varInSub.name, varInSub.position, varInSub),
+                    IVariablesAndConsts.StaticSubroutineVariable(varInSub.datatype, varInSub.name, var2InSub.zeropage, varInSub.position, varInSub),
-                    IVariablesAndConsts.StaticSubroutineVariable(var2InSub.datatype, var2InSub.name, var2InSub.position, var2InSub)
+                    IVariablesAndConsts.StaticSubroutineVariable(var2InSub.datatype, var2InSub.name, var2InSub.zeropage, var2InSub.position, var2InSub)
                 )
                 subroutineConsts = mutableMapOf()
                 subroutineMemvars = mutableMapOf()

compilerAst/src/prog8/ast/Program.kt

@@ -4,9 +4,10 @@ import prog8.ast.base.DataType
 import prog8.ast.base.FatalAstException
 import prog8.ast.base.Position
 import prog8.ast.base.VarDeclType
+import prog8.ast.expressions.IdentifierReference
 import prog8.ast.expressions.StringLiteralValue
 import prog8.ast.statements.*
-import prog8.compilerinterface.Encoding
+import prog8.ast.walk.IAstVisitor
 import prog8.compilerinterface.IMemSizer
 import prog8.compilerinterface.IStringEncoding
 import prog8.parser.SourceCode
@@ -74,7 +75,8 @@ class Program(val name: String,
         get() = toplevelModule.loadAddress
 
     var actualLoadAddress = 0u
-    private val internedStringsUnique = mutableMapOf<Pair<String, Encoding>, List<String>>()
+
+    private val internedStringsReferenceCounts = mutableMapOf<VarDecl, Int>()
 
     fun internString(string: StringLiteralValue): List<String> {
         // Move a string literal into the internal, deduplicated, string pool
@@ -85,10 +87,11 @@ class Program(val name: String,
             throw FatalAstException("cannot intern a string literal that's part of a vardecl")
         }
 
-        fun getScopedName(string: StringLiteralValue): List<String> {
+        val internedStringsBlock = modules
-            val internedStringsBlock = modules
+            .first { it.name == internedStringsModuleName }.statements
-                .first { it.name == internedStringsModuleName }.statements
+            .first { it is Block && it.name == internedStringsModuleName } as Block
-                .first { it is Block && it.name == internedStringsModuleName } as Block
+
+        fun addNewInternedStringvar(string: StringLiteralValue): Pair<List<String>, VarDecl> {
             val varName = "string_${internedStringsBlock.statements.size}"
             val decl = VarDecl(
                 VarDeclType.VAR, VarDeclOrigin.STRINGLITERAL, DataType.STR, ZeropageWish.NOT_IN_ZEROPAGE, null, varName, string,
@@ -96,16 +99,58 @@ class Program(val name: String,
             )
             internedStringsBlock.statements.add(decl)
             decl.linkParents(internedStringsBlock)
-            return listOf(internedStringsModuleName, decl.name)
+            return Pair(listOf(internedStringsModuleName, decl.name), decl)
         }
 
-        val key = Pair(string.value, string.encoding)
+        val existingDecl = internedStringsBlock.statements.singleOrNull {
-        val existing = internedStringsUnique[key]
+            val declString = (it as VarDecl).value as StringLiteralValue
-        if (existing != null)
+            declString.encoding == string.encoding && declString.value == string.value
-            return existing
+        }
-
+        return if (existingDecl != null) {
-        val scopedName = getScopedName(string)
+            existingDecl as VarDecl
-        internedStringsUnique[key] = scopedName
+            internedStringsReferenceCounts[existingDecl] = internedStringsReferenceCounts.getValue(existingDecl)+1
-        return scopedName
+            existingDecl.scopedName
+        }
+        else {
+            val (newName, newDecl) = addNewInternedStringvar(string)
+            internedStringsReferenceCounts[newDecl] = 1
+            newName
+        }
     }
+
+    fun removeInternedStringsFromRemovedSubroutine(sub: Subroutine) {
+        val s = StringSearch(this)
+        sub.accept(s)
+        s.removeStrings(modules)
+    }
+
+    fun removeInternedStringsFromRemovedBlock(block: Block) {
+        val s = StringSearch(this)
+        block.accept(s)
+        s.removeStrings(modules)
+    }
+
+    private class StringSearch(val program: Program): IAstVisitor {
+        val removals = mutableListOf<List<String>>()
+        override fun visit(identifier: IdentifierReference) {
+            if(identifier.wasStringLiteral(program))
+                removals.add(identifier.nameInSource)
+        }
+
+        fun removeStrings(modules: List<Module>) {
+            if(removals.isNotEmpty()) {
+                val internedStringsBlock = modules
+                    .first { it.name == internedStringsModuleName }.statements
+                    .first { it is Block && it.name == internedStringsModuleName } as Block
+                removals.forEach { scopedname ->
+                    val decl = internedStringsBlock.statements.single { decl -> (decl as VarDecl).scopedName == scopedname } as VarDecl
+                    val numRefs = program.internedStringsReferenceCounts.getValue(decl) - 1
+                    program.internedStringsReferenceCounts[decl] = numRefs
+                    if(numRefs==0)
+                        internedStringsBlock.statements.remove(decl)
+                }
+            }
+        }
+    }
+
 }

compilerInterfaces/src/prog8/compilerinterface/IVariablesAndConsts.kt

@@ -6,6 +6,7 @@ import prog8.ast.expressions.Expression
 import prog8.ast.statements.Block
 import prog8.ast.statements.Subroutine
 import prog8.ast.statements.VarDecl
+import prog8.ast.statements.ZeropageWish
 
 /**
  * Experimental attempt for:
@@ -16,8 +17,8 @@ interface IVariablesAndConsts {
     data class ConstantNumberSymbol(val type: DataType, val name: String, val value: Double, val position: Position)
     data class MemoryMappedVariable(val type: DataType, val name: String, val address: UInt, val position: Position)
     // TODO should get rid of origVar altogether in the following two:
-    data class StaticBlockVariable(val type: DataType, val name: String, val initialValue: Expression?, val position: Position, val origVar: VarDecl)
+    data class StaticBlockVariable(val type: DataType, val name: String, val initialValue: Expression?, val zp: ZeropageWish, val position: Position, val origVar: VarDecl)
-    data class StaticSubroutineVariable(val type: DataType, val name: String, val position: Position, val origVar: VarDecl)
+    data class StaticSubroutineVariable(val type: DataType, val name: String, val zp: ZeropageWish, val position: Position, val origVar: VarDecl)
 
     fun dump(memsizer: IMemSizer)
 

docs/source/todo.rst

@@ -3,7 +3,8 @@ TODO
 
 For next release
 ^^^^^^^^^^^^^^^^
-...
+- (newvaralloc) UnusedCodeRemover after(decl: VarDecl): fix that vars defined in a library can also safely be removed if unused. Currently this breaks programs such as textelite (due to diskio.save().end_address ?)
+
 
 Need help with
 ^^^^^^^^^^^^^^

examples/test.p8

@@ -2,42 +2,43 @@
 %zeropage basicsafe
 
 main {
-    ubyte mainglobal1 = 10
+    ubyte @zp mainglobal1=10
-    ubyte mainglobal2 = 20
-    ubyte mainglobal3 = 30
-    ubyte mainglobal4 = 40
 
     sub start() {
-        ubyte startval1 = 100
+        c64.SETNAM(1, "$")
-        ubyte startval2 = 110
+        txt.print("1")
-        ubyte startval3 = 120
+        txt.print("12")
-        ubyte startval4 = 130
+        txt.print("test.")
-
+        txt.print("test.")
-        txt.print_ub(mainglobal1)
+;        foobar(2,1,1,1)
-        txt.nl()
+;        foobar2(2,1,1,1)
-        txt.print_ub(startval1)
-        txt.nl()
-        derp()
-        derp()
-        foobar()
-        startval1++
-        mainglobal1++
-        start2()
-
-        sub start2() {
-            uword @shared startval1 = 2002
-            ubyte[2] @shared barr
-            uword[2] @shared warr
-            uword[] @shared warr2 = [1,2]
-        }
     }
 
-    asmsub derp() {
+    sub unusedsubroutine() {
+        c64.SETNAM(1, "$")      ; TODO fix don't remove this interned string because referenced in start()
+        txt.print("this string should be removed from the pool")
+        txt.print("this string should be removed from the pool")
+        txt.print("this string should be removed from the pool")
     }
 
-    sub foobar() {
+;    asmsub foobar2(ubyte argument @A, uword a2 @R0, uword a3 @R1, uword a4 @R2) -> ubyte @A {
-        uword @shared startval1 = 2002
+;        %asm {{
-        uword @shared mainglobal1 = 2002
+;            lda  #0
-        txt.print("foobar\n")
+;            rts
+;        }}
+;    }
+;    sub foobar(ubyte argument, uword a2, uword a3, uword a4) -> ubyte {
+;        argument++
+;        return lsb(a2)
+;    }
+}
+
+foobar {
+    str name = "don't remove this one"
+
+    sub unusedinfoobar() {
+        name = "should be removed"                  ; TODO remove from interned strings because subroutine got removed
+        txt.print(name)
+        txt.print("should also be removed2")        ; TODO remove from interned strings because subroutine got removed
     }
 }