diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt index 96bf60b25..cd8554db1 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt @@ -100,7 +100,7 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, } } - throw FatalAstException("assignment should be augmentable $binExpr") + throw FatalAstException("assignment should follow augmentable rules $binExpr") } private fun inplaceModification(target: AsmAssignTarget, operator: String, origValue: Expression) { diff --git a/codeOptimizers/src/prog8/optimizer/BinExprSplitter.kt b/codeOptimizers/src/prog8/optimizer/BinExprSplitter.kt index 099982080..b48b602bb 100644 --- a/codeOptimizers/src/prog8/optimizer/BinExprSplitter.kt +++ b/codeOptimizers/src/prog8/optimizer/BinExprSplitter.kt @@ -49,7 +49,7 @@ X = BinExpr X = LeftExpr if(assignment.target isSameAs binExpr.left || assignment.target isSameAs binExpr.right) return noModifications - if(binExpr.right.isSimple && !assignment.isAugmentable) { + if(binExpr.right.isSimple) { val firstAssign = Assignment(assignment.target.copy(), binExpr.left, binExpr.left.position) val targetExpr = assignment.target.toExpression() val augExpr = BinaryExpression(targetExpr, binExpr.operator, binExpr.right, binExpr.right.position) diff --git a/compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt b/compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt index 05f3773c2..51b3b23c2 100644 --- a/compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt +++ b/compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt @@ -97,19 +97,20 @@ internal class VariousCleanups(val program: Program, val errors: IErrorReporter) val nextAssign = assignment.nextSibling() as? Assignment if(nextAssign!=null && nextAssign.target.isSameAs(assignment.target, program)) { - // TODO hmm, if both assignments assign to the same thing, can't we just remove the first altogether??? + // TODO hmm, if both assignments assign to the same thing, can't we just remove the first altogether??? as long as there isn't a function call in the value! if(nextAssign.value isSameAs assignment.value) return listOf(IAstModification.Remove(assignment, parent as IStatementContainer)) if((assignment.value as? NumericLiteralValue)?.number==0.0 && nextAssign.isAugmentable) { val value = nextAssign.value as BinaryExpression - require(value.left isSameAs assignment.target) - val assign = Assignment(assignment.target, value.right, nextAssign.position) - return listOf( - IAstModification.Remove(assignment, parent as IStatementContainer), - IAstModification.ReplaceNode(nextAssign, assign, parent) - ) + if(value.left isSameAs assignment.target) { + val assign = Assignment(assignment.target, value.right, nextAssign.position) + return listOf( + IAstModification.Remove(assignment, parent as IStatementContainer), + IAstModification.ReplaceNode(nextAssign, assign, parent) + ) + } } } diff --git a/compiler/test/TestOptimization.kt b/compiler/test/TestOptimization.kt index 81d2fa417..68e48fbeb 100644 --- a/compiler/test/TestOptimization.kt +++ b/compiler/test/TestOptimization.kt @@ -455,4 +455,21 @@ class TestOptimization: FunSpec({ errors.errors[0] shouldContain "type of value BYTE doesn't match target UBYTE" errors.errors[1] shouldContain "value '-1' out of range for unsigned byte" } + + test("test augmented expression asmgen") { + val src = """ + main { + sub start() { + ubyte c + ubyte r + ubyte q + r = (q+r)-c + q=r + r = q+(r-c) + q=r + } + }""" + val result = compileText(C64Target, optimize=false, src, writeAssembly=true).assertSuccess() + result.program.entrypoint.statements.size shouldBe 10 + } }) diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index 97d0c6c9c..6c720d56a 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt @@ -351,6 +351,21 @@ open class Assignment(var target: AssignTarget, var value: Expression, final ove if(binExpr.left isSameAs target) return true // A = A Something + if(binExpr.operator in "+-") { + val leftBinExpr = binExpr.left as? BinaryExpression + if(leftBinExpr!=null && leftBinExpr.operator in "+-") { + // A = (A +- x) +- y + if(leftBinExpr.left isSameAs target || leftBinExpr.right isSameAs target || binExpr.right isSameAs target) + return true + } + val rightBinExpr = binExpr.right as? BinaryExpression + if(rightBinExpr!=null && rightBinExpr.operator in "+-") { + // A = y +- (A +- x) + if(rightBinExpr.left isSameAs target || rightBinExpr.right isSameAs target || binExpr.left isSameAs target) + return true + } + } + if(binExpr.operator in associativeOperators) { if (binExpr.left !is BinaryExpression && binExpr.right isSameAs target) return true // A = v A diff --git a/compilerAst/test/TestProg8Parser.kt b/compilerAst/test/TestProg8Parser.kt index 6eba78f75..42a171119 100644 --- a/compilerAst/test/TestProg8Parser.kt +++ b/compilerAst/test/TestProg8Parser.kt @@ -742,14 +742,11 @@ class TestProg8Parser: FunSpec( { r = not q ; #9 no r = (q+r)+5 ; #10 yes r = q+(r+5) ; #11 yes - r = (q+r)-5 ; #12 yes TODO FIX THIS ONE - r = q+(r-5) ; #13 yes TODO FIX THIS ONE + r = (q+r)-5 ; #12 yes + r = q+(r-5) ; #13 yes } }""") - // TODO fix augmented result for the above mentioned assignments - // TODO certain typecast expressions are also augmentable?? what does this even mean, and does the generated code make a difference??? - val module = parseModule(src) val program = Program("test", DummyFunctions, DummyMemsizer, DummyStringEncoder) program.addModule(module) @@ -766,7 +763,7 @@ class TestProg8Parser: FunSpec( { for((idx, pp) in stmts.drop(2).zip(expectedResults).withIndex()) { val assign = pp.first as Assignment val expected = pp.second - withClue("#${idx+1}: should be augmentable but isn't : $assign") { + withClue("#${idx+1}: should${if(expected) "" else "n't"} be augmentable: $assign") { assign.isAugmentable shouldBe expected assign.value shouldBe (instanceOf() or instanceOf() or instanceOf()) } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index f9ff5b63b..60a01417c 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,7 +3,9 @@ TODO For next compiler release (7.4) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -BUG: fix "assignment isAugmented correctness" test +fix "test augmented expression asmgen" unittest (and textelite compilation) + +TODO certain typecast expressions are also augmentable?? what does this even mean, and does the generated code make a difference??? optimize TODO in "Add assignment to initialize with zero" in StatementReorderer optimize TODO in after(assignment) in VariousCleanups @@ -12,6 +14,7 @@ optimize: there is an optimizations in AsmOptimizer that can only be done correc if it knows about regular ram vs io space ram distinction. + Blocked by an official Commander-x16 v39 release ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - simplify cx16.joystick_get2() once this cx16 rom issue is resolved: https://github.com/commanderx16/x16-rom/issues/203 @@ -21,6 +24,7 @@ Blocked by an official Commander-x16 v39 release Future ^^^^^^ - use UByte instead of Short +- rethink the whole "isAugmentable" business. Because the way this is determined, should always also be exactly mirrorred in the AugmentableAssignmentAsmGen or you'll get a crash at code gen time. - 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 (just consider them syntactic sugar)