diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt index 98db45f4c..5f82491c2 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt @@ -1019,10 +1019,6 @@ class AsmGen(private val program: Program, requireComparisonExpression(stmt.condition) // IfStatement: condition must be of form 'x ' val booleanCondition = stmt.condition as BinaryExpression - // DISABLED FOR NOW: -// if(!booleanCondition.left.isSimple || !booleanCondition.right.isSimple) -// throw AssemblyError("both operands for if comparison expression should have been simplified") - if (stmt.elsepart.isEmpty()) { val endLabel = makeLabel("if_end") expressionsAsmGen.translateComparisonExpressionWithJumpIfFalse(booleanCondition, endLabel) @@ -1197,6 +1193,7 @@ $repeatLabel lda $counterVar private fun translate(stmt: WhileLoop) { requireComparisonExpression(stmt.condition) // WhileLoop: condition must be of form 'x ' val booleanCondition = stmt.condition as BinaryExpression + val whileLabel = makeLabel("while") val endLabel = makeLabel("whileend") loopEndLabels.push(endLabel) @@ -1211,6 +1208,7 @@ $repeatLabel lda $counterVar private fun translate(stmt: UntilLoop) { requireComparisonExpression(stmt.condition) // UntilLoop: condition must be of form 'x ' val booleanCondition = stmt.condition as BinaryExpression + val repeatLabel = makeLabel("repeat") val endLabel = makeLabel("repeatend") loopEndLabels.push(endLabel) diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/ExpressionsAsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/ExpressionsAsmGen.kt index afde9234f..07c386677 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/ExpressionsAsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/ExpressionsAsmGen.kt @@ -30,7 +30,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge is ArrayIndexedExpression -> translateExpression(expression) is TypecastExpression -> translateExpression(expression) is AddressOf -> translateExpression(expression) - is DirectMemoryRead -> translateDirectMemReadExpression(expression, true) + is DirectMemoryRead -> translateDirectMemReadExpressionToRegAorStack(expression, true) is NumericLiteralValue -> translateExpression(expression) is IdentifierReference -> translateExpression(expression) is FunctionCall -> translateFunctionCallResultOntoStack(expression) @@ -40,6 +40,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge } } + // TODO move this function to AsmGen class so that this one only exposes the toplevel translateExpression() internal fun translateComparisonExpressionWithJumpIfFalse(expr: BinaryExpression, jumpIfFalseLabel: String) { // This is a helper routine called from while, do-util, and if expressions to generate optimized conditional branching code. // First, if it is of the form: X , then flip the expression so the constant is always the right operand. @@ -439,7 +440,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge } else if (left is DirectMemoryRead) { return if(rightConstVal.number.toInt()!=0) { - translateDirectMemReadExpression(left, false) + translateDirectMemReadExpressionToRegAorStack(left, false) code("#${rightConstVal.number}") } else @@ -663,7 +664,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge asmgen.out(" beq $jumpIfFalseLabel") } else if (left is DirectMemoryRead) { - translateDirectMemReadExpression(left, false) + translateDirectMemReadExpressionToRegAorStack(left, false) return if(rightConstVal.number.toInt()!=0) code("#${rightConstVal.number}") else @@ -828,7 +829,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge asmgen.out(" bne $jumpIfFalseLabel") } else if (left is DirectMemoryRead) { - translateDirectMemReadExpression(left, false) + translateDirectMemReadExpressionToRegAorStack(left, false) return if(rightConstVal.number.toInt()!=0) code("#${rightConstVal.number}") else @@ -996,7 +997,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge } else if (left is DirectMemoryRead) { if(rightConstVal.number.toInt()!=0) { - translateDirectMemReadExpression(left, false) + translateDirectMemReadExpressionToRegAorStack(left, false) code("#${rightConstVal.number}") } return @@ -1139,7 +1140,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge asmgen.out(" bne $jumpIfFalseLabel") } else if (left is DirectMemoryRead) { - translateDirectMemReadExpression(left, false) + translateDirectMemReadExpressionToRegAorStack(left, false) return if(rightConstVal.number.toInt()!=0) code("#${rightConstVal.number}") else @@ -1176,7 +1177,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge asmgen.out(" beq $jumpIfFalseLabel") } else if (left is DirectMemoryRead) { - translateDirectMemReadExpression(left, false) + translateDirectMemReadExpressionToRegAorStack(left, false) return if(rightConstVal.number.toInt()!=0) code("#${rightConstVal.number}") else @@ -1766,7 +1767,8 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge asmgen.out(" lda #<$name | sta P8ESTACK_LO,x | lda #>$name | sta P8ESTACK_HI,x | dex") } - internal fun translateDirectMemReadExpression(expr: DirectMemoryRead, pushResultOnEstack: Boolean) { + // TODO move this function to another class so that this one only exposes the toplevel translateExpression() + internal fun translateDirectMemReadExpressionToRegAorStack(expr: DirectMemoryRead, pushResultOnEstack: Boolean) { fun assignViaExprEval() { asmgen.assignExpressionToVariable(expr.addressExpression, "P8ZP_SCRATCH_W2", DataType.UWORD, null) diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AsmAssignment.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AsmAssignment.kt index 6c7785a51..6bccbbadc 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AsmAssignment.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AsmAssignment.kt @@ -209,7 +209,7 @@ internal class AsmAssignment(val source: AsmAssignSource, if(target.register !in arrayOf(RegisterOrPair.XY, RegisterOrPair.AX, RegisterOrPair.AY)) require(source.datatype != DataType.UNDEFINED) { "must not be placeholder/undefined datatype" } require(memsizer.memorySize(source.datatype) <= memsizer.memorySize(target.datatype)) { - "source storage size must be less or equal to target datatype storage size" + "source storage size must be less or equal to target datatype storage size at $position" } } } 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 e427a220e..e5867bf0c 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt @@ -657,14 +657,14 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, private fun inplaceModification_byte_memread_to_variable(name: String, dt: DataType, operator: String, memread: DirectMemoryRead) { when (operator) { "+" -> { - exprAsmGen.translateDirectMemReadExpression(memread, false) + exprAsmGen.translateDirectMemReadExpressionToRegAorStack(memread, false) asmgen.out(""" clc adc $name sta $name""") } "-" -> { - exprAsmGen.translateDirectMemReadExpression(memread, false) + exprAsmGen.translateDirectMemReadExpressionToRegAorStack(memread, false) asmgen.out(""" sta P8ZP_SCRATCH_B1 lda $name @@ -673,15 +673,15 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, sta $name""") } "|", "or" -> { - exprAsmGen.translateDirectMemReadExpression(memread, false) + exprAsmGen.translateDirectMemReadExpressionToRegAorStack(memread, false) asmgen.out(" ora $name | sta $name") } "&", "and" -> { - exprAsmGen.translateDirectMemReadExpression(memread, false) + exprAsmGen.translateDirectMemReadExpressionToRegAorStack(memread, false) asmgen.out(" and $name | sta $name") } "^", "xor" -> { - exprAsmGen.translateDirectMemReadExpression(memread, false) + exprAsmGen.translateDirectMemReadExpressionToRegAorStack(memread, false) asmgen.out(" eor $name | sta $name") } // TODO: tuned code for more operators @@ -694,7 +694,7 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, private fun inplaceModification_word_memread_to_variable(name: String, dt: DataType, operator: String, memread: DirectMemoryRead) { when (operator) { "+" -> { - exprAsmGen.translateDirectMemReadExpression(memread, false) + exprAsmGen.translateDirectMemReadExpressionToRegAorStack(memread, false) asmgen.out(""" clc adc $name @@ -704,7 +704,7 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, +""") } "-" -> { - exprAsmGen.translateDirectMemReadExpression(memread, false) + exprAsmGen.translateDirectMemReadExpressionToRegAorStack(memread, false) asmgen.out(""" sta P8ZP_SCRATCH_B1 lda $name @@ -716,11 +716,11 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, +""") } "|", "or" -> { - exprAsmGen.translateDirectMemReadExpression(memread, false) + exprAsmGen.translateDirectMemReadExpressionToRegAorStack(memread, false) asmgen.out(" ora $name | sta $name") } "&", "and" -> { - exprAsmGen.translateDirectMemReadExpression(memread, false) + exprAsmGen.translateDirectMemReadExpressionToRegAorStack(memread, false) asmgen.out(" and $name | sta $name") if(dt in WordDatatypes) { if(asmgen.isTargetCpu(CpuType.CPU65c02)) @@ -730,7 +730,7 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, } } "^", "xor" -> { - exprAsmGen.translateDirectMemReadExpression(memread, false) + exprAsmGen.translateDirectMemReadExpressionToRegAorStack(memread, false) asmgen.out(" eor $name | sta $name") } // TODO: tuned code for more operators diff --git a/compiler/res/prog8lib/prog8_lib.p8 b/compiler/res/prog8lib/prog8_lib.p8 index b60704943..918d041f5 100644 --- a/compiler/res/prog8lib/prog8_lib.p8 +++ b/compiler/res/prog8lib/prog8_lib.p8 @@ -6,12 +6,14 @@ prog8_lib { %asminclude "library:prog8_lib.asm" %asminclude "library:prog8_funcs.asm" - ; to store intermediary expression results for return values (hopefully allocated on ZP to reduce code size): + ; to store intermediary expression results for return values: ; NOTE: these variables are used in the StatementReorderer and StatementOptimizer uword @zp retval_interm_uw word @zp retval_interm_w ubyte @zp retval_interm_ub byte @zp retval_interm_b + word retval_interm_w2 + byte retval_interm_b2 asmsub pattern_match(str string @AY, str pattern @R0) clobbers(Y) -> ubyte @A { diff --git a/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt b/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt index 606bf92bc..288225432 100644 --- a/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt +++ b/compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt @@ -11,6 +11,7 @@ import prog8.ast.walk.AstWalker import prog8.ast.walk.IAstModification import prog8.ast.walk.IAstVisitor import prog8.compiler.astprocessing.isSubroutineParameter +import prog8.compiler.target.AssemblyError import prog8.compilerinterface.* @@ -201,12 +202,74 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o return listOf(IAstModification.ReplaceNode(ifStatement.condition, booleanExpr, ifStatement)) } - if((binExpr.operator=="==" || binExpr.operator=="!=") && - (binExpr.left as? NumericLiteralValue)?.number==0 && + if((binExpr.left as? NumericLiteralValue)?.number==0 && (binExpr.right as? NumericLiteralValue)?.number!=0) - throw InternalCompilerException("if 0==X should have been swapped to if X==0") + throw FatalAstException("0==X should have been swapped to if X==0") - return noModifications + // simplify the conditional expression, introduce simple assignments if required. + // TODO sometimes this increases code size significantly (Petaxian) !!! FIX THIS + val simplify = simplifyConditionalExpression(binExpr) + val modifications = mutableListOf() + if(simplify.rightVarAssignment!=null) { + modifications += IAstModification.ReplaceNode(binExpr.right, simplify.rightOperandReplacement!!, binExpr) + modifications += IAstModification.InsertBefore(ifStatement, simplify.rightVarAssignment, parent as IStatementContainer) + } + if(simplify.leftVarAssignment!=null) { + modifications += IAstModification.ReplaceNode(binExpr.left, simplify.leftOperandReplacement!!, binExpr) + modifications += IAstModification.InsertBefore(ifStatement, simplify.leftVarAssignment, parent as IStatementContainer) + } + + return modifications + } + + private class CondExprSimplificationResult( + val leftVarAssignment: Assignment?, + val leftOperandReplacement: Expression?, + val rightVarAssignment: Assignment?, + val rightOperandReplacement: Expression? + ) + + private fun simplifyConditionalExpression(expr: BinaryExpression): CondExprSimplificationResult { + var leftAssignment: Assignment? = null + var leftOperandReplacement: Expression? = null + var rightAssignment: Assignment? = null + var rightOperandReplacement: Expression? = null + if(!expr.left.isSimple) { + val dt = expr.left.inferType(program) + val name = when { + dt.istype(DataType.UBYTE) -> listOf("cx16","r15L") + dt.istype(DataType.UWORD) -> listOf("cx16","r15") + dt.istype(DataType.BYTE) -> listOf("prog8_lib","retval_interm_b") + dt.istype(DataType.WORD) -> listOf("prog8_lib","retval_interm_w") + else -> throw AssemblyError("invalid dt") + } + leftOperandReplacement = IdentifierReference(name, expr.position) + leftAssignment = Assignment( + AssignTarget(IdentifierReference(name, expr.position), null, null, expr.position), + expr.left, + expr.position + ) + } + if(!expr.right.isSimple) { + val dt = expr.right.inferType(program) + val name = when { + dt.istype(DataType.UBYTE) -> listOf("prog8_lib","retval_interm_ub") + dt.istype(DataType.UWORD) -> listOf("prog8_lib","retval_interm_uw") + dt.istype(DataType.BYTE) -> listOf("prog8_lib","retval_interm_b2") + dt.istype(DataType.WORD) -> listOf("prog8_lib","retval_interm_w2") + else -> throw AssemblyError("invalid dt") + } + rightOperandReplacement = IdentifierReference(name, expr.position) + rightAssignment = Assignment( + AssignTarget(IdentifierReference(name, expr.position), null, null, expr.position), + expr.right, + expr.position + ) + } + return CondExprSimplificationResult( + leftAssignment, leftOperandReplacement, + rightAssignment, rightOperandReplacement + ) } @Suppress("DuplicatedCode") @@ -224,6 +287,13 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o val booleanExpr = BinaryExpression(untilLoop.condition, "!=", NumericLiteralValue.optimalInteger(0, untilLoop.condition.position), untilLoop.condition.position) return listOf(IAstModification.ReplaceNode(untilLoop.condition, booleanExpr, untilLoop)) } + + if((binExpr.left as? NumericLiteralValue)?.number==0 && + (binExpr.right as? NumericLiteralValue)?.number!=0) + throw FatalAstException("0==X should have been swapped to if X==0") + + // TODO simplify conditional expression like in if-statement + return noModifications } @@ -242,6 +312,13 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, private val o val booleanExpr = BinaryExpression(whileLoop.condition, "!=", NumericLiteralValue.optimalInteger(0, whileLoop.condition.position), whileLoop.condition.position) return listOf(IAstModification.ReplaceNode(whileLoop.condition, booleanExpr, whileLoop)) } + + if((binExpr.left as? NumericLiteralValue)?.number==0 && + (binExpr.right as? NumericLiteralValue)?.number!=0) + throw FatalAstException("0==X should have been swapped to if X==0") + + // TODO simplify conditional expression like in if-statement + return noModifications } diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 6c1f8e8ab..80a30a5bf 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -161,7 +161,6 @@ internal class AstChecker(private val program: Program, super.visit(forLoop) } - override fun visit(jump: Jump) { val ident = jump.identifier if(ident!=null) { diff --git a/docs/source/todo.rst b/docs/source/todo.rst index d174edea2..730ec1e9b 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,7 +3,8 @@ TODO For next compiler release (7.3) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -... +- if-statement expression simplification sometimes increases code size (Petaxian) FIX THIS! +- add expression simplification to while and until loops as well. Blocked by Commander-x16 v39 release @@ -16,7 +17,6 @@ Future ^^^^^^ - fix the asm-labels problem (github issue #62) - find a way to optimize asm-subroutine param passing where it now sometimes uses the evalstack? -- [complicated?] find a way to optimize if-statement codegen so that "if var & %10000" doesn't use evalstack & subroutine call, but also that the simple case "if X {...}" remains fast - document the various compiler command line options in more detail. See "Compiling program code" in the docs - get rid of all TODO's in the code - improve testability further, add more tests diff --git a/examples/test.p8 b/examples/test.p8 index 72da2bfee..23dfe74e5 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -3,16 +3,18 @@ %zeropage basicsafe main { - uword xw - ubyte xb - - sub sub1() -> uword { - return xw+xb - } sub start() { - xw=sub1() + ubyte unused ; TODO FIX : why is this not removed as an unused variable? + + ubyte iteration_in_progress + uword num_bytes + + ubyte qq = not iteration_in_progress or not num_bytes ; TODO FIX COMPILER CRASH (STORAGE SIZE) + + if not iteration_in_progress or not num_bytes + return ; word xx=0 ; word[] xarr = [1,2,3] @@ -34,13 +36,14 @@ main { ; txt.print("xx is zero\n") ; } -; ubyte yy=$30 + ubyte yy=$30 ; ubyte zz=9 ; sys.memset(xx+200, yy*2, ~yy) ; -; if yy & %10000 { -; yy++ -; } + + if yy & %10000 { + yy++ + } ; ; @($c030) = 10 ; @(~xx) *= 2