From a0cf1889a3ffa73ccee9f6f4ff622603477e7a44 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Thu, 17 Oct 2024 01:43:33 +0200 Subject: [PATCH] omit more redundant 0-initializations ("stz's") --- .../compiler/astprocessing/AstPreprocessor.kt | 2 +- .../astprocessing/StatementReorderer.kt | 63 +++++++++++++++++-- compiler/test/TestCompilerOnCharLit.kt | 14 ++++- compiler/test/TestOptimization.kt | 4 +- compiler/test/ast/TestIntermediateAst.kt | 55 ---------------- compiler/test/ast/TestSubroutines.kt | 10 +-- compiler/test/codegeneration/TestVariables.kt | 35 ++++++++++- .../src/prog8/ast/statements/AstStatements.kt | 6 -- docs/source/todo.rst | 5 +- examples/test.p8 | 39 +++--------- 10 files changed, 123 insertions(+), 110 deletions(-) diff --git a/compiler/src/prog8/compiler/astprocessing/AstPreprocessor.kt b/compiler/src/prog8/compiler/astprocessing/AstPreprocessor.kt index 43c2c383a..fd12790c3 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstPreprocessor.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstPreprocessor.kt @@ -172,7 +172,7 @@ class AstPreprocessor(val program: Program, override fun after(decl: VarDecl, parent: Node): Iterable { val nextAssignment = decl.nextSibling() as? Assignment if(nextAssignment!=null && nextAssignment.origin!=AssignmentOrigin.VARINIT) { - // check if it's a proper initializer assignment for the variable + // check if the following assignment initializes the variable if(decl.value==null && nextAssignment.target.identifier?.targetVarDecl(program)===decl) { if(!nextAssignment.value.referencesIdentifier(nextAssignment.target.identifier!!.nameInSource)) nextAssignment.origin = AssignmentOrigin.VARINIT diff --git a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt index 2fc9b67cb..026bf89c0 100644 --- a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt +++ b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt @@ -52,11 +52,8 @@ internal class StatementReorderer( // This allows you to restart the program and have the same starting values of the variables // So basically consider 'ubyte xx' as a short form for 'ubyte xx; xx=0' decl.value = null - if(decl.findInitializer(program)!=null) - return noModifications // an initializer assignment for a vardecl is already here - val nextFor = decl.nextSibling() as? ForLoop - val hasNextForWithThisLoopvar = nextFor?.loopVar?.nameInSource==listOf(decl.name) - if (!hasNextForWithThisLoopvar) { + val canskip = canSkipInitializationWith0(decl) + if (!canskip) { // Add assignment to initialize with zero // Note: for block-level vars, this will introduce assignments in the block scope. These have to be dealt with correctly later. val identifier = IdentifierReference(listOf(decl.name), decl.position) @@ -106,6 +103,62 @@ internal class StatementReorderer( return noModifications } + private fun canSkipInitializationWith0(decl: VarDecl): Boolean { + // if there is an assignment to the variable below it (regular assign, or For loop), + // and there is nothing important in between, + // we can skip the initialization. + val statements = (decl.parent as? IStatementContainer)?.statements ?: return false + val following = statements.asSequence().dropWhile { it!==decl }.drop(1) + for(stmt in following) { + when(stmt) { + is Assignment -> { + if (!stmt.isAugmentable) { + val assignTgt = stmt.target.identifier?.targetVarDecl(program) + if (assignTgt == decl) + return true + } + return false + } + + is ChainedAssignment -> { + var chained: ChainedAssignment? = stmt + while(chained!=null) { + val assignTgt = chained.target.identifier?.targetVarDecl(program) + if (assignTgt == decl) + return true + if(chained.nested is Assignment) { + if ((chained.nested as Assignment).target.identifier?.targetVarDecl(program) == decl) + return true + } + chained = chained.nested as? ChainedAssignment + } + } + + is ForLoop -> return stmt.loopVar.nameInSource == listOf(decl.name) + + is IFunctionCall, + is Jump, + is Break, + is BuiltinFunctionPlaceholder, + is ConditionalBranch, + is Continue, + is Return, + is Subroutine, + is InlineAssembly, + is Block, + is AnonymousScope, + is IfElse, + is RepeatLoop, + is UnrollLoop, + is UntilLoop, + is When, + is WhileLoop -> return false + else -> {} + } + } + return false + } + private fun directivesToTheTop(statements: MutableList) { val directives = statements.filterIsInstance().filter {it.directive in directivesToMove} statements.removeAll(directives.toSet()) diff --git a/compiler/test/TestCompilerOnCharLit.kt b/compiler/test/TestCompilerOnCharLit.kt index 3e6bac15d..64726b32c 100644 --- a/compiler/test/TestCompilerOnCharLit.kt +++ b/compiler/test/TestCompilerOnCharLit.kt @@ -6,8 +6,13 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.kotest.matchers.types.instanceOf import prog8.ast.IFunctionCall +import prog8.ast.IStatementContainer +import prog8.ast.Program import prog8.ast.expressions.IdentifierReference import prog8.ast.expressions.NumericLiteral +import prog8.ast.statements.Assignment +import prog8.ast.statements.AssignmentOrigin +import prog8.ast.statements.VarDecl import prog8.ast.statements.VarDeclType import prog8.code.core.DataType import prog8.code.core.Encoding @@ -22,6 +27,13 @@ import prog8tests.helpers.compileText */ class TestCompilerOnCharLit: FunSpec({ + fun findInitializer(vardecl: VarDecl, program: Program): Assignment? = + (vardecl.parent as IStatementContainer).statements + .asSequence() + .filterIsInstance() + .singleOrNull { it.origin== AssignmentOrigin.VARINIT && it.target.identifier?.targetVarDecl(program) === vardecl } + + test("testCharLitAsRomsubArg") { val platform = Cx16Target() val result = compileText(platform, false, """ @@ -70,7 +82,7 @@ class TestCompilerOnCharLit: FunSpec({ withClue("initializer value should have been moved to separate assignment"){ decl.value shouldBe null } - val assignInitialValue = decl.findInitializer(program)!! + val assignInitialValue = findInitializer(decl, program)!! assignInitialValue.target.identifier!!.nameInSource shouldBe listOf("ch") withClue("char literal should have been replaced by ubyte literal") { assignInitialValue.value shouldBe instanceOf() diff --git a/compiler/test/TestOptimization.kt b/compiler/test/TestOptimization.kt index a9080013e..e2e5b7eeb 100644 --- a/compiler/test/TestOptimization.kt +++ b/compiler/test/TestOptimization.kt @@ -504,9 +504,9 @@ main { xx += 6 */ val stmts = result.compilerAst.entrypoint.statements - stmts.size shouldBe 7 + stmts.size shouldBe 6 stmts.filterIsInstance().size shouldBe 3 - stmts.filterIsInstance().size shouldBe 4 + stmts.filterIsInstance().size shouldBe 3 } test("only substitue assignments with 0 after a =0 initializer if it is the same variable") { diff --git a/compiler/test/ast/TestIntermediateAst.kt b/compiler/test/ast/TestIntermediateAst.kt index 8912c721b..25c2091e4 100644 --- a/compiler/test/ast/TestIntermediateAst.kt +++ b/compiler/test/ast/TestIntermediateAst.kt @@ -15,61 +15,6 @@ import prog8tests.helpers.compileText class TestIntermediateAst: FunSpec({ - test("creation") { - val text=""" - %import textio - %import graphics - main { - sub start() { - bool cc - ubyte dd - ubyte[] array = [1,2,3] - cc = 11 in array - dd = sqrt(lsb(dd)) - } - } - """ - val target = C64Target() - val errors = ErrorReporterForTests() - val result = compileText(target, false, text, writeAssembly = false)!! - val ast = IntermediateAstMaker(result.compilerAst, errors).transform() - ast.name shouldBe result.compilerAst.name - ast.allBlocks().any() shouldBe true - val entry = ast.entrypoint() ?: fail("no main.start() found") - entry.children.size shouldBe 7 - entry.name shouldBe "start" - entry.scopedName shouldBe "main.start" - val blocks = ast.allBlocks().toList() - blocks.size shouldBeGreaterThan 1 - blocks[0].name shouldBe "main" - blocks[0].scopedName shouldBe "main" - - val ccInit = entry.children[3] as PtAssignment - ccInit.target.identifier?.name shouldBe "main.start.cc" - (ccInit.value as PtBool).value shouldBe false - val ddInit = entry.children[4] as PtAssignment - ddInit.target.identifier?.name shouldBe "main.start.dd" - (ddInit.value as PtNumber).number shouldBe 0.0 - - val ccdecl = entry.children[0] as PtVariable - ccdecl.name shouldBe "cc" - ccdecl.scopedName shouldBe "main.start.cc" - ccdecl.type shouldBe DataType.BOOL - val dddecl = entry.children[1] as PtVariable - dddecl.name shouldBe "dd" - dddecl.scopedName shouldBe "main.start.dd" - dddecl.type shouldBe DataType.UBYTE - - val arraydecl = entry.children[2] as IPtVariable - arraydecl.name shouldBe "array" - arraydecl.type shouldBe DataType.ARRAY_UB - - val ccAssignV = (entry.children[5] as PtAssignment).value - ccAssignV shouldBe instanceOf() - val ddAssignV = (entry.children[6] as PtAssignment).value - ddAssignV shouldBe instanceOf() - } - test("isSame on binaryExpressions") { val expr1 = PtBinaryExpression("/", DataType.UBYTE, Position.DUMMY) expr1.add(PtNumber(DataType.UBYTE, 1.0, Position.DUMMY)) diff --git a/compiler/test/ast/TestSubroutines.kt b/compiler/test/ast/TestSubroutines.kt index a80971c83..dfe7ae7d1 100644 --- a/compiler/test/ast/TestSubroutines.kt +++ b/compiler/test/ast/TestSubroutines.kt @@ -194,11 +194,11 @@ main { val result = compileText(Cx16Target(), false, src, errors, true)!! errors.errors.size shouldBe 0 val start = result.codegenAst!!.entrypoint()!! - start.children.size shouldBe 10 - val a1_1 = start.children[5] as PtAssignment - val a1_2 = start.children[6] as PtAssignment - val a1_3 = start.children[7] as PtAssignment - val a1_4 = start.children[8] as PtAssignment + start.children.size shouldBe 9 + val a1_1 = start.children[4] as PtAssignment + val a1_2 = start.children[5] as PtAssignment + val a1_3 = start.children[6] as PtAssignment + val a1_4 = start.children[7] as PtAssignment a1_1.multiTarget shouldBe true a1_2.multiTarget shouldBe true a1_3.multiTarget shouldBe true diff --git a/compiler/test/codegeneration/TestVariables.kt b/compiler/test/codegeneration/TestVariables.kt index a97c01ee7..dae5c75dd 100644 --- a/compiler/test/codegeneration/TestVariables.kt +++ b/compiler/test/codegeneration/TestVariables.kt @@ -4,9 +4,11 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe import io.kotest.matchers.string.shouldContain +import io.kotest.matchers.types.instanceOf import prog8.ast.statements.Assignment import prog8.ast.statements.AssignmentOrigin -import prog8.code.ast.PtAssignment +import prog8.ast.statements.ForLoop +import prog8.ast.statements.VarDecl import prog8.code.target.C64Target import prog8tests.helpers.ErrorReporterForTests import prog8tests.helpers.compileText @@ -189,4 +191,35 @@ main { assigns[5].value.constValue(result) shouldBe null assigns[6].value.constValue(result) shouldBe null } + + test("not inserting redundant 0-initializations") { + val src=""" +main { + sub start() { + ubyte v0 + ubyte v1 + ubyte v2 + ubyte v3 + v0 = v1 = v2 = 99 + for v3 in 10 to 20 { + cx16.r0L++ + } + } +}""" + val result = compileText(C64Target(), false, src, writeAssembly = false)!!.compilerAst + val st = result.entrypoint.statements + st.size shouldBe 8 + st[0] shouldBe instanceOf() + st[1] shouldBe instanceOf() + st[2] shouldBe instanceOf() + st[3] shouldBe instanceOf() + st[4] shouldBe instanceOf() + st[5] shouldBe instanceOf() + st[6] shouldBe instanceOf() + st[7] shouldBe instanceOf() + + (st[4] as Assignment).target.identifier?.nameInSource shouldBe listOf("v2") + (st[5] as Assignment).target.identifier?.nameInSource shouldBe listOf("v1") + (st[6] as Assignment).target.identifier?.nameInSource shouldBe listOf("v0") + } }) diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index 85c0b6920..11f021302 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt @@ -290,12 +290,6 @@ class VarDecl(val type: VarDeclType, return copy } - fun findInitializer(program: Program): Assignment? = - (parent as IStatementContainer).statements - .asSequence() - .filterIsInstance() - .singleOrNull { it.origin==AssignmentOrigin.VARINIT && it.target.identifier?.targetVarDecl(program) === this } - override fun referencesIdentifier(nameInSource: List): Boolean = value?.referencesIdentifier(nameInSource)==true || this.arraysize?.referencesIdentifier(nameInSource)==true diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 0033a592a..75df00d43 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -9,15 +9,13 @@ Maybe this routine can be made more intelligent. See usesOtherRegistersWhileEva Future Things and Ideas ^^^^^^^^^^^^^^^^^^^^^^^ -- improve detection that a variable is not read before being written so that initializing it to zero can be omitted - (only happens now if a vardecl is immediately followed by a for loop for instance) BUT this may break stuff if the variable is read from a function call for instance in between? - Improve the SublimeText syntax file for prog8, you can also install this for 'bat': https://github.com/sharkdp/bat?tab=readme-ov-file#adding-new-syntaxes--language-definitions - Can we support signed % (remainder) somehow? - Don't add "random" rts to %asm blocks but instead give a warning about it? (but this breaks existing behavior that others already depend on... command line switch? block directive?) - IR: implement missing operators in AssignmentGen (array shifts etc) - instead of copy-pasting inline asmsubs, make them into a 64tass macro and use that instead. that will allow them to be reused from custom user written assembly code as well. -- Multidimensional arrays and chained indexing, purely as syntactic sugar over regular arrays. +- Multidimensional arrays and chained indexing, purely as syntactic sugar over regular arrays. Probaby only useful if we have typed pointers. - make a form of "manual generics" possible like: varsub routine(T arg)->T where T is expanded to a specific type (this is already done hardcoded for several of the builtin functions) @@ -48,6 +46,7 @@ Future Things and Ideas But all library code written in asm uses .proc already..... (textual search/replace when writing the actual asm?) Once new codegen is written that is based on the IR, this point is mostly moot anyway as that will have its own dead code removal on the IR level. - Zig-like try-based error handling where the V flag could indicate error condition? and/or BRK to jump into monitor on failure? (has to set BRK vector for that) But the V flag is also set on certain normal instructions +- Zig-like defer to clean up stuff when leaving the scope. But as we don't have closures, it's more limited. Still useful? Libraries: diff --git a/examples/test.p8 b/examples/test.p8 index b8437f7b8..2f2975c92 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -3,37 +3,14 @@ %zeropage basicsafe main { - - bool[] barray = [true, false, true, false] - uword[] warray = [&value1, &barray, &value5, 4242] - - ubyte @shared integer = 99 - bool @shared value1 - bool @shared value2 = barray[2] ; should be const! - bool @shared value3 = true - bool @shared value4 = false - bool @shared value5 = barray[cx16.r0L] ; cannot be const - uword @shared value6 = warray[3] ; should be const! - uword @shared value7 = warray[2] ; cannot be const - sub start() { - txt.print_ub(integer) - integer++ - txt.spc() - txt.print_ub(integer) - txt.nl() - - txt.print_bool(value1) - txt.spc() - txt.print_bool(value2) - txt.spc() - txt.print_bool(value3) - txt.spc() - txt.print_bool(value4) - txt.spc() - txt.print_bool(value5) - txt.spc() - txt.print_uw(value6) - txt.nl() + ubyte v0 + ubyte v1 + ubyte v2 + ubyte v3 + v0 = v1 = v2 = 99 + for v3 in 10 to 20 { + cx16.r0L++ + } } }