diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/ProgramAndVarsGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/ProgramAndVarsGen.kt index c776628df..99dfaf806 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/ProgramAndVarsGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/ProgramAndVarsGen.kt @@ -328,10 +328,9 @@ internal class ProgramAndVarsGen( if (initializers.isNotEmpty()) { asmgen.out("prog8_init_vars\t.block") initializers.forEach { assign -> - if((assign.value as? PtNumber)?.number != 0.0 || allocator.isZpVar(assign.target.identifier!!.name)) + val constvalue = assign.value as? PtNumber + if(constvalue==null || constvalue.number!=0.0 || allocator.isZpVar(assign.target.identifier!!.name)) asmgen.translate(assign) - else - throw AssemblyError("non-zp variable should not be initialized to zero; it will be zeroed as part of BSS clear") // the other variables that should be set to zero are done so as part of the BSS section clear. } asmgen.out(" rts\n .bend") diff --git a/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt b/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt index 29ec32296..9ac1bb953 100644 --- a/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt +++ b/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt @@ -75,7 +75,7 @@ class IRCodeGen( initsToRemove += block to initialization } is PtNumber -> { - require(initValue.number!=0.0) { "variable should not be initialized with 0, it will already be zeroed as part of BSS clear, initializer=$initialization" } + require(initValue.number!=0.0 || variable.zpwish!=ZeropageWish.NOT_IN_ZEROPAGE) {"non-zp variable should not be initialized with 0, it will already be zeroed as part of BSS clear, initializer=$initialization" } variable.setOnetimeInitNumeric(initValue.number) initsToRemove += block to initialization } diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt index e458c2adc..1b7d342e8 100644 --- a/compiler/src/prog8/compiler/Compiler.kt +++ b/compiler/src/prog8/compiler/Compiler.kt @@ -468,7 +468,7 @@ private fun processAst(program: Program, errors: IErrorReporter, compilerOptions errors.report() program.constantFold(errors, compilerOptions) errors.report() - program.reorderStatements(errors) + program.reorderStatements(compilerOptions, errors) errors.report() program.desugaring(errors, compilerOptions) errors.report() diff --git a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt index 6fd7406f4..cf698dca4 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt @@ -20,8 +20,8 @@ internal fun Program.checkValid(errors: IErrorReporter, compilerOptions: Compila checker.visit(this) } -internal fun Program.reorderStatements(errors: IErrorReporter) { - val reorder = StatementReorderer(this, errors) +internal fun Program.reorderStatements(options: CompilationOptions, errors: IErrorReporter) { + val reorder = StatementReorderer(this, options, errors) reorder.visit(this) if(errors.noErrors()) { reorder.applyModifications() diff --git a/compiler/src/prog8/compiler/astprocessing/SimplifiedAstMaker.kt b/compiler/src/prog8/compiler/astprocessing/SimplifiedAstMaker.kt index d93146791..af79cb32e 100644 --- a/compiler/src/prog8/compiler/astprocessing/SimplifiedAstMaker.kt +++ b/compiler/src/prog8/compiler/astprocessing/SimplifiedAstMaker.kt @@ -165,8 +165,11 @@ class SimplifiedAstMaker(private val program: Program, private val errors: IErro } } - if(srcAssign.origin == AssignmentOrigin.VARINIT && srcAssign.parent is Block && srcAssign.value.constValue(program)?.number==0.0) - throw FatalAstException("should not have a redundant block-level variable=0 assignment; it will be zeroed as part of BSS clear") + if(srcAssign.origin == AssignmentOrigin.VARINIT && srcAssign.parent is Block && srcAssign.value.constValue(program)?.number==0.0) { + val zeropages = srcAssign.target.targetIdentifiers().mapNotNull { it.targetVarDecl()?.zeropage } + if(zeropages.any {it==ZeropageWish.NOT_IN_ZEROPAGE}) + throw FatalAstException("should not have a redundant block-level variable=0 assignment for a non-ZP variable; it will be zeroed as part of BSS clear") + } val assign = PtAssignment(srcAssign.position, srcAssign.origin==AssignmentOrigin.VARINIT) val multi = srcAssign.target.multi diff --git a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt index fc4c4efca..c3b5867b5 100644 --- a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt +++ b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt @@ -5,13 +5,11 @@ import prog8.ast.expressions.* import prog8.ast.statements.* import prog8.ast.walk.AstWalker import prog8.ast.walk.IAstModification -import prog8.code.core.AssociativeOperators -import prog8.code.core.BaseDataType -import prog8.code.core.DataType -import prog8.code.core.IErrorReporter +import prog8.code.core.* internal class StatementReorderer( val program: Program, + val options: CompilationOptions, val errors: IErrorReporter ) : AstWalker() { // Reorders the statements in a way the compiler needs. @@ -114,10 +112,17 @@ internal class StatementReorderer( } private fun canSkipInitializationWith0(decl: VarDecl): Boolean { - // if the variable is declared in a block, we can omit the init with 0 because - // the variable will be initialized to zero when the BSS section is cleared as a whole. - if(decl.parent is Block) - return true + if(decl.parent is Block) { + // if the variable is declared in a block and is NOT in ZEROPAGE, we can omit the init with 0 because + // the variable will be initialized to zero when the BSS section is cleared as a whole. + if (decl.zeropage == ZeropageWish.NOT_IN_ZEROPAGE) + return true + + // block level zp var that is not in zeropage, doesn't have to be cleared (will be done as part of bss clear at startup) + // note: subroutine level var HAS to be cleared because it needs to be zero at every subroutine call! + if (decl.zeropage == ZeropageWish.DONTCARE && options.zeropage == ZeropageType.DONTUSE) + return true + } // 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. diff --git a/compiler/test/codegeneration/TestVariables.kt b/compiler/test/codegeneration/TestVariables.kt index 61c57f3dd..b2649320b 100644 --- a/compiler/test/codegeneration/TestVariables.kt +++ b/compiler/test/codegeneration/TestVariables.kt @@ -10,7 +10,10 @@ import prog8.ast.statements.Assignment import prog8.ast.statements.AssignmentOrigin import prog8.ast.statements.ForLoop import prog8.ast.statements.VarDecl +import prog8.code.ast.PtAssignment +import prog8.code.ast.PtNumber import prog8.code.target.C64Target +import prog8.code.target.Cx16Target import prog8tests.helpers.ErrorReporterForTests import prog8tests.helpers.compileText @@ -106,6 +109,7 @@ class TestVariables: FunSpec({ test("global var init with array lookup should sometimes be const") { val src=""" +%zeropage dontuse main { bool[] barray = [true, false, true, false] @@ -220,4 +224,85 @@ main { (st[5] as Assignment).target.identifier?.nameInSource shouldBe listOf("v1") (st[6] as Assignment).target.identifier?.nameInSource shouldBe listOf("v0") } + + test("nondirty zp variables should be explicitly initialized to 0") { + val src=""" +main { + ubyte @shared @requirezp zpvar + ubyte @shared @requirezp @dirty dirtyzpvar + + sub start() { + ubyte @shared @requirezp zpvar2 + ubyte @shared @requirezp @dirty dirtyzpvar2 + } +}""" + + val result = compileText(Cx16Target(), false, src, outputDir, writeAssembly = true)!!.codegenAst + + val main = result!!.allBlocks().first { it.name=="p8b_main" } + main.children.size shouldBe 4 + val zeroassignlobal = main.children.single { it is PtAssignment } as PtAssignment + (zeroassignlobal.value as PtNumber).number shouldBe 0.0 + zeroassignlobal.target.identifier!!.name shouldBe "p8b_main.p8v_zpvar" + + val st = result.entrypoint()!!.children + st.size shouldBe 4 + val zeroassign = st.single { it is PtAssignment } as PtAssignment + (zeroassign.value as PtNumber).number shouldBe 0.0 + zeroassign.target.identifier!!.name shouldBe "p8b_main.p8s_start.p8v_zpvar2" + } + + test("nondirty non zp variables in block scope should not be explicitly initialized to 0 (bss clear takes care of it)") { + val src=""" +%zeropage dontuse + +main { + ubyte @shared v1 + ubyte @shared @dirty dv1 + sub start() { + ubyte @shared v2 + ubyte @shared @dirty dv2 + } +}""" + + val result = compileText(Cx16Target(), false, src, outputDir, writeAssembly = true)!!.codegenAst + + // block level should not be intialized to 0 (will be done by BSS clear) + val main = result!!.allBlocks().first { it.name=="p8b_main" } + main.children.size shouldBe 3 + main.children.any { it is PtAssignment } shouldBe false + + // subroutine should be initialized to 0 because that needs to be done on every call to the subroutine + val st = result.entrypoint()!!.children + st.size shouldBe 4 + val zeroassign = st.single { it is PtAssignment } as PtAssignment + (zeroassign.value as PtNumber).number shouldBe 0.0 + zeroassign.target.identifier!!.name shouldBe "p8b_main.p8s_start.p8v_v2" + } + + test("nondirty explicit non zp variables in block scope should not be explicitly initialized to 0 (bss clear takes care of it)") { + val src=""" +main { + ubyte @shared @nozp v1 + ubyte @shared @dirty @nozp dv1 + sub start() { + ubyte @shared @nozp v2 + ubyte @shared @dirty @nozp dv2 + } +}""" + + val result = compileText(Cx16Target(), false, src, outputDir, writeAssembly = true)!!.codegenAst + + // block level should not be intialized to 0 (will be done by BSS clear) + val main = result!!.allBlocks().first { it.name=="p8b_main" } + main.children.size shouldBe 3 + main.children.any { it is PtAssignment } shouldBe false + + // subroutine should be initialized to 0 because that needs to be done on every call to the subroutine + val st = result.entrypoint()!!.children + st.size shouldBe 4 + val zeroassign = st.single { it is PtAssignment } as PtAssignment + (zeroassign.value as PtNumber).number shouldBe 0.0 + zeroassign.target.identifier!!.name shouldBe "p8b_main.p8s_start.p8v_v2" + } }) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 91c36c4ea..5d55a422d 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,7 +1,9 @@ TODO ==== -BUG: fix ZP variables no longer getting initialized to 0 (see test.p8) +Since fixing the missing zp-var initialization, programs grew in size again (assem) +Are there any redundant block-level variable initializations to 0 that we can remove in peephole optimization for example? + STRUCTS: are being developed in their own separate branch for now, called "structs". Idea is to make it feature complete in the IR/Virtual target, then merge it to master?, and then start building the 6502 code generation for it. diff --git a/examples/test.p8 b/examples/test.p8 index 89f0b54fc..059283fa3 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -4,13 +4,47 @@ %zpallowed 224,255 main { - uword @shared @requirezp var1 = 0 - uword @shared @requirezp var2 = 0 - uword @shared @requirezp var3 = 0 - uword @shared @requirezp var4 = 0 - uword @shared @requirezp var5 = 0 + uword @shared @requirezp zpvar1 + uword @shared @requirezp zpvar2 + uword @shared @requirezp zpvar3 + uword @shared @requirezp zpvar4 + uword @shared @requirezp zpvar5 + uword @shared @requirezp @dirty dzpvar1 + uword @shared @requirezp @dirty dzpvar2 + uword @shared @requirezp @dirty dzpvar3 + uword @shared @requirezp @dirty dzpvar4 + uword @shared @requirezp @dirty dzpvar5 + uword @shared @nozp var1 + uword @shared @nozp var2 + uword @shared @nozp var3 + uword @shared @nozp var4 + uword @shared @nozp var5 + uword @shared @nozp @dirty dvar1 + uword @shared @nozp @dirty dvar2 + uword @shared @nozp @dirty dvar3 + uword @shared @nozp @dirty dvar4 + uword @shared @nozp @dirty dvar5 sub start() { + txt.print("address start of zpvars: ") + txt.print_uw(&zpvar1) + txt.nl() + txt.print("address start of normal vars: ") + txt.print_uw(&var1) + txt.nl() + + txt.print("non-dirty zp should all be 0: ") + txt.print_uw(zpvar1) + txt.spc() + txt.print_uw(zpvar2) + txt.spc() + txt.print_uw(zpvar3) + txt.spc() + txt.print_uw(zpvar4) + txt.spc() + txt.print_uw(zpvar5) + txt.nl() + txt.print("non-dirty should all be 0: ") txt.print_uw(var1) txt.spc() txt.print_uw(var2) @@ -22,6 +56,29 @@ main { txt.print_uw(var5) txt.nl() + txt.print("dirty zp may be random: ") + txt.print_uw(dzpvar1) + txt.spc() + txt.print_uw(dzpvar2) + txt.spc() + txt.print_uw(dzpvar3) + txt.spc() + txt.print_uw(dzpvar4) + txt.spc() + txt.print_uw(dzpvar5) + txt.nl() + txt.print("dirty may be random: ") + txt.print_uw(dvar1) + txt.spc() + txt.print_uw(dvar2) + txt.spc() + txt.print_uw(dvar3) + txt.spc() + txt.print_uw(dvar4) + txt.spc() + txt.print_uw(dvar5) + txt.nl() + repeat {} } } diff --git a/simpleAst/src/prog8/code/SymbolTable.kt b/simpleAst/src/prog8/code/SymbolTable.kt index c646fdc57..ff3c039ad 100644 --- a/simpleAst/src/prog8/code/SymbolTable.kt +++ b/simpleAst/src/prog8/code/SymbolTable.kt @@ -200,7 +200,7 @@ class StStaticVariable(name: String, // Certain codegens might want to put them back into the variable directly. // For strings and arrays this doesn't occur - these are always already specced at creation time. - require(number!=0.0) { "variable should not be initialized with 0, it will already be zeroed as part of BSS clear" } + require(number!=0.0 || zpwish!=ZeropageWish.NOT_IN_ZEROPAGE) { "non-zp variable should not be initialized with 0, it will already be zeroed as part of BSS clear" } initializationNumericValue = number }