From 53ac11983b11eae44738ddf5c3886ce47c471e36 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Thu, 11 Nov 2021 03:03:21 +0100 Subject: [PATCH] better unused variable removal --- .../src/prog8/optimizer/UnusedCodeRemover.kt | 23 ++++++++++++- compiler/test/TestCompilerOnRanges.kt | 2 +- compiler/test/TestOptimization.kt | 33 ++++++++++++++++--- .../src/prog8/compilerinterface/CallGraph.kt | 12 ++++--- examples/test.p8 | 3 -- 5 files changed, 60 insertions(+), 13 deletions(-) diff --git a/codeOptimizers/src/prog8/optimizer/UnusedCodeRemover.kt b/codeOptimizers/src/prog8/optimizer/UnusedCodeRemover.kt index 9fd0cf848..2f03c439d 100644 --- a/codeOptimizers/src/prog8/optimizer/UnusedCodeRemover.kt +++ b/codeOptimizers/src/prog8/optimizer/UnusedCodeRemover.kt @@ -107,9 +107,30 @@ class UnusedCodeRemover(private val program: Program, if(decl.type==VarDeclType.VAR) { val forceOutput = "force_output" in decl.definingBlock.options() if (!forceOutput && !decl.autogeneratedDontRemove && !decl.sharedWithAsm && !decl.definingBlock.isInLibrary) { - if (callgraph.unused(decl)) { + val usages = callgraph.usages(decl) + if (usages.isEmpty()) { errors.warn("removing unused variable '${decl.name}'", decl.position) return listOf(IAstModification.Remove(decl, parent as IStatementContainer)) + } else { + // if all usages are just an assignment to this vardecl + // and it is in regular RAM, then remove the var as well including all assignments + val assignTargets = usages.mapNotNull { + if(it.parent is AssignTarget) + it.parent as AssignTarget + else if(it.parent.parent is AssignTarget) + it.parent.parent as AssignTarget + else null + }.filter { + it.isInRegularRAMof(compTarget.machine) + } + if(assignTargets.size==usages.size) { + errors.warn("removing unused variable '${decl.name}'", decl.position) + return assignTargets.map { it.parent to it.definingScope}.toSet().map { + IAstModification.Remove(it.first, it.second) + } + listOf( + IAstModification.Remove(decl, parent as IStatementContainer) + ) + } } } } diff --git a/compiler/test/TestCompilerOnRanges.kt b/compiler/test/TestCompilerOnRanges.kt index 0bf0bf89f..83585abfd 100644 --- a/compiler/test/TestCompilerOnRanges.kt +++ b/compiler/test/TestCompilerOnRanges.kt @@ -29,7 +29,7 @@ class TestCompilerOnRanges: FunSpec({ test("testUByteArrayInitializerWithRange_char_to_char") { val platform = Cx16Target - val result = compileText(platform, true, """ + val result = compileText(platform, false, """ main { sub start() { ubyte[] cs = @'a' to 'z' ; values are computed at compile time diff --git a/compiler/test/TestOptimization.kt b/compiler/test/TestOptimization.kt index 59ff37b25..0635c3976 100644 --- a/compiler/test/TestOptimization.kt +++ b/compiler/test/TestOptimization.kt @@ -93,10 +93,10 @@ class TestOptimization: FunSpec({ main { sub start() { const ubyte TEST = 10 - byte x1 = TEST as byte + 1 - byte x2 = 1 + TEST as byte - ubyte y1 = TEST + 1 as byte - ubyte y2 = 1 as byte + TEST + byte @shared x1 = TEST as byte + 1 + byte @shared x2 = 1 + TEST as byte + ubyte @shared y1 = TEST + 1 as byte + ubyte @shared y2 = 1 as byte + TEST } } """ @@ -215,4 +215,29 @@ class TestOptimization: FunSpec({ val asm = generateAssembly(result1.program) asm.valid shouldBe true } + + test("unused variable removal") { + val src=""" + main { + sub start() { + ubyte unused + ubyte @shared unused_but_shared ; this one should remain + ubyte usedvar_only_written + usedvar_only_written=2 + usedvar_only_written++ + ubyte usedvar ; and this one too + usedvar = msb(usedvar) + } + } + """ + val result = compileText(C64Target, optimize=true, src, writeAssembly=false).assertSuccess() + result.program.entrypoint.statements.size shouldBe 4 // unused_but_shared decl, unused_but_shared=0, usedvar decl, usedvar assign + val (decl, assign, decl2, assign2) = result.program.entrypoint.statements + decl shouldBe instanceOf() + (decl as VarDecl).name shouldBe "unused_but_shared" + assign shouldBe instanceOf() + decl2 shouldBe instanceOf() + (decl2 as VarDecl).name shouldBe "usedvar" + assign2 shouldBe instanceOf() + } }) diff --git a/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt b/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt index 6ff91b39d..fbf614427 100644 --- a/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt +++ b/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt @@ -173,14 +173,18 @@ class CallGraph(private val program: Program) : IAstVisitor { } fun unused(decl: VarDecl): Boolean { + // Don't check assembly just for occurrences of variables, if they're not used in prog8 itself, just kill them + return usages(decl).isEmpty() + } + + fun usages(decl: VarDecl): List { if(decl.type!=VarDeclType.VAR || decl.autogeneratedDontRemove || decl.sharedWithAsm) - return false + return emptyList() if(decl.definingBlock !in usedBlocks) - return false + return emptyList() - val allReferencedVardecls = allIdentifiersAndTargets.filter { it.value is VarDecl }.map { it.value }.toSet() - return decl !in allReferencedVardecls // Don't check assembly just for occurrences of variables, if they're not used in prog8 itself, just kill them + return allIdentifiersAndTargets.filter { decl===it.value }.map{ it.key.first } } private fun nameInAssemblyCode(name: String) = allAssemblyNodes.any { it.assembly.contains(name) } diff --git a/examples/test.p8 b/examples/test.p8 index 9458b4adb..72ed36f36 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -5,9 +5,6 @@ main { sub start() { - ubyte unused ; TODO FIX : why is this not removed as an unused variable? - ubyte @shared unused2 - ubyte bb uword ww ww = not bb or not ww ; TODO WHY DOES THIS USE STACK EVAL