diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/ProgramGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/ProgramGen.kt index 545b86cac..5ca76d8a9 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/ProgramGen.kt +++ b/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) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/VariableAllocation.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/VariableAllocation.kt index e80f3b11c..6ccb863dc 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/VariableAllocation.kt +++ b/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() - .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 -> { diff --git a/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt b/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt index 575d0ac68..969a4ef9f 100644 --- a/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt +++ b/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) ) } } diff --git a/codeOptimizers/src/prog8/optimizer/UnusedCodeRemover.kt b/codeOptimizers/src/prog8/optimizer/UnusedCodeRemover.kt index 490417428..964181b34 100644 --- a/codeOptimizers/src/prog8/optimizer/UnusedCodeRemover.kt +++ b/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 { if(decl.type==VarDeclType.VAR) { - val forceOutput = "force_output" in decl.definingBlock.options() - if (!forceOutput && decl.origin==VarDeclOrigin.USERCODE && !decl.sharedWithAsm && !decl.definingBlock.isInLibrary) { + val block = decl.definingBlock + 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) { - errors.warn("removing unused variable '${decl.name}'", decl.position) + if(assignment.origin==AssignmentOrigin.VARINIT) { + 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) diff --git a/compiler/res/prog8lib/prog8_lib.p8 b/compiler/res/prog8lib/prog8_lib.p8 index 4e50d20b4..a238e1110 100644 --- a/compiler/res/prog8lib/prog8_lib.p8 +++ b/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 - uword @zp retval_interm_uw - word @zp retval_interm_w - ubyte @zp retval_interm_ub - byte @zp retval_interm_b - word retval_interm_w2 - byte retval_interm_b2 + ; NOTE: these variables can be used in the StatementReorderer and StatementOptimizer + uword @zp @shared retval_interm_uw + word @zp @shared retval_interm_w + ubyte @zp @shared retval_interm_ub + byte @zp @shared retval_interm_b + word @shared retval_interm_w2 + 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 diff --git a/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt b/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt index 6122bc029..25d9ef1dc 100644 --- a/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt +++ b/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) { - is Block -> { - val decls = allBlockVars[scope] ?: mutableSetOf() - decls.add(decl) - allBlockVars[scope] = decls - } - is Subroutine -> { - val decls = allSubroutineVars[scope] ?: mutableSetOf() - decls.add(decl) - allSubroutineVars[scope] = decls - } - else -> { - throw FatalAstException("var can only occur in subroutine or block scope") + if(decl.origin!=VarDeclOrigin.SUBROUTINEPARAM) { + when (scope) { + is Block -> { + val decls = allBlockVars[scope] ?: mutableSetOf() + decls.add(decl) + allBlockVars[scope] = decls + } + is Subroutine -> { + val decls = allSubroutineVars[scope] ?: mutableSetOf() + decls.add(decl) + allSubroutineVars[scope] = decls + } + 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) -> diff --git a/compiler/test/TestImportedModulesOrderAndOptions.kt b/compiler/test/TestImportedModulesOrderAndOptions.kt index eebf16846..2c1eda105 100644 --- a/compiler/test/TestImportedModulesOrderAndOptions.kt +++ b/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", diff --git a/compiler/test/codegeneration/TestAsmGenSymbols.kt b/compiler/test/codegeneration/TestAsmGenSymbols.kt index 2920b5b42..1fa4c18cc 100644 --- a/compiler/test/codegeneration/TestAsmGenSymbols.kt +++ b/compiler/test/codegeneration/TestAsmGenSymbols.kt @@ -79,13 +79,13 @@ class TestAsmGenSymbols: StringSpec({ override val subroutineMemvars: Map> 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(var2InSub.datatype, var2InSub.name, var2InSub.position, var2InSub) + IVariablesAndConsts.StaticSubroutineVariable(varInSub.datatype, varInSub.name, var2InSub.zeropage, varInSub.position, varInSub), + IVariablesAndConsts.StaticSubroutineVariable(var2InSub.datatype, var2InSub.name, var2InSub.zeropage, var2InSub.position, var2InSub) ) subroutineConsts = mutableMapOf() subroutineMemvars = mutableMapOf() diff --git a/compilerAst/src/prog8/ast/Program.kt b/compilerAst/src/prog8/ast/Program.kt index 830dc4f81..0e77c0112 100644 --- a/compilerAst/src/prog8/ast/Program.kt +++ b/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, List>() + + private val internedStringsReferenceCounts = mutableMapOf() fun internString(string: StringLiteralValue): List { // 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 { - val internedStringsBlock = modules - .first { it.name == internedStringsModuleName }.statements - .first { it is Block && it.name == internedStringsModuleName } as Block + val internedStringsBlock = modules + .first { it.name == internedStringsModuleName }.statements + .first { it is Block && it.name == internedStringsModuleName } as Block + + fun addNewInternedStringvar(string: StringLiteralValue): Pair, 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 existing = internedStringsUnique[key] - if (existing != null) - return existing - - val scopedName = getScopedName(string) - internedStringsUnique[key] = scopedName - return scopedName + val existingDecl = internedStringsBlock.statements.singleOrNull { + val declString = (it as VarDecl).value as StringLiteralValue + declString.encoding == string.encoding && declString.value == string.value + } + return if (existingDecl != null) { + existingDecl as VarDecl + internedStringsReferenceCounts[existingDecl] = internedStringsReferenceCounts.getValue(existingDecl)+1 + 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>() + override fun visit(identifier: IdentifierReference) { + if(identifier.wasStringLiteral(program)) + removals.add(identifier.nameInSource) + } + + fun removeStrings(modules: List) { + 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) + } + } + } + } + } diff --git a/compilerInterfaces/src/prog8/compilerinterface/IVariablesAndConsts.kt b/compilerInterfaces/src/prog8/compilerinterface/IVariablesAndConsts.kt index 1beb7fe47..d81511042 100644 --- a/compilerInterfaces/src/prog8/compilerinterface/IVariablesAndConsts.kt +++ b/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 StaticSubroutineVariable(val type: DataType, val name: String, 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 zp: ZeropageWish, val position: Position, val origVar: VarDecl) fun dump(memsizer: IMemSizer) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 1e7144c1c..e017f7c49 100644 --- a/docs/source/todo.rst +++ b/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 ^^^^^^^^^^^^^^ diff --git a/examples/test.p8 b/examples/test.p8 index 532dbdfd6..8730daed6 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -2,42 +2,43 @@ %zeropage basicsafe main { - ubyte mainglobal1 = 10 - ubyte mainglobal2 = 20 - ubyte mainglobal3 = 30 - ubyte mainglobal4 = 40 + ubyte @zp mainglobal1=10 sub start() { - ubyte startval1 = 100 - ubyte startval2 = 110 - ubyte startval3 = 120 - ubyte startval4 = 130 - - txt.print_ub(mainglobal1) - txt.nl() - 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] - } + c64.SETNAM(1, "$") + txt.print("1") + txt.print("12") + txt.print("test.") + txt.print("test.") +; foobar(2,1,1,1) +; foobar2(2,1,1,1) } - 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() { - uword @shared startval1 = 2002 - uword @shared mainglobal1 = 2002 - txt.print("foobar\n") +; asmsub foobar2(ubyte argument @A, uword a2 @R0, uword a3 @R1, uword a4 @R2) -> ubyte @A { +; %asm {{ +; lda #0 +; 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 } }