diff --git a/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt b/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt index c83b00a23..e9fa04632 100644 --- a/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt +++ b/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt @@ -196,28 +196,6 @@ class ExpressionSimplifier(private val program: Program, private val errors: IEr // unsigned < 0 --> false return listOf(IAstModification.ReplaceNode(expr, NumericLiteralValue.fromBoolean(false, expr.position), parent)) } - when(leftDt) { - DataType.BYTE -> { - // signed < 0 --> signed & $80 - return listOf(IAstModification.ReplaceNode( - expr, - BinaryExpression(expr.left, "&", NumericLiteralValue.optimalInteger(0x80, expr.position), expr.position), - parent - )) - } - DataType.WORD -> { - // signedw < 0 --> msb(signedw) & $80 - return listOf(IAstModification.ReplaceNode( - expr, - BinaryExpression(FunctionCallExpression(IdentifierReference(listOf("msb"), expr.position), - mutableListOf(expr.left), - expr.position - ), "&", NumericLiteralValue.optimalInteger(0x80, expr.position), expr.position), - parent - )) - } - else -> {} - } } // simplify when a term is constant and directly determines the outcome diff --git a/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt b/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt index 7007462a9..4764cfe5c 100644 --- a/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt +++ b/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt @@ -10,14 +10,15 @@ import prog8.ast.statements.* import prog8.ast.walk.AstWalker import prog8.ast.walk.IAstModification import prog8.ast.walk.IAstVisitor -import prog8.compiler.astprocessing.isSubroutineParameter import prog8.codegen.target.AssemblyError -import prog8.compilerinterface.* +import prog8.compilerinterface.CompilationOptions +import prog8.compilerinterface.IErrorReporter +import prog8.compilerinterface.isIOAddress import prog8.optimizer.getTempVarName - internal class BeforeAsmGenerationAstChanger(val program: Program, private val options: CompilationOptions, - private val errors: IErrorReporter) : AstWalker() { + private val errors: IErrorReporter +) : AstWalker() { private val subroutineVariables = mutableMapOf>>() @@ -52,7 +53,7 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o } override fun after(decl: VarDecl, parent: Node): Iterable { - if(decl.type==VarDeclType.VAR && decl.value != null && decl.datatype in NumericDatatypes) + 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") rememberSubroutineVar(decl) return noModifications @@ -63,8 +64,8 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o // this triggers the more efficent augmented assignment code generation more often. // But it can only be done if the target variable IS NOT OCCURRING AS AN OPERAND ITSELF. if(!assignment.isAugmentable - && assignment.target.identifier != null - && !assignment.target.isIOAddress(options.compTarget.machine)) { + && assignment.target.identifier != null + && !assignment.target.isIOAddress(options.compTarget.machine)) { val binExpr = assignment.value as? BinaryExpression if(binExpr!=null && binExpr.inferType(program) istype DataType.FLOAT && !options.optimizeFloatExpressions) @@ -79,20 +80,28 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o // A = // use the other part of the expression to split. val sourceDt = binExpr.right.inferType(program).getOrElse { throw AssemblyError("unknown dt") } - val (_, right) = binExpr.right.typecastTo(assignment.target.inferType(program).getOrElse { throw AssemblyError("unknown dt") }, sourceDt, implicit=true) + val (_, right) = binExpr.right.typecastTo(assignment.target.inferType(program).getOrElse { throw AssemblyError( + "unknown dt" + ) + }, sourceDt, implicit=true) val assignRight = Assignment(assignment.target, right, assignment.position) return listOf( - IAstModification.InsertBefore(assignment, assignRight, parent as IStatementContainer), - IAstModification.ReplaceNode(binExpr.right, binExpr.left, binExpr), - IAstModification.ReplaceNode(binExpr.left, assignment.target.toExpression(), binExpr)) + IAstModification.InsertBefore(assignment, assignRight, parent as IStatementContainer), + IAstModification.ReplaceNode(binExpr.right, binExpr.left, binExpr), + IAstModification.ReplaceNode(binExpr.left, assignment.target.toExpression(), binExpr) + ) } } else { val sourceDt = binExpr.left.inferType(program).getOrElse { throw AssemblyError("unknown dt") } - val (_, left) = binExpr.left.typecastTo(assignment.target.inferType(program).getOrElse { throw AssemblyError("unknown dt") }, sourceDt, implicit=true) + val (_, left) = binExpr.left.typecastTo(assignment.target.inferType(program).getOrElse { throw AssemblyError( + "unknown dt" + ) + }, sourceDt, implicit=true) val assignLeft = Assignment(assignment.target, left, assignment.position) return listOf( - IAstModification.InsertBefore(assignment, assignLeft, parent as IStatementContainer), - IAstModification.ReplaceNode(binExpr.left, assignment.target.toExpression(), binExpr)) + IAstModification.InsertBefore(assignment, assignLeft, parent as IStatementContainer), + IAstModification.ReplaceNode(binExpr.left, assignment.target.toExpression(), binExpr) + ) } } } @@ -126,9 +135,9 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o if (subroutine.asmAddress == null && !subroutine.inline) { if(subroutine.statements.isEmpty() || (subroutine.amountOfRtsInAsm() == 0 - && subroutine.statements.lastOrNull { it !is VarDecl } !is Return - && subroutine.statements.last() !is Subroutine)) { - mods += IAstModification.InsertLast(returnStmt, subroutine) + && subroutine.statements.lastOrNull { it !is VarDecl } !is Return + && subroutine.statements.last() !is Subroutine)) { + mods += IAstModification.InsertLast(returnStmt, subroutine) } } @@ -138,73 +147,39 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o val subroutineStmtIdx = outerStatements.indexOf(subroutine) if (subroutineStmtIdx > 0) { val prevStmt = outerStatements[subroutineStmtIdx-1] - if(outerScope !is Block - && (prevStmt !is Jump) - && prevStmt !is Subroutine - && prevStmt !is Return) { + if(outerScope !is Block + && (prevStmt !is Jump) + && prevStmt !is Subroutine + && prevStmt !is Return + ) { mods += IAstModification.InsertAfter(outerStatements[subroutineStmtIdx - 1], returnStmt, outerScope) } } return mods } - override fun after(typecast: TypecastExpression, parent: Node): Iterable { - // see if we can remove redundant typecasts (outside of expressions) - // such as casting byte<->ubyte, word<->uword - // Also the special typecast of a reference type (str, array) to an UWORD will be changed into address-of, - // UNLESS it's a str parameter in the containing subroutine - then we remove the typecast altogether - val sourceDt = typecast.expression.inferType(program).getOr(DataType.UNDEFINED) - if (typecast.type in ByteDatatypes && sourceDt in ByteDatatypes - || typecast.type in WordDatatypes && sourceDt in WordDatatypes) { - if(typecast.parent !is Expression) { - return listOf(IAstModification.ReplaceNode(typecast, typecast.expression, parent)) - } - } - - if(sourceDt in PassByReferenceDatatypes) { - if(typecast.type==DataType.UWORD) { - val identifier = typecast.expression as? IdentifierReference - if(identifier!=null) { - return if(identifier.isSubroutineParameter(program)) { - listOf(IAstModification.ReplaceNode( - typecast, - typecast.expression, - parent - )) - } else { - listOf(IAstModification.ReplaceNode( - typecast, - AddressOf(identifier, typecast.position), - parent - )) - } - } else if(typecast.expression is IFunctionCall) { - return listOf(IAstModification.ReplaceNode( - typecast, - typecast.expression, - parent - )) - } - } else { - errors.err("cannot cast pass-by-reference value to type ${typecast.type} (only to UWORD)", typecast.position) - } - } - - return noModifications - } - override fun after(ifElse: IfElse, parent: Node): Iterable { val prefixExpr = ifElse.condition as? PrefixExpression if(prefixExpr!=null && prefixExpr.operator=="not") { // if not x -> if x==0 - val booleanExpr = BinaryExpression(prefixExpr.expression, "==", NumericLiteralValue.optimalInteger(0, ifElse.condition.position), ifElse.condition.position) + val booleanExpr = BinaryExpression( + prefixExpr.expression, + "==", + NumericLiteralValue.optimalInteger(0, ifElse.condition.position), + ifElse.condition.position + ) return listOf(IAstModification.ReplaceNode(ifElse.condition, booleanExpr, ifElse)) } val binExpr = ifElse.condition as? BinaryExpression if(binExpr==null || binExpr.operator !in ComparisonOperators) { // if x -> if x!=0, if x+5 -> if x+5 != 0 - val booleanExpr = BinaryExpression(ifElse.condition, "!=", NumericLiteralValue.optimalInteger(0, ifElse.condition.position), ifElse.condition.position) + val booleanExpr = BinaryExpression( + ifElse.condition, + "!=", + NumericLiteralValue.optimalInteger(0, ifElse.condition.position), + ifElse.condition.position + ) return listOf(IAstModification.ReplaceNode(ifElse.condition, booleanExpr, ifElse)) } @@ -219,11 +194,19 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o val modifications = mutableListOf() if(simplify.rightVarAssignment!=null) { modifications += IAstModification.ReplaceNode(binExpr.right, simplify.rightOperandReplacement!!, binExpr) - modifications += IAstModification.InsertBefore(ifElse, simplify.rightVarAssignment, parent as IStatementContainer) + modifications += IAstModification.InsertBefore( + ifElse, + simplify.rightVarAssignment, + parent as IStatementContainer + ) } if(simplify.leftVarAssignment!=null) { modifications += IAstModification.ReplaceNode(binExpr.left, simplify.leftOperandReplacement!!, binExpr) - modifications += IAstModification.InsertBefore(ifElse, simplify.leftVarAssignment, parent as IStatementContainer) + modifications += IAstModification.InsertBefore( + ifElse, + simplify.leftVarAssignment, + parent as IStatementContainer + ) } return modifications @@ -299,13 +282,13 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o if(dt1 in ByteDatatypes) { if(dt2 in ByteDatatypes) return noModifications - val (replaced, cast) = arg1.typecastTo(if(dt1==DataType.UBYTE) DataType.UWORD else DataType.WORD, dt1, true) + val (replaced, cast) = arg1.typecastTo(if(dt1== DataType.UBYTE) DataType.UWORD else DataType.WORD, dt1, true) if(replaced) return listOf(IAstModification.ReplaceNode(arg1, cast, functionCallStatement)) } else { if(dt2 in WordDatatypes) return noModifications - val (replaced, cast) = arg2.typecastTo(if(dt2==DataType.UBYTE) DataType.UWORD else DataType.WORD, dt2, true) + val (replaced, cast) = arg2.typecastTo(if(dt2== DataType.UBYTE) DataType.UWORD else DataType.WORD, dt2, true) if(replaced) return listOf(IAstModification.ReplaceNode(arg2, cast, functionCallStatement)) } @@ -372,11 +355,18 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o val statement = expr.containingStatement val dt = expr.indexer.indexExpr.inferType(program) val tempvar = if(dt.isBytes) listOf("prog8_lib","retval_interm_ub") else listOf("prog8_lib","retval_interm_b") - val target = AssignTarget(IdentifierReference(tempvar, expr.indexer.position), null, null, expr.indexer.position) + val target = + AssignTarget(IdentifierReference(tempvar, expr.indexer.position), null, null, expr.indexer.position) val assign = Assignment(target, expr.indexer.indexExpr, expr.indexer.position) modifications.add(IAstModification.InsertBefore(statement, assign, statement.parent as IStatementContainer)) - modifications.add(IAstModification.ReplaceNode(expr.indexer.indexExpr, target.identifier!!.copy(), expr.indexer)) + modifications.add( + IAstModification.ReplaceNode( + expr.indexer.indexExpr, + target.identifier!!.copy(), + expr.indexer + ) + ) return modifications } -} +} \ No newline at end of file diff --git a/compiler/src/prog8/compiler/BeforeAsmTypecastCleaner.kt b/compiler/src/prog8/compiler/BeforeAsmTypecastCleaner.kt new file mode 100644 index 000000000..99e4c671f --- /dev/null +++ b/compiler/src/prog8/compiler/BeforeAsmTypecastCleaner.kt @@ -0,0 +1,77 @@ +package prog8.compiler + +import prog8.ast.IFunctionCall +import prog8.ast.Node +import prog8.ast.Program +import prog8.ast.base.ByteDatatypes +import prog8.ast.base.DataType +import prog8.ast.base.PassByReferenceDatatypes +import prog8.ast.base.WordDatatypes +import prog8.ast.expressions.AddressOf +import prog8.ast.expressions.Expression +import prog8.ast.expressions.IdentifierReference +import prog8.ast.expressions.TypecastExpression +import prog8.ast.walk.AstWalker +import prog8.ast.walk.IAstModification +import prog8.compiler.astprocessing.isSubroutineParameter +import prog8.compilerinterface.IErrorReporter + +internal class BeforeAsmTypecastCleaner(val program: Program, + private val errors: IErrorReporter +) : AstWalker() { + + override fun after(typecast: TypecastExpression, parent: Node): Iterable { + // see if we can remove redundant typecasts (outside of expressions) + // such as casting byte<->ubyte, word<->uword or even redundant casts (sourcetype = target type). + // Also the special typecast of a reference type (str, array) to an UWORD will be changed into address-of, + // UNLESS it's a str parameter in the containing subroutine - then we remove the typecast altogether + val sourceDt = typecast.expression.inferType(program).getOr(DataType.UNDEFINED) + if (typecast.type in ByteDatatypes && sourceDt in ByteDatatypes + || typecast.type in WordDatatypes && sourceDt in WordDatatypes + ) { + if(typecast.parent !is Expression) { + return listOf(IAstModification.ReplaceNode(typecast, typecast.expression, parent)) + } + } + + if(typecast.type==sourceDt) + return listOf(IAstModification.ReplaceNode(typecast, typecast.expression, parent)) + + if(sourceDt in PassByReferenceDatatypes) { + if(typecast.type== DataType.UWORD) { + val identifier = typecast.expression as? IdentifierReference + if(identifier!=null) { + return if(identifier.isSubroutineParameter(program)) { + listOf( + IAstModification.ReplaceNode( + typecast, + typecast.expression, + parent + ) + ) + } else { + listOf( + IAstModification.ReplaceNode( + typecast, + AddressOf(identifier, typecast.position), + parent + ) + ) + } + } else if(typecast.expression is IFunctionCall) { + return listOf( + IAstModification.ReplaceNode( + typecast, + typecast.expression, + parent + ) + ) + } + } else { + errors.err("cannot cast pass-by-reference value to type ${typecast.type} (only to UWORD)", typecast.position) + } + } + + return noModifications + } +} \ No newline at end of file diff --git a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt index 8e2eab4cf..42cb9145a 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt @@ -10,6 +10,7 @@ import prog8.ast.statements.Directive import prog8.ast.walk.AstWalker import prog8.ast.walk.IAstModification import prog8.compiler.BeforeAsmGenerationAstChanger +import prog8.compiler.BeforeAsmTypecastCleaner import prog8.compilerinterface.CompilationOptions import prog8.compilerinterface.IErrorReporter import prog8.compilerinterface.IStringEncoding @@ -28,6 +29,11 @@ internal fun Program.processAstBeforeAsmGeneration(compilerOptions: CompilationO while(errors.noErrors() && fixer.applyModifications()>0) { fixer.visit(this) } + val cleaner = BeforeAsmTypecastCleaner(this, errors) + cleaner.visit(this) + while(errors.noErrors() && cleaner.applyModifications()>0) { + cleaner.visit(this) + } } internal fun Program.reorderStatements(errors: IErrorReporter, options: CompilationOptions) { diff --git a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt index 1dc1878d2..618b0ff53 100644 --- a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt +++ b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt @@ -64,8 +64,30 @@ class TypecastsAdder(val program: Program, val options: CompilationOptions, val NumericLiteralValue(leftDt.getOr(DataType.UNDEFINED), value, expr.right.position), expr)) } + + if(leftDt istype DataType.BYTE && rightDt.oneOf(DataType.UBYTE, DataType.UWORD)) { + // cast left to unsigned + val cast = TypecastExpression(expr.left, rightDt.getOr(DataType.UNDEFINED), true, expr.left.position) + return listOf(IAstModification.ReplaceNode(expr.left, cast, expr)) + } + if(leftDt istype DataType.WORD && rightDt.oneOf(DataType.UBYTE, DataType.UWORD)) { + // cast left to unsigned + val cast = TypecastExpression(expr.left, rightDt.getOr(DataType.UNDEFINED), true, expr.left.position) + return listOf(IAstModification.ReplaceNode(expr.left, cast, expr)) + } + if(rightDt istype DataType.BYTE && leftDt.oneOf(DataType.UBYTE, DataType.UWORD)) { + // cast right to unsigned + val cast = TypecastExpression(expr.right, leftDt.getOr(DataType.UNDEFINED), true, expr.right.position) + return listOf(IAstModification.ReplaceNode(expr.right, cast, expr)) + } + if(rightDt istype DataType.WORD && leftDt.oneOf(DataType.UBYTE, DataType.UWORD)) { + // cast right to unsigned + val cast = TypecastExpression(expr.right, leftDt.getOr(DataType.UNDEFINED), true, expr.right.position) + return listOf(IAstModification.ReplaceNode(expr.right, cast, expr)) + } } + // determine common datatype and add typecast as required to make left and right equal types val (commonDt, toFix) = BinaryExpression.commonDatatype(leftDt.getOr(DataType.UNDEFINED), rightDt.getOr(DataType.UNDEFINED), expr.left, expr.operator, expr.right) if(toFix!=null) { diff --git a/compiler/test/TestOptimization.kt b/compiler/test/TestOptimization.kt index 4d32f1f76..d90d7708f 100644 --- a/compiler/test/TestOptimization.kt +++ b/compiler/test/TestOptimization.kt @@ -14,9 +14,10 @@ import prog8.ast.base.ParentSentinel import prog8.ast.base.Position import prog8.ast.expressions.* import prog8.ast.statements.* -import prog8.compiler.BeforeAsmGenerationAstChanger import prog8.compiler.printProgram import prog8.codegen.target.C64Target +import prog8.compiler.BeforeAsmGenerationAstChanger +import prog8.compiler.BeforeAsmTypecastCleaner import prog8.compilerinterface.* import prog8tests.helpers.* import prog8tests.helpers.DummyFunctions @@ -304,16 +305,16 @@ class TestOptimization: FunSpec({ expr.inferType(result.program).getOrElse { fail("dt") } shouldBe DataType.UBYTE val options = CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.DONTUSE, emptyList(), false, true, C64Target) - val changer = BeforeAsmGenerationAstChanger(result.program, - options, - ErrorReporterForTests() - ) - + val changer = BeforeAsmGenerationAstChanger(result.program, options, ErrorReporterForTests()) changer.visit(result.program) while(changer.applyModifications()>0) { changer.visit(result.program) } - + val cleaner = BeforeAsmTypecastCleaner(result.program, ErrorReporterForTests()) + cleaner.visit(result.program) + while(cleaner.applyModifications()>0) { + cleaner.visit(result.program) + } // assignment is now split into: // bb = not bb // bb = (bb or not ww) diff --git a/compiler/test/TestTypecasts.kt b/compiler/test/TestTypecasts.kt index 68a580c4b..79428e661 100644 --- a/compiler/test/TestTypecasts.kt +++ b/compiler/test/TestTypecasts.kt @@ -4,6 +4,7 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.kotest.matchers.string.shouldContain import prog8.codegen.target.C64Target +import prog8.compiler.printProgram import prog8tests.helpers.ErrorReporterForTests import prog8tests.helpers.assertFailure import prog8tests.helpers.assertSuccess @@ -107,4 +108,81 @@ class TestTypecasts: FunSpec({ errors.errors[0] shouldContain "can't cast" errors.errors[1] shouldContain "can't cast" } + + test("correct implicit casts of signed number comparison and logical expressions") { + val text = """ + %import floats + + main { + sub start() { + byte bb = -10 + word ww = -1000 + + if bb>0 { + bb++ + } + if bb < 0 { + bb ++ + } + if bb & 1 { + bb++ + } + if bb & 128 { + bb++ + } + if bb & 255 { + bb++ + } + + if ww>0 { + ww++ + } + if ww < 0 { + ww ++ + } + if ww & 1 { + ww++ + } + if ww & 32768 { + ww++ + } + if ww & 65535 { + ww++ + } + } + } + """ + val result = compileText(C64Target, false, text, writeAssembly = true).assertSuccess() + printProgram(result.program) + val statements = result.program.entrypoint.statements + statements.size shouldBe 27 + } + + test("cast to unsigned in conditional") { + val text = """ + main { + sub start() { + byte bb + word ww + + ubyte iteration_in_progress + uword num_bytes + + if not iteration_in_progress or not num_bytes { + num_bytes++ + } + + if bb as ubyte { + bb++ + } + if ww as uword { + ww++ + } + } + }""" + val result = compileText(C64Target, false, text, writeAssembly = true).assertSuccess() + printProgram(result.program) + val statements = result.program.entrypoint.statements + statements.size shouldBe 16 + } }) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index bd4bef585..ff02f988e 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,7 +3,8 @@ TODO For next compiler release (7.7) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- make pipe statement also an expression so that it can return a result value +- fix codegen bug for pipe expressions to actually return correct value and not corrupt X register +- why is wormfood 40 bytes larger now since 7.6??? - optimize codegen of pipe operator to avoid needless assigns to temp var - copying floats around: do it with a subroutine rather than 5 lda/sta pairs . is slower but floats are very slow already anyway and this should take a lot less program size. @@ -25,6 +26,7 @@ Blocked by an official Commander-x16 r39 release Future Things and Ideas ^^^^^^^^^^^^^^^^^^^^^^^ - can we promise a left-to-right function call argument evaluation? without sacrificing performance +- unify FunctioncallExpression + FunctioncallStatement and PipeExpression + Pipe statement, may require moving Expression/Statement into interfaces instead of abstract base classes - for the pipe operator: recognise a placeholder (``?`` or ``%`` or ``_``) in a non-unary function call to allow things as ``4 |> mkword(?, $44) |> print_uw`` - make it possible to use cpu opcodes such as 'nop' as variable names by prefixing all asm vars with something such as ``v_`` then we can get rid of the instruction lists in the machinedefinitions as well?