diff --git a/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt b/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt index 8cb049c7e..eda223be7 100644 --- a/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt +++ b/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt @@ -305,22 +305,22 @@ class StatementOptimizer(private val program: Program, if(rNum!=null) { if (op1 == "+" || op1 == "-") { if (op2 == "+") { - // A = A +/- B + N + // A = A +/- B + N ---> A = A +/- B ; A = A + N val expr2 = BinaryExpression(binExpr.left, binExpr.operator, rExpr.left, binExpr.position) val addConstant = Assignment( - assignment.target, - BinaryExpression(binExpr.left, "+", rExpr.right, rExpr.position), + assignment.target.copy(), + BinaryExpression(binExpr.left.copy(), "+", rExpr.right, rExpr.position), assignment.position ) return listOf( IAstModification.ReplaceNode(binExpr, expr2, binExpr.parent), IAstModification.InsertAfter(assignment, addConstant, parent as IStatementContainer)) } else if (op2 == "-") { - // A = A +/- B - N + // A = A +/- B - N ---> A = A +/- B ; A = A - N val expr2 = BinaryExpression(binExpr.left, binExpr.operator, rExpr.left, binExpr.position) val subConstant = Assignment( - assignment.target, - BinaryExpression(binExpr.left, "-", rExpr.right, rExpr.position), + assignment.target.copy(), + BinaryExpression(binExpr.left.copy(), "-", rExpr.right, rExpr.position), assignment.position ) return listOf( @@ -375,7 +375,7 @@ class StatementOptimizer(private val program: Program, repeat(rightCv.toInt()) { incs.statements.add(PostIncrDecr(assignment.target.copy(), "++", assignment.position)) } - return listOf(IAstModification.ReplaceNode(assignment, incs, parent)) + listOf(IAstModification.ReplaceNode(assignment, if(incs.statements.size==1) incs.statements[0] else incs, parent)) } } } diff --git a/compiler/test/TestOptimization.kt b/compiler/test/TestOptimization.kt index 3c87e248f..95a0742aa 100644 --- a/compiler/test/TestOptimization.kt +++ b/compiler/test/TestOptimization.kt @@ -321,4 +321,43 @@ class TestOptimization: FunSpec({ func2.statements.size shouldBe 1 (func2.statements[0] as Assignment).target.identifier!!.nameInSource shouldBe listOf("cx16", "r0") } + + test("test simple augmented assignment optimization correctly initializes all variables") { + val src=""" + main { + sub start() { + ubyte z1 + z1 = 10 + ubyte z2 + z2 = z1+z2+5 + } + }""" + val result = compileText(C64Target, optimize=true, src, writeAssembly=false).assertSuccess() + /* expected: + ubyte z1 + z1 = 10 + ubyte z2 + z2 = 0 + z2 += z1 ; TODO actually add optimization to make this even better: no =0, and this should become z2 = z1 + z2 += 5 + */ + val statements = result.program.entrypoint.statements + statements.size shouldBe 6 // TODO 5 + val z1decl = statements[0] as VarDecl + val z1init = statements[1] as Assignment + val z2decl = statements[2] as VarDecl + val z2init = statements[3] as Assignment + val z2plus1 = statements[4] as Assignment + val z2plus2= statements[5] as Assignment + + z1decl.name shouldBe "z1" + z1init.value shouldBe NumericLiteralValue(DataType.UBYTE, 10.0, Position.DUMMY) + z2decl.name shouldBe "z2" + z2init.value shouldBe NumericLiteralValue(DataType.UBYTE, 0.0, Position.DUMMY) + z2plus1.isAugmentable shouldBe true + (z2plus1.value as BinaryExpression).operator shouldBe "+" + (z2plus1.value as BinaryExpression).right shouldBe IdentifierReference(listOf("z1"), Position.DUMMY) + (z2plus2.value as BinaryExpression).operator shouldBe "+" + (z2plus2.value as BinaryExpression).right shouldBe NumericLiteralValue(DataType.UBYTE, 5.0, Position.DUMMY) + } }) diff --git a/compilerAst/src/prog8/ast/AstToplevel.kt b/compilerAst/src/prog8/ast/AstToplevel.kt index 7e8ef2b85..66bffc8c2 100644 --- a/compilerAst/src/prog8/ast/AstToplevel.kt +++ b/compilerAst/src/prog8/ast/AstToplevel.kt @@ -7,6 +7,7 @@ import prog8.ast.statements.* import prog8.ast.walk.AstWalker import prog8.ast.walk.IAstVisitor import prog8.parser.SourceCode +import kotlin.reflect.typeOf const val internedStringsModuleName = "prog8_interned_strings" @@ -262,6 +263,7 @@ interface Node { } fun replaceChildNode(node: Node, replacement: Node) + fun copy(): Node } @@ -302,6 +304,8 @@ open class Module(final override var statements: MutableList, replacement.parent = this } + override fun copy(): Node = throw NotImplementedError("no support for duplicating a Module") + override fun toString() = "Module(name=$name, pos=$position, lib=${isLibrary})" fun accept(visitor: IAstVisitor) = visitor.visit(this) @@ -317,6 +321,8 @@ class GlobalNamespace(val modules: Iterable): Node, INameScope { override val statements = mutableListOf() // not used override var parent: Node = ParentSentinel + override fun copy(): Node = throw NotImplementedError("no support for duplicating a GlobalNamespace") + override fun lookup(scopedName: List): Statement? { throw NotImplementedError("use lookup on actual ast node instead") } diff --git a/compilerAst/src/prog8/ast/base/Base.kt b/compilerAst/src/prog8/ast/base/Base.kt index c63e654b0..799bb690e 100644 --- a/compilerAst/src/prog8/ast/base/Base.kt +++ b/compilerAst/src/prog8/ast/base/Base.kt @@ -184,6 +184,8 @@ object ParentSentinel : Node { override fun replaceChildNode(node: Node, replacement: Node) { replacement.parent = this } + + override fun copy(): Node = throw FatalAstException("should never duplicate a ParentSentinel") } data class Position(val file: String, val line: Int, val startCol: Int, val endCol: Int) { diff --git a/compilerAst/src/prog8/ast/expressions/AstExpressions.kt b/compilerAst/src/prog8/ast/expressions/AstExpressions.kt index 2dae978ac..af6818b1d 100644 --- a/compilerAst/src/prog8/ast/expressions/AstExpressions.kt +++ b/compilerAst/src/prog8/ast/expressions/AstExpressions.kt @@ -6,7 +6,6 @@ import prog8.ast.base.* import prog8.ast.statements.* import prog8.ast.walk.AstWalker import prog8.ast.walk.IAstVisitor -import prog8.compilerinterface.IMemSizer import java.util.* import kotlin.math.round @@ -18,6 +17,7 @@ val logicalOperators = setOf("and", "or", "xor", "not") sealed class Expression: Node { + abstract override fun copy(): Expression abstract fun constValue(program: Program): NumericLiteralValue? abstract fun accept(visitor: IAstVisitor) abstract fun accept(visitor: AstWalker, parent: Node) @@ -92,6 +92,7 @@ class PrefixExpression(val operator: String, var expression: Expression, overrid replacement.parent = this } + override fun copy() = PrefixExpression(operator, expression.copy(), position) override fun constValue(program: Program): NumericLiteralValue? = null override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node)= visitor.visit(this, parent) @@ -145,9 +146,8 @@ class BinaryExpression(var left: Expression, var operator: String, var right: Ex replacement.parent = this } - override fun toString(): String { - return "[$left $operator $right]" - } + override fun copy() = BinaryExpression(left.copy(), operator, right.copy(), position) + override fun toString() = "[$left $operator $right]" override val isSimple = false @@ -292,7 +292,7 @@ class ArrayIndexedExpression(var arrayvar: IdentifierReference, return "ArrayIndexed(ident=$arrayvar, arraysize=$indexer; pos=$position)" } - fun copy() = ArrayIndexedExpression(arrayvar.copy(), indexer.copy(), position) + override fun copy() = ArrayIndexedExpression(arrayvar.copy(), indexer.copy(), position) } class TypecastExpression(var expression: Expression, var type: DataType, val implicit: Boolean, override val position: Position) : Expression() { @@ -311,6 +311,7 @@ class TypecastExpression(var expression: Expression, var type: DataType, val imp replacement.parent = this } + override fun copy() = TypecastExpression(expression.copy(), type, implicit, position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node)= visitor.visit(this, parent) @@ -346,6 +347,7 @@ data class AddressOf(var identifier: IdentifierReference, override val position: replacement.parent = this } + override fun copy() = AddressOf(identifier.copy(), position) override fun constValue(program: Program): NumericLiteralValue? = null override fun referencesIdentifier(vararg scopedName: String) = false override fun inferType(program: Program) = InferredTypes.knownFor(DataType.UWORD) @@ -369,6 +371,7 @@ class DirectMemoryRead(var addressExpression: Expression, override val position: replacement.parent = this } + override fun copy() = DirectMemoryRead(addressExpression.copy(), position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node)= visitor.visit(this, parent) @@ -388,7 +391,7 @@ class NumericLiteralValue(val type: DataType, // only numerical types allowed val number: Double = if(type==DataType.FLOAT) numbervalue else round(numbervalue) override val isSimple = true - fun copy() = NumericLiteralValue(type, number, position) + override fun copy() = NumericLiteralValue(type, number, position) companion object { fun fromBoolean(bool: Boolean, position: Position) = @@ -537,6 +540,7 @@ class CharLiteral(val value: Char, throw FatalAstException("can't replace here") } + override fun copy() = CharLiteral(value, altEncoding, position) override fun referencesIdentifier(vararg scopedName: String) = false override fun constValue(program: Program): NumericLiteralValue { val bytevalue = program.encoding.encodeString(value.toString(), altEncoding).single() @@ -566,7 +570,7 @@ class StringLiteralValue(val value: String, } override val isSimple = true - fun copy() = StringLiteralValue(value, altEncoding, position) + override fun copy() = StringLiteralValue(value, altEncoding, position) override fun replaceChildNode(node: Node, replacement: Node) { throw FatalAstException("can't replace here") @@ -598,6 +602,7 @@ class ArrayLiteralValue(val type: InferredTypes.InferredType, // inferred be value.forEach {it.linkParents(this)} } + override fun copy() = throw NotImplementedError("no support for duplicating a ArrayLiteralValue") override val isSimple = true override fun replaceChildNode(node: Node, replacement: Node) { @@ -709,6 +714,7 @@ class RangeExpr(var from: Expression, replacement.parent = this } + override fun copy() = RangeExpr(from.copy(), to.copy(), step.copy(), position) override fun constValue(program: Program): NumericLiteralValue? = null override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node)= visitor.visit(this, parent) @@ -766,6 +772,7 @@ data class IdentifierReference(val nameInSource: List, override val posi throw FatalAstException("can't replace here") } + override fun copy() = IdentifierReference(nameInSource, position) override fun constValue(program: Program): NumericLiteralValue? { val node = definingScope.lookup(nameInSource) ?: throw UndefinedSymbolError(this) val vardecl = node as? VarDecl @@ -816,6 +823,7 @@ class FunctionCall(override var target: IdentifierReference, args.forEach { it.linkParents(this) } } + override fun copy() = throw NotImplementedError("no support for duplicating a FunctionCall") override val isSimple = target.nameInSource.size==1 && (target.nameInSource[0] in arrayOf("msb", "lsb", "peek", "peekw")) override fun replaceChildNode(node: Node, replacement: Node) { diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index 53f338d10..85cebf2e7 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt @@ -12,6 +12,7 @@ interface INamedStatement { } sealed class Statement : Node { + abstract override fun copy(): Statement abstract fun accept(visitor: IAstVisitor) abstract fun accept(visitor: AstWalker, parent: Node) @@ -62,6 +63,8 @@ class BuiltinFunctionStatementPlaceholder(val name: String, override val positio override fun replaceChildNode(node: Node, replacement: Node) { replacement.parent = this } + + override fun copy() = throw NotImplementedError("no support for duplicating a BuiltinFunctionStatementPlaceholder") } data class RegisterOrStatusflag(val registerOrPair: RegisterOrPair?, val statusflag: Statusflag?) @@ -73,6 +76,8 @@ class Block(override val name: String, override val position: Position) : Statement(), INameScope { override lateinit var parent: Node + override fun copy() = throw NotImplementedError("no support for duplicating a Block") + override fun linkParents(parent: Node) { this.parent = parent statements.forEach {it.linkParents(this)} @@ -104,6 +109,7 @@ data class Directive(val directive: String, val args: List, overri } override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here") + override fun copy() = Directive(directive, args.map { it.copy() }, position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) } @@ -115,6 +121,7 @@ data class DirectiveArg(val str: String?, val name: String?, val int: Int?, over this.parent = parent } override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here") + override fun copy() = DirectiveArg(str, name, int, position) } data class Label(override val name: String, override val position: Position) : Statement(), INamedStatement { @@ -125,6 +132,7 @@ data class Label(override val name: String, override val position: Position) : S } override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here") + override fun copy() = Label(name, position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) @@ -147,6 +155,7 @@ open class Return(var value: Expression?, final override val position: Position) replacement.parent = this } + override fun copy() = Return(value?.copy(), position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) @@ -163,6 +172,7 @@ class Break(override val position: Position) : Statement() { } override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here") + override fun copy() = Break(position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) } @@ -258,8 +268,8 @@ open class VarDecl(val type: VarDeclType, throw IllegalArgumentException("attempt to get zero value for vardecl that shouldn't get it") } - fun copy(): VarDecl { - val c = VarDecl(type, declaredDatatype, zeropage, arraysize, name, value, isArray, autogeneratedDontRemove, sharedWithAsm, position) + override fun copy(): VarDecl { + val c = VarDecl(type, declaredDatatype, zeropage, arraysize?.copy(), name, value?.copy(), isArray, autogeneratedDontRemove, sharedWithAsm, position) c.allowInitializeWithZero = this.allowInitializeWithZero return c } @@ -301,7 +311,7 @@ class ArrayIndex(var indexExpr: Expression, fun constIndex() = (indexExpr as? NumericLiteralValue)?.number?.toInt() infix fun isSameAs(other: ArrayIndex): Boolean = indexExpr isSameAs other.indexExpr - fun copy() = ArrayIndex(indexExpr, position) + override fun copy() = ArrayIndex(indexExpr.copy(), position) } open class Assignment(var target: AssignTarget, var value: Expression, final override val position: Position) : Statement() { @@ -322,6 +332,7 @@ open class Assignment(var target: AssignTarget, var value: Expression, final ove replacement.parent = this } + override fun copy()= Assignment(target.copy(), value.copy(), position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) @@ -468,7 +479,7 @@ data class AssignTarget(var identifier: IdentifierReference?, return false } - fun copy() = AssignTarget(identifier?.copy(), arrayindexed?.copy(), memoryAddress?.copy(), position) + override fun copy() = AssignTarget(identifier?.copy(), arrayindexed?.copy(), memoryAddress?.copy(), position) } class PostIncrDecr(var target: AssignTarget, val operator: String, override val position: Position) : Statement() { @@ -485,6 +496,7 @@ class PostIncrDecr(var target: AssignTarget, val operator: String, override val replacement.parent = this } + override fun copy() = PostIncrDecr(target.copy(), operator, position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) @@ -505,6 +517,7 @@ class Jump(val address: Int?, } override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here") + override fun copy() = Jump(address, identifier?.copy(), generatedLabel, position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) @@ -525,6 +538,8 @@ class FunctionCallStatement(override var target: IdentifierReference, args.forEach { it.linkParents(this) } } + override fun copy() = throw NotImplementedError("no support for duplicating a FunctionCallStatement") + override fun replaceChildNode(node: Node, replacement: Node) { if(node===target) target = replacement as IdentifierReference @@ -538,9 +553,7 @@ class FunctionCallStatement(override var target: IdentifierReference, override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) - override fun toString(): String { - return "FunctionCallStatement(target=$target, pos=$position)" - } + override fun toString() = "FunctionCallStatement(target=$target, pos=$position)" } class InlineAssembly(val assembly: String, override val position: Position) : Statement() { @@ -550,6 +563,9 @@ class InlineAssembly(val assembly: String, override val position: Position) : St this.parent = parent } + override fun copy() = throw NotImplementedError("no support for duplicating a InlineAssembly") + + override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here") override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) @@ -571,6 +587,7 @@ class AnonymousScope(override var statements: MutableList, replacement.parent = this } + override fun copy() = AnonymousScope(statements.map { it.copy() }.toMutableList(), position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) } @@ -583,6 +600,7 @@ class NopStatement(override val position: Position): Statement() { } override fun replaceChildNode(node: Node, replacement: Node) = throw FatalAstException("can't replace here") + override fun copy() = NopStatement(position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) } @@ -636,6 +654,8 @@ class Subroutine(override val name: String, val asmGenInfo = AsmGenInfo() val scopedname: String by lazy { makeScopedName(name) } + override fun copy() = throw NotImplementedError("no support for duplicating a Subroutine") + override fun linkParents(parent: Node) { this.parent = parent parameters.forEach { it.linkParents(this) } @@ -701,6 +721,8 @@ open class SubroutineParameter(val name: String, override fun replaceChildNode(node: Node, replacement: Node) { throw FatalAstException("can't replace anything in a subroutineparameter node") } + + override fun copy() = SubroutineParameter(name, type, position) } class IfStatement(var condition: Expression, @@ -716,6 +738,8 @@ class IfStatement(var condition: Expression, elsepart.linkParents(this) } + override fun copy() = throw NotImplementedError("no support for duplicating a IfStatement") + override fun replaceChildNode(node: Node, replacement: Node) { when { node===condition -> condition = replacement as Expression @@ -743,6 +767,8 @@ class BranchStatement(var condition: BranchCondition, elsepart.linkParents(this) } + override fun copy() = throw NotImplementedError("no support for duplicating a BranchStatement") + override fun replaceChildNode(node: Node, replacement: Node) { when { node===truepart -> truepart = replacement as AnonymousScope @@ -770,6 +796,8 @@ class ForLoop(var loopVar: IdentifierReference, body.linkParents(this) } + override fun copy() = throw NotImplementedError("no support for duplicating a ForLoop") + override fun replaceChildNode(node: Node, replacement: Node) { when { node===loopVar -> loopVar = replacement as IdentifierReference @@ -801,6 +829,8 @@ class WhileLoop(var condition: Expression, body.linkParents(this) } + override fun copy() = throw NotImplementedError("no support for duplicating a WhileLoop") + override fun replaceChildNode(node: Node, replacement: Node) { when { node===condition -> condition = replacement as Expression @@ -823,6 +853,8 @@ class RepeatLoop(var iterations: Expression?, var body: AnonymousScope, override body.linkParents(this) } + override fun copy() = throw NotImplementedError("no support for duplicating a RepeatLoop") + override fun replaceChildNode(node: Node, replacement: Node) { when { node===iterations -> iterations = replacement as Expression @@ -846,6 +878,7 @@ class UntilLoop(var body: AnonymousScope, condition.linkParents(this) body.linkParents(this) } + override fun copy() = throw NotImplementedError("no support for duplicating a UntilLoop") override fun replaceChildNode(node: Node, replacement: Node) { when { @@ -871,6 +904,8 @@ class WhenStatement(var condition: Expression, choices.forEach { it.linkParents(this) } } + override fun copy() = throw NotImplementedError("no support for duplicating a WhenStatement") + override fun replaceChildNode(node: Node, replacement: Node) { if(node===condition) condition = replacement as Expression @@ -927,6 +962,7 @@ class WhenChoice(var values: MutableList?, // if null, th } } + override fun copy() = WhenChoice(values?.map{ it.copy() }?.toMutableList(), statements.copy(), position) override fun toString(): String { return "Choice($values at $position)" } @@ -955,5 +991,5 @@ class DirectMemoryWrite(var addressExpression: Expression, override val position fun accept(visitor: IAstVisitor) = visitor.visit(this) fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) - fun copy() = DirectMemoryWrite(addressExpression, position) + override fun copy() = DirectMemoryWrite(addressExpression.copy(), position) } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index d48d34f67..e1c6bed09 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -4,9 +4,6 @@ TODO For next compiler release (7.4) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ BUG: Fix C-64sound issue in petaxian (regression since 7.3, sound on c64 build works fine on older versions) -BUG: ubyte z1 - ubyte z2 - z2=z1+z2+5 CRASHES COMPILER Blocked by an official Commander-x16 v39 release @@ -17,6 +14,7 @@ Blocked by an official Commander-x16 v39 release Future ^^^^^^ +- use UByte instead of Short - simplifyConditionalExpression() should not split expression if it still results in stack-based evaluation - remove special code generation for while and util expression by rewriting while and until expressions into if+jump (consider them syntactic sugar)