From 54d92a027a3c7895e0657706f1d03b23241e8a7f Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Mon, 12 Apr 2021 22:25:33 +0200 Subject: [PATCH] fix problems with moving vardecls from inner scope to subroutine scope --- .../compiler/BeforeAsmGenerationAstChanger.kt | 33 +++++++++++------- .../astprocessing/StatementReorderer.kt | 26 ++++++-------- .../src/prog8/ast/statements/AstStatements.kt | 7 +++- docs/source/todo.rst | 3 -- examples/test.p8 | 34 ++++--------------- 5 files changed, 43 insertions(+), 60 deletions(-) diff --git a/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt b/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt index b594813b8..b44b9c7ee 100644 --- a/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt +++ b/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt @@ -18,15 +18,17 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, val errors: I 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 + // 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.definingScope().nextSibling(decl) as? Assignment if (nextAssign != null && nextAssign.target isSameAs IdentifierReference(listOf(decl.name), Position.DUMMY)) decl.value = null - else + else { decl.value = decl.zeroElementValue() + } } } return noModifications @@ -80,17 +82,22 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, val errors: I val sub = scope.definingSubroutine() if (sub != null) { // move vardecls of the scope into the upper scope. Make sure the position remains the same! - val numericVarsWithValue = decls.filter { it.value != null && it.datatype in NumericDatatypes } - val replaceVardecls =numericVarsWithValue.map { - val initValue = it.value!! // assume here that value has always been set by now - it.value = null // make sure no value init assignment for this vardecl will be created later (would be superfluous) - val target = AssignTarget(IdentifierReference(listOf(it.name), it.position), null, null, it.position) - val assign = Assignment(target, initValue, it.position) - initValue.parent = assign - IAstModification.ReplaceNode(it, assign, scope) + val replacements = mutableListOf() + val movements = mutableListOf() + + for(decl in decls) { + if(decl.value!=null && decl.datatype in NumericDatatypes) { + val target = AssignTarget(IdentifierReference(listOf(decl.name), decl.position), null, null, decl.position) + val assign = Assignment(target, decl.value!!, decl.position) + replacements.add(IAstModification.ReplaceNode(decl, assign, scope)) + decl.value = null + decl.allowInitializeWithZero = false + } else { + replacements.add(IAstModification.Remove(decl, scope)) + } + movements.add(IAstModification.InsertFirst(decl, sub)) } - val moveVardeclsUp = decls.map { IAstModification.InsertFirst(it, sub) } - return replaceVardecls + moveVardeclsUp + return replacements + movements } return noModifications } diff --git a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt index f4f594e59..cf69d28f4 100644 --- a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt +++ b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt @@ -1,9 +1,6 @@ package prog8.compiler.astprocessing -import prog8.ast.IFunctionCall -import prog8.ast.Module -import prog8.ast.Node -import prog8.ast.Program +import prog8.ast.* import prog8.ast.base.* import prog8.ast.expressions.* import prog8.ast.statements.* @@ -201,18 +198,15 @@ internal class StatementReorderer(val program: Program, val errors: IErrorReport if(declValue!=null && decl.type== VarDeclType.VAR && decl.datatype in NumericDatatypes) { val declConstValue = declValue.constValue(program) if(declConstValue==null) { - // move the vardecl (without value) to the scope and replace this with a regular assignment - // Unless we're dealing with a floating point variable because that will actually make things less efficient at the moment (because floats are mostly calcualated via the stack) - if(decl.datatype!=DataType.FLOAT) { - decl.value = null - decl.allowInitializeWithZero = false - val target = AssignTarget(IdentifierReference(listOf(decl.name), decl.position), null, null, decl.position) - val assign = Assignment(target, declValue, decl.position) - return listOf( - IAstModification.ReplaceNode(decl, assign, parent), - IAstModification.InsertFirst(decl.copy(), decl.definingScope()) - ) - } + // move the vardecl (without value) to the scope of the defining subroutine and put a regular assignment in its place here. + decl.value = null + decl.allowInitializeWithZero = false + val target = AssignTarget(IdentifierReference(listOf(decl.name), decl.position), null, null, decl.position) + val assign = Assignment(target, declValue, decl.position) + return listOf( + IAstModification.ReplaceNode(decl, assign, parent), + IAstModification.InsertFirst(decl, decl.definingSubroutine() as INameScope) + ) } } return noModifications diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index 32c7135a9..4ca1408c6 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt @@ -281,7 +281,12 @@ open class VarDecl(val type: VarDeclType, return result } - fun copy() = VarDecl(type, declaredDatatype, zeropage, arraysize, name, structName, value, isArray, autogeneratedDontRemove, position) + fun copy(): VarDecl { + val c = VarDecl(type, declaredDatatype, zeropage, arraysize, name, structName, value, isArray, autogeneratedDontRemove, position) + c.allowInitializeWithZero = this.allowInitializeWithZero + c.structHasBeenFlattened = this.structHasBeenFlattened + return c + } } // a vardecl used only for subroutine parameters diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 823238bc5..41b9fc305 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -2,9 +2,6 @@ TODO ==== -- fix: cube3d is suddenly way bigger than it was a few changes ago - - - make sure that in if statements, the left and right operand of the comparison is never a complex expression anymore (only number, variable, addressof or memread) by rewriting if {..} into: if_eval_left = left diff --git a/examples/test.p8 b/examples/test.p8 index 9d2db9b8d..8b9cf3685 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -2,35 +2,15 @@ %zeropage basicsafe main { + sub start() { + word total - sub start() { - ubyte yy - ubyte joy=1 - ubyte zz - - joy >>= 1 - if_cs - yy++ - - joy >>= 1 - if_cs - yy++ - -; TODO the shifting checks above result in way smaller code than this: - if joy+44 > 33 { - yy++ + repeat 10 { + word wcosa + word zz=1 + total += zz } - yy=joy+44>33 - if yy { - yy++ - } - - if joy & %00000001 - yy++ - if joy & %00000010 - yy++ - + txt.print_w(total) } } -