From 1110bd08511d230c0393e7e7a59cbe8da30a5fd2 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Mon, 1 Nov 2021 00:24:15 +0100 Subject: [PATCH] fix vardecl initialization value to not use stack eval anymore but separate assignment (this causes the optimized assignment code gen to be used instead) but some programs now end up larger in output size --- .../compiler/target/cpu6502/codegen/AsmGen.kt | 43 +++----------- .../test/helpers/ErrorReporterForTests.kt | 2 + .../compiler/BeforeAsmGenerationAstChanger.kt | 58 +------------------ compiler/src/prog8/compiler/Compiler.kt | 2 +- .../compiler/astprocessing/AstChecker.kt | 4 ++ .../astprocessing/StatementReorderer.kt | 58 +++++++++++++++---- compiler/test/TestCompilerOnCharLit.kt | 10 +++- compiler/test/TestSubroutines.kt | 26 ++++----- .../test/helpers/ErrorReporterForTests.kt | 2 + .../src/prog8/ast/statements/AstStatements.kt | 9 +++ docs/source/todo.rst | 1 - examples/test.p8 | 28 +++++++-- 12 files changed, 118 insertions(+), 125 deletions(-) diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt index 6a60a76c4..9d1e613ca 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt @@ -46,7 +46,6 @@ class AsmGen(private val program: Program, private val assignmentAsmGen = AssignmentAsmGen(program, this, expressionsAsmGen) private val builtinFunctionsAsmGen = BuiltinFunctionsAsmGen(program, this, assignmentAsmGen) internal val loopEndLabels = ArrayDeque() - private val blockLevelVarInits = mutableMapOf>() internal val slabs = mutableMapOf() internal val removals = mutableListOf>() @@ -218,26 +217,17 @@ class AsmGen(private val program: Program, stmts.forEach { translate(it) } subroutine.forEach { translateSubroutine(it as Subroutine) } - // if any global vars need to be initialized, generate a subroutine that does this - // it will be called from program init. - if(block in blockLevelVarInits) { + // generate subroutine to initialize block-level variables + val initializers = block.statements.filterIsInstance() + if(initializers.isNotEmpty()) { out("prog8_init_vars\t.proc\n") - blockLevelVarInits.getValue(block).forEach { decl -> - val scopedFullName = decl.makeScopedName(decl.name).split('.') - require(scopedFullName.first()==block.name) - assignInitialValueToVar(decl, scopedFullName.drop(1)) - } + initializers.forEach { assign -> translate(assign) } out(" rts\n .pend") } out(if("force_output" in block.options()) "\n\t.bend\n" else "\n\t.pend\n") } - private fun assignInitialValueToVar(decl: VarDecl, variableName: List) { - val asmName = asmVariableName(variableName) - assignmentAsmGen.assignExpressionToVariable(decl.value!!, asmName, decl.datatype, decl.definingSubroutine) - } - private var generatedLabelSequenceNumber: Int = 0 internal fun makeLabel(postfix: String): String { @@ -905,7 +895,7 @@ class AsmGen(private val program: Program, out("; program startup initialization") out(" cld") program.allBlocks.forEach { - if(it.statements.filterIsInstance().any { vd->vd.value!=null && vd.type==VarDeclType.VAR && vd.datatype in NumericDatatypes}) + if(it.statements.any { stmt -> stmt is Assignment }) out(" jsr ${it.name}.prog8_init_vars") } out(""" @@ -1291,25 +1281,10 @@ $repeatLabel lda $counterVar } } - private fun translate(stmt: VarDecl) { - if(stmt.value!=null && stmt.type==VarDeclType.VAR && stmt.datatype in NumericDatatypes) { - // generate an assignment statement to (re)initialize the variable's value. - // if the vardecl is not in a subroutine however, we have to initialize it globally. - if(stmt.definingSubroutine ==null) { - val block = stmt.definingBlock - var inits = blockLevelVarInits[block] - if(inits==null) { - inits = mutableSetOf() - blockLevelVarInits[block] = inits - } - inits.add(stmt) - } else { - val next = stmt.nextSibling() - if (next !is ForLoop || next.loopVar.nameInSource.single() != stmt.name) { - assignInitialValueToVar(stmt, listOf(stmt.name)) - } - } - } + private fun translate(decl: VarDecl) { + if(decl.type==VarDeclType.VAR && decl.value != null && decl.datatype in NumericDatatypes) + throw AssemblyError("vardecls for variables, with initial numerical value, should have been rewritten as plain vardecl + assignment $decl") + // at this time, nothing has to be done here anymore code-wise } /** diff --git a/codeGeneration/test/helpers/ErrorReporterForTests.kt b/codeGeneration/test/helpers/ErrorReporterForTests.kt index 8a775ab48..c321c959f 100644 --- a/codeGeneration/test/helpers/ErrorReporterForTests.kt +++ b/codeGeneration/test/helpers/ErrorReporterForTests.kt @@ -20,6 +20,8 @@ internal class ErrorReporterForTests(private val throwExceptionAtReportIfErrors: override fun noErrors(): Boolean = errors.isEmpty() override fun report() { + warnings.forEach { println("UNITTEST COMPILATION REPORT: WARNING: $it") } + errors.forEach { println("UNITTEST COMPILATION REPORT: ERROR: $it") } if(throwExceptionAtReportIfErrors) finalizeNumErrors(errors.size, warnings.size) errors.clear() diff --git a/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt b/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt index 879087823..26deb6c3e 100644 --- a/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt +++ b/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt @@ -20,22 +20,10 @@ import prog8.compilerinterface.isInRegularRAMof internal class BeforeAsmGenerationAstChanger(val program: Program, val errors: IErrorReporter, private val compTarget: ICompilationTarget) : AstWalker() { override fun after(decl: VarDecl, parent: Node): Iterable { - subroutineVariables.add(decl.name to decl) - if (decl.value == null && !decl.autogeneratedDontRemove && decl.type == VarDeclType.VAR && decl.datatype in NumericDatatypes) { - // A numeric vardecl without an initial value is initialized with zero, - // unless there's already an assignment below, that initializes the value. - // This allows you to restart the program and have the same starting values of the variables - if(decl.allowInitializeWithZero) - { - val nextAssign = decl.nextSibling() as? Assignment - if (nextAssign != null && nextAssign.target isSameAs IdentifierReference(listOf(decl.name), Position.DUMMY)) - decl.value = null - else { - decl.value = decl.zeroElementValue() - } - } - } + if(decl.type==VarDeclType.VAR && decl.value != null && decl.datatype in NumericDatatypes) + throw FatalAstException("vardecls for variables, with initial numerical value, should have been rewritten as plain vardecl + assignment $decl") + subroutineVariables.add(decl.name to decl) return noModifications } @@ -208,49 +196,9 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, val errors: I (binExpr.right as? NumericLiteralValue)?.number!=0) throw InternalCompilerException("if 0==X should have been swapped to if X==0") - // split the conditional expression into separate variables if the operand(s) is not simple. - // DISABLED FOR NOW AS IT GENEREATES LARGER CODE IN THE SIMPLE CASES LIKE IF X {...} or IF NOT X {...} -// val modifications = mutableListOf() -// if(!binExpr.left.isSimple) { -// val sub = binExpr.definingSubroutine()!! -// val (variable, isNew, assignment) = addIfOperandVar(sub, "left", binExpr.left) -// if(isNew) -// modifications.add(IAstModification.InsertFirst(variable, sub)) -// modifications.add(IAstModification.InsertBefore(ifStatement, assignment, parent as INameScope)) -// modifications.add(IAstModification.ReplaceNode(binExpr.left, IdentifierReference(listOf(variable.name), binExpr.position), binExpr)) -// addedIfConditionVars.add(Pair(sub, variable.name)) -// } -// if(!binExpr.right.isSimple) { -// val sub = binExpr.definingSubroutine()!! -// val (variable, isNew, assignment) = addIfOperandVar(sub, "right", binExpr.right) -// if(isNew) -// modifications.add(IAstModification.InsertFirst(variable, sub)) -// modifications.add(IAstModification.InsertBefore(ifStatement, assignment, parent as INameScope)) -// modifications.add(IAstModification.ReplaceNode(binExpr.right, IdentifierReference(listOf(variable.name), binExpr.position), binExpr)) -// addedIfConditionVars.add(Pair(sub, variable.name)) -// } -// return modifications return noModifications } -// private fun addIfOperandVar(sub: Subroutine, side: String, operand: Expression): Triple { -// val dt = operand.inferType(program).typeOrElse(DataType.UNDEFINED) -// val varname = "prog8_ifvar_${side}_${dt.name.toLowerCase()}" -// val tgt = AssignTarget(IdentifierReference(listOf(varname), operand.position), null, null, operand.position) -// val assign = Assignment(tgt, operand, operand.position) -// if(Pair(sub, varname) in addedIfConditionVars) { -// val vardecl = VarDecl(VarDeclType.VAR, dt, ZeropageWish.DONTCARE, null, varname, null, null, false, true, operand.position) -// return Triple(vardecl, false, assign) -// } -// val existing = sub.statements.firstOrNull { it is VarDecl && it.name == varname} as VarDecl? -// return if (existing == null) { -// val vardecl = VarDecl(VarDeclType.VAR, dt, ZeropageWish.DONTCARE, null, varname, null, null, false, true, operand.position) -// Triple(vardecl, true, assign) -// } else { -// Triple(existing, false, assign) -// } -// } - override fun after(untilLoop: UntilLoop, parent: Node): Iterable { val binExpr = untilLoop.condition as? BinaryExpression if(binExpr==null || binExpr.operator !in comparisonOperators) { diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt index 066ac3497..01cad3fbd 100644 --- a/compiler/src/prog8/compiler/Compiler.kt +++ b/compiler/src/prog8/compiler/Compiler.kt @@ -318,7 +318,7 @@ private fun writeAssembly(program: Program, program.processAstBeforeAsmGeneration(errors, compilerOptions.compTarget) errors.report() -// printAst(program) + // printAst(program) compilerOptions.compTarget.machine.initializeZeropage(compilerOptions) val assembly = asmGeneratorFor(compilerOptions.compTarget, diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 8b0875953..df223aca4 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -194,6 +194,10 @@ internal class AstChecker(private val program: Program, is InlineAssembly, is IStatementContainer, is NopStatement -> true + is Assignment -> { + val target = statement.target.identifier!!.targetStatement(program) + target === statement.previousSibling() // an initializer assignment is okay + } else -> false } if (!ok) { diff --git a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt index 428548799..1d04a5096 100644 --- a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt +++ b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt @@ -1,11 +1,7 @@ package prog8.compiler.astprocessing -import prog8.ast.IFunctionCall -import prog8.ast.Module -import prog8.ast.Node -import prog8.ast.Program -import prog8.ast.base.DataType -import prog8.ast.base.FatalAstException +import prog8.ast.* +import prog8.ast.base.* import prog8.ast.expressions.* import prog8.ast.statements.* import prog8.ast.walk.AstWalker @@ -38,15 +34,53 @@ internal class StatementReorderer(val program: Program, val errors: IErrorReport module.statements.add(0, mainBlock) } - reorderVardeclsAndDirectives(module.statements) + directivesToTheTop(module.statements) return noModifications } - private fun reorderVardeclsAndDirectives(statements: MutableList) { - val varDecls = statements.filterIsInstance() - statements.removeAll(varDecls) - statements.addAll(0, varDecls) + private val declsProcessedWithInitAssignment = mutableSetOf() + override fun after(decl: VarDecl, parent: Node): Iterable { + if(decl.type == VarDeclType.VAR && decl.datatype in NumericDatatypes) { + if(decl !in declsProcessedWithInitAssignment) { + declsProcessedWithInitAssignment.add(decl) + if (decl.value == null) { + if (!decl.autogeneratedDontRemove && decl.allowInitializeWithZero) { + // A numeric vardecl without an initial value is initialized with zero, + // unless there's already an assignment below, that initializes the value. + // 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 + val nextAssign = decl.nextSibling() as? Assignment + if (nextAssign == null || !(nextAssign.target isSameAs IdentifierReference(listOf(decl.name), Position.DUMMY))) { + // 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) + val assignzero = Assignment(AssignTarget(identifier, null, null, decl.position), decl.zeroElementValue(), decl.position) + return listOf(IAstModification.InsertAfter( + decl, assignzero, parent as IStatementContainer + )) + } + } + } else { + // Transform the vardecl with initvalue to a plain vardecl + assignment + // this allows for other optimizations to kick in. + // So basically consider 'ubyte xx=99' as a short form for 'ubyte xx; xx=99' + val pos = decl.value!!.position + val identifier = IdentifierReference(listOf(decl.name), pos) + val assign = Assignment(AssignTarget(identifier, null, null, pos), decl.value!!, pos) + decl.value = null + return listOf(IAstModification.InsertAfter( + decl, assign, parent as IStatementContainer + )) + } + } + } + + return noModifications + } + + private fun directivesToTheTop(statements: MutableList) { val directives = statements.filterIsInstance().filter {it.directive in directivesToMove} statements.removeAll(directives) statements.addAll(0, directives) @@ -61,7 +95,7 @@ internal class StatementReorderer(val program: Program, val errors: IErrorReport ) } - reorderVardeclsAndDirectives(block.statements) + directivesToTheTop(block.statements) return noModifications } diff --git a/compiler/test/TestCompilerOnCharLit.kt b/compiler/test/TestCompilerOnCharLit.kt index 0b73219d2..87395cc54 100644 --- a/compiler/test/TestCompilerOnCharLit.kt +++ b/compiler/test/TestCompilerOnCharLit.kt @@ -7,11 +7,13 @@ import prog8.ast.base.DataType import prog8.ast.base.VarDeclType import prog8.ast.expressions.IdentifierReference import prog8.ast.expressions.NumericLiteralValue +import prog8.ast.statements.Assignment import prog8.compiler.target.Cx16Target import prog8tests.helpers.assertSuccess import prog8tests.helpers.compileText import kotlin.test.assertEquals import kotlin.test.assertIs +import kotlin.test.assertNull /** @@ -73,9 +75,11 @@ class TestCompilerOnCharLit { // val initializerValue = decl.value as CharLiteral // assertEquals('\n', (initializerValue as CharLiteral).value) - assertIs(decl.value, - "char literal should have been replaced by ubyte literal") - val initializerValue = decl.value as NumericLiteralValue + assertNull(decl.value, "initializer value should have been moved to separate assignment") + val assignInitialValue = decl.nextSibling() as Assignment + assertEquals(listOf("ch"), assignInitialValue.target.identifier!!.nameInSource) + assertIs(assignInitialValue.value, "char literal should have been replaced by ubyte literal") + val initializerValue = assignInitialValue.value as NumericLiteralValue assertEquals(DataType.UBYTE, initializerValue.type) assertEquals(platform.encodeString("\n", false)[0], initializerValue.number.toShort()) } diff --git a/compiler/test/TestSubroutines.kt b/compiler/test/TestSubroutines.kt index 9a0189576..4451dc0c4 100644 --- a/compiler/test/TestSubroutines.kt +++ b/compiler/test/TestSubroutines.kt @@ -55,15 +55,15 @@ class TestSubroutines { assertTrue(asmfunc.statements.isEmpty()) assertFalse(func.isAsmSubroutine) assertEquals(DataType.STR, func.parameters.single().type) - assertEquals(3, func.statements.size) + assertEquals(4, func.statements.size) val paramvar = func.statements[0] as VarDecl assertEquals("thing", paramvar.name) assertEquals(DataType.STR, paramvar.datatype) - val t2var = func.statements[1] as VarDecl - assertEquals("t2", t2var.name) - assertTrue(t2var.value is TypecastExpression, "str param in function body should not be transformed by normal compiler steps") - assertEquals(DataType.UWORD, (t2var.value as TypecastExpression).type) - val call = func.statements[2] as FunctionCallStatement + val assign = func.statements[2] as Assignment + assertEquals(listOf("t2"), assign.target.identifier!!.nameInSource) + assertTrue(assign.value is TypecastExpression, "str param in function body should not be transformed by normal compiler steps") + assertEquals(DataType.UWORD, (assign.value as TypecastExpression).type) + val call = func.statements[3] as FunctionCallStatement assertEquals("asmfunc", call.target.nameInSource.single()) assertTrue(call.args.single() is IdentifierReference, "str param in function body should not be transformed by normal compiler steps") assertEquals("thing", (call.args.single() as IdentifierReference).nameInSource.single()) @@ -105,16 +105,16 @@ class TestSubroutines { assertEquals(DataType.UWORD, func.parameters.single().type, "asmgen should have changed str to uword type") assertTrue(asmfunc.statements.last() is Return) - assertEquals(4, func.statements.size) - assertTrue(func.statements[3] is Return) + assertEquals(5, func.statements.size) + assertTrue(func.statements[4] is Return) val paramvar = func.statements[0] as VarDecl assertEquals("thing", paramvar.name) assertEquals(DataType.UWORD, paramvar.datatype, "pre-asmgen should have changed str to uword type") - val t2var = func.statements[1] as VarDecl - assertEquals("t2", t2var.name) - assertTrue(t2var.value is IdentifierReference, "str param in function body should be treated as plain uword before asmgen") - assertEquals("thing", (t2var.value as IdentifierReference).nameInSource.single()) - val call = func.statements[2] as FunctionCallStatement + val assign = func.statements[2] as Assignment + assertEquals(listOf("t2"), assign.target.identifier!!.nameInSource) + assertTrue(assign.value is IdentifierReference, "str param in function body should be treated as plain uword before asmgen") + assertEquals("thing", (assign.value as IdentifierReference).nameInSource.single()) + val call = func.statements[3] as FunctionCallStatement assertEquals("asmfunc", call.target.nameInSource.single()) assertTrue(call.args.single() is IdentifierReference, "str param in function body should be treated as plain uword and not been transformed") assertEquals("thing", (call.args.single() as IdentifierReference).nameInSource.single()) diff --git a/compiler/test/helpers/ErrorReporterForTests.kt b/compiler/test/helpers/ErrorReporterForTests.kt index a11dc1d97..eddbb248a 100644 --- a/compiler/test/helpers/ErrorReporterForTests.kt +++ b/compiler/test/helpers/ErrorReporterForTests.kt @@ -20,6 +20,8 @@ internal class ErrorReporterForTests(private val throwExceptionAtReportIfErrors: override fun noErrors(): Boolean = errors.isEmpty() override fun report() { + warnings.forEach { println("UNITTEST COMPILATION REPORT: WARNING: $it") } + errors.forEach { println("UNITTEST COMPILATION REPORT: ERROR: $it") } if(throwExceptionAtReportIfErrors) finalizeNumErrors(errors.size, warnings.size) errors.clear() diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index ee39c88fe..30860df81 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt @@ -41,6 +41,15 @@ sealed class Statement : Node { else null } + + fun previousSibling(): Statement? { + val statements = (parent as? IStatementContainer)?.statements ?: return null + val previousIdx = statements.indexOfFirst { it===this } - 1 + return if(previousIdx >= 0) + statements[previousIdx] + else + null + } } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 22cf93b71..20b73e8cf 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -4,7 +4,6 @@ TODO For next compiler release (7.2) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - fix the asm-labels problem (github issue #62) -- fix that "uword qq =$1000+xx" uses stack eval, while "uword qq; qq=$1000+xx" uses optimized code - find a way to optimize asm-subroutine param passing where it now sometimes uses the evalstack? diff --git a/examples/test.p8 b/examples/test.p8 index f0c3ed87a..2bc49e233 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,23 +1,39 @@ %import textio %zeropage dontuse +; TODO why are these bigger now than before the var-initializer optimization: +; wizzine +; wormfood +; cube3d-float (THIS ONE IS A LOT BIGGER!!) +; cube3d-sprites +; textelite +; etc. + + main { + ubyte globalvar1 + ubyte globalvar2 = 99 + sub start() { + uword xx0 + ubyte xx = 100 - uword qq =$1234+xx ; TODO FIX THAT THIS USES STACK + uword qq =$1234+xx uword ww ww=$1234+xx + ; ubyte cv ; sys.memset($1000+xx, 10, 255) ; TODO uses stack eval now to precalc parameters -; -; xx = xx & %0001 ; doesn't use stack... because it uses AugmentableAssignmentAsmGen -; ;yy = xx & %0001 ; doesn't use stack... because it uses AugmentableAssignmentAsmGen -; -; ;ubyte yy = xx & %0001 ; TODO uses stack eval.... + + ;xx = xx & %0001 ; doesn't use stack... because it uses AugmentableAssignmentAsmGen + ;yy = xx & %0001 ; doesn't use stack... because it uses AugmentableAssignmentAsmGen + + ubyte yy = xx & %0001 ; TODO uses stack eval.... + ; if xx & %0001 { ; TODO why does this use stack? because it uses asmgen.assignExpressionToRegister eventually line 253 in AssignmentAsmGen no augmentable-assignment. ; xx-- ; }