From fe175663701194070efceeae8b104290c34e5964 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 31 Oct 2021 02:34:17 +0100 Subject: [PATCH] improved reporting of slow stack based evaluation code --- .../compiler/target/cpu6502/codegen/AsmGen.kt | 3 - .../cpu6502/codegen/ExpressionsAsmGen.kt | 47 +++++++------ .../cpu6502/codegen/FunctionCallAsmGen.kt | 3 +- .../codegen/assignment/AssignmentAsmGen.kt | 33 ++++------ compiler/src/prog8/compiler/Compiler.kt | 3 +- compiler/src/prog8/compiler/ErrorReporter.kt | 14 ++-- docs/source/todo.rst | 3 +- examples/test.p8 | 66 ++++++++----------- 8 files changed, 82 insertions(+), 90 deletions(-) diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt index f780497e9..79ba9fd67 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt @@ -838,9 +838,6 @@ class AsmGen(private val program: Program, internal fun translateExpression(expression: Expression) = expressionsAsmGen.translateExpression(expression) - internal fun translateExpression(indexer: ArrayIndex) = - expressionsAsmGen.translateExpression(indexer) - internal fun translateBuiltinFunctionCallExpression(functionCall: FunctionCall, signature: FSignature, resultToStack: Boolean, resultRegister: RegisterOrPair?) = builtinFunctionsAsmGen.translateFunctioncallExpression(functionCall, signature, resultToStack, resultRegister) diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/ExpressionsAsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/ExpressionsAsmGen.kt index 04309a8d8..3a07f6b25 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/ExpressionsAsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/ExpressionsAsmGen.kt @@ -3,7 +3,6 @@ package prog8.compiler.target.cpu6502.codegen import prog8.ast.Program import prog8.ast.base.* import prog8.ast.expressions.* -import prog8.ast.statements.ArrayIndex import prog8.ast.statements.BuiltinFunctionStatementPlaceholder import prog8.ast.statements.Subroutine import prog8.ast.toHex @@ -15,7 +14,15 @@ import kotlin.math.absoluteValue internal class ExpressionsAsmGen(private val program: Program, private val asmgen: AsmGen) { - internal fun translateExpression(expression: Expression) { + internal fun translateExpression(expression:Expression) { + if (this.asmgen.options.slowCodegenWarnings) { + asmgen.errors.warn("slow stack evaluation used for expression $expression", expression.position) + } + translateExpressionInternal(expression) + } + + private fun translateExpressionInternal(expression: Expression) { + when(expression) { is PrefixExpression -> translateExpression(expression) is BinaryExpression -> translateExpression(expression) @@ -1690,7 +1697,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge } private fun translateExpression(typecast: TypecastExpression) { - translateExpression(typecast.expression) + translateExpressionInternal(typecast.expression) when(typecast.expression.inferType(program).getOr(DataType.UNDEFINED)) { DataType.UBYTE -> { when(typecast.type) { @@ -1856,7 +1863,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge val leftVal = expr.left.constValue(program)?.number?.toInt() val rightVal = expr.right.constValue(program)?.number?.toInt() if (leftVal!=null && leftVal in -4..4) { - translateExpression(expr.right) + translateExpressionInternal(expr.right) if(rightDt in ByteDatatypes) { val incdec = if(leftVal<0) "dec" else "inc" repeat(leftVal.absoluteValue) { @@ -1886,7 +1893,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge } else if (rightVal!=null && rightVal in -4..4) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) if(leftDt in ByteDatatypes) { val incdec = if(rightVal<0) "dec" else "inc" repeat(rightVal.absoluteValue) { @@ -1921,7 +1928,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge val rightVal = expr.right.constValue(program)?.number?.toInt() if (rightVal!=null && rightVal in -4..4) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) if(leftDt in ByteDatatypes) { val incdec = if(rightVal<0) "inc" else "dec" repeat(rightVal.absoluteValue) { @@ -1954,7 +1961,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge ">>" -> { val amount = expr.right.constValue(program)?.number?.toInt() if(amount!=null) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) when (leftDt) { DataType.UBYTE -> { if (amount <= 2) @@ -2025,7 +2032,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge "<<" -> { val amount = expr.right.constValue(program)?.number?.toInt() if(amount!=null) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) if (leftDt in ByteDatatypes) { if (amount <= 2) repeat(amount) { asmgen.out(" asl P8ESTACK_LO+1,x") } @@ -2062,7 +2069,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge val amount = value.number.toInt() if(amount==2) { // optimize x*2 common case - translateExpression(expr.left) + translateExpressionInternal(expr.left) if(leftDt in ByteDatatypes) { asmgen.out(" asl P8ESTACK_LO+1,x") } else { @@ -2073,38 +2080,38 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge when(rightDt) { DataType.UBYTE -> { if(amount in asmgen.optimizedByteMultiplications) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) asmgen.out(" jsr math.stack_mul_byte_$amount") return } } DataType.BYTE -> { if(amount in asmgen.optimizedByteMultiplications) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) asmgen.out(" jsr math.stack_mul_byte_$amount") return } if(amount.absoluteValue in asmgen.optimizedByteMultiplications) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) asmgen.out(" jsr prog8_lib.neg_b | jsr math.stack_mul_byte_${amount.absoluteValue}") return } } DataType.UWORD -> { if(amount in asmgen.optimizedWordMultiplications) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) asmgen.out(" jsr math.stack_mul_word_$amount") return } } DataType.WORD -> { if(amount in asmgen.optimizedWordMultiplications) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) asmgen.out(" jsr math.stack_mul_word_$amount") return } if(amount.absoluteValue in asmgen.optimizedWordMultiplications) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) asmgen.out(" jsr prog8_lib.neg_w | jsr math.stack_mul_word_${amount.absoluteValue}") return } @@ -2118,7 +2125,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge if(leftDt in IntegerDatatypes && rightDt in IntegerDatatypes) { val rightVal = expr.right.constValue(program)?.number?.toInt() if(rightVal!=null && rightVal==2) { - translateExpression(expr.left) + translateExpressionInternal(expr.left) when(leftDt) { DataType.UBYTE -> asmgen.out(" lsr P8ESTACK_LO+1,x") DataType.BYTE -> asmgen.out(" asl P8ESTACK_LO+1,x | ror P8ESTACK_LO+1,x") @@ -2141,8 +2148,8 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge } else { // the general, non-optimized cases TODO optimize more cases.... - translateExpression(expr.left) - translateExpression(expr.right) + translateExpressionInternal(expr.left) + translateExpressionInternal(expr.right) when (leftDt) { in ByteDatatypes -> translateBinaryOperatorBytes(expr.operator, leftDt) in WordDatatypes -> translateBinaryOperatorWords(expr.operator, leftDt) @@ -2169,7 +2176,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge } private fun translateExpression(expr: PrefixExpression) { - translateExpression(expr.expression) + translateExpressionInternal(expr.expression) val itype = expr.inferType(program) if(!itype.isKnown) throw AssemblyError("unknown dt") @@ -2254,8 +2261,6 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge } } - fun translateExpression(indexer: ArrayIndex) = asmgen.translateExpression(indexer.indexExpr) - private fun translateBinaryOperatorBytes(operator: String, types: DataType) { when(operator) { "**" -> throw AssemblyError("** operator requires floats") diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt index cfed3a885..db286a136 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt @@ -64,7 +64,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg argumentViaVariable(sub, arg.first, arg.second) } } else { - // via registers + require(sub.isAsmSubroutine) if(sub.parameters.size==1) { // just a single parameter, no risk of clobbering registers argumentViaRegister(sub, IndexedValue(0, sub.parameters.single()), stmt.args[0]) @@ -104,6 +104,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg } else -> { // Risk of clobbering due to complex expression args. Evaluate first, then assign registers. + // TODO find another way to prepare the arguments, without using the eval stack registerArgsViaStackEvaluation(stmt, sub) } } diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AssignmentAsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AssignmentAsmGen.kt index b82284c32..a24fe73ac 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AssignmentAsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AssignmentAsmGen.kt @@ -21,7 +21,7 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen fun translate(assignment: Assignment) { val target = AsmAssignTarget.fromAstAssignment(assignment, program, asmgen) - val source = AsmAssignSource.Companion.fromAstSource(assignment.value, program, asmgen).adjustSignedUnsigned(target) + val source = AsmAssignSource.fromAstSource(assignment.value, program, asmgen).adjustSignedUnsigned(target) val assign = AsmAssignment(source, target, assignment.isAugmentable, program.memsizer, assignment.position) target.origAssign = assign @@ -171,13 +171,13 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen DataType.STR, DataType.ARRAY_UB, DataType.ARRAY_B -> { // copy the actual string result into the target string variable asmgen.out(""" - pha - lda #<${assign.target.asmVarname} - sta P8ZP_SCRATCH_W1 - lda #>${assign.target.asmVarname} - sta P8ZP_SCRATCH_W1+1 - pla - jsr prog8_lib.strcpy""") + pha + lda #<${assign.target.asmVarname} + sta P8ZP_SCRATCH_W1 + lda #>${assign.target.asmVarname} + sta P8ZP_SCRATCH_W1+1 + pla + jsr prog8_lib.strcpy""") } else -> throw AssemblyError("weird target dt") } @@ -251,12 +251,9 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen } else -> { // Everything else just evaluate via the stack. - // (we can't use the assignment helper functions to do it via registers here, + // (we can't use the assignment helper functions (assignExpressionTo...) to do it via registers here, // because the code here is the implementation of exactly that...) - if (value.parent is Return) { - if (this.asmgen.options.slowCodegenWarnings) - println("warning: slow stack evaluation used for return: $value target=${assign.target.kind} at ${value.position}") - } + // TODO FIX THIS... by using a temp var? so that it becomes augmentable assignment expression? exprAsmgen.translateExpression(value) if (assign.target.datatype in WordDatatypes && assign.source.datatype in ByteDatatypes) asmgen.signExtendStackLsb(assign.source.datatype) @@ -373,7 +370,7 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen } DataType.FLOAT -> { assignExpressionToRegister(value, RegisterOrPair.FAC1) - assignTypecastedFloatFAC1(target.asmVarname, targetDt) + assignTypeCastedFloatFAC1(target.asmVarname, targetDt) } in PassByReferenceDatatypes -> { // str/array value cast (most likely to UWORD, take address-of) @@ -433,8 +430,6 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen // give up, do it via eval stack // note: cannot use assignTypeCastedValue because that is ourselves :P // TODO optimize typecasts for more special cases? - if(this.asmgen.options.slowCodegenWarnings) - println("warning: slow stack evaluation used for typecast: $value into $targetDt (target=${target.kind} at ${value.position}") asmgen.translateExpression(origTypeCastExpression) // this performs the actual type cast in translateExpression(Typecast) assignStackValue(target) } @@ -447,7 +442,7 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen translateNormalAssignment(assign) } - private fun assignTypecastedFloatFAC1(targetAsmVarName: String, targetDt: DataType) { + private fun assignTypeCastedFloatFAC1(targetAsmVarName: String, targetDt: DataType) { if(targetDt==DataType.FLOAT) throw AssemblyError("typecast to identical type") @@ -2158,14 +2153,14 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen } internal fun assignExpressionToRegister(expr: Expression, register: RegisterOrPair) { - val src = AsmAssignSource.Companion.fromAstSource(expr, program, asmgen) + val src = AsmAssignSource.fromAstSource(expr, program, asmgen) val tgt = AsmAssignTarget.fromRegisters(register, null, program, asmgen) val assign = AsmAssignment(src, tgt, false, program.memsizer, expr.position) translateNormalAssignment(assign) } internal fun assignExpressionToVariable(expr: Expression, asmVarName: String, dt: DataType, scope: Subroutine?) { - val src = AsmAssignSource.Companion.fromAstSource(expr, program, asmgen) + val src = AsmAssignSource.fromAstSource(expr, program, asmgen) val tgt = AsmAssignTarget(TargetStorageKind.VARIABLE, program, asmgen, dt, scope, variableAsmName = asmVarName) val assign = AsmAssignment(src, tgt, false, program.memsizer, expr.position) translateNormalAssignment(assign) diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt index 1f6a83193..066ac3497 100644 --- a/compiler/src/prog8/compiler/Compiler.kt +++ b/compiler/src/prog8/compiler/Compiler.kt @@ -327,17 +327,16 @@ private fun writeAssembly(program: Program, compilerOptions.compTarget.machine.zeropage, compilerOptions, outputDir).compileToAssembly() + errors.report() return if(assembly.valid && errors.noErrors()) { val assemblerReturnStatus = assembly.assemble(quietAssembler, compilerOptions) if(assemblerReturnStatus!=0) WriteAssemblyResult.Fail("assembler step failed with return code $assemblerReturnStatus") else { - errors.report() WriteAssemblyResult.Ok(assembly.name) } } else { - errors.report() WriteAssemblyResult.Fail("compiler failed with errors") } } diff --git a/compiler/src/prog8/compiler/ErrorReporter.kt b/compiler/src/prog8/compiler/ErrorReporter.kt index ce91f17e7..fdd893232 100644 --- a/compiler/src/prog8/compiler/ErrorReporter.kt +++ b/compiler/src/prog8/compiler/ErrorReporter.kt @@ -24,21 +24,27 @@ internal class ErrorReporter: IErrorReporter { var numErrors = 0 var numWarnings = 0 messages.forEach { + val printer = when(it.severity) { + MessageSeverity.WARNING -> System.out + MessageSeverity.ERROR -> System.err + } when(it.severity) { - MessageSeverity.ERROR -> System.err.print("\u001b[91m") // bright red - MessageSeverity.WARNING -> System.err.print("\u001b[93m") // bright yellow + MessageSeverity.ERROR -> printer.print("\u001b[91m") // bright red + MessageSeverity.WARNING -> printer.print("\u001b[93m") // bright yellow } val msg = "${it.position.toClickableStr()} ${it.severity} ${it.message}".trim() if(msg !in alreadyReportedMessages) { - System.err.println(msg) + printer.println(msg) alreadyReportedMessages.add(msg) when(it.severity) { MessageSeverity.WARNING -> numWarnings++ MessageSeverity.ERROR -> numErrors++ } } - System.err.print("\u001b[0m") // reset color + printer.print("\u001b[0m") // reset color } + System.out.flush() + System.err.flush() messages.clear() finalizeNumErrors(numErrors, numWarnings) } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index b35d8edd2..20b73e8cf 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -4,7 +4,7 @@ TODO For next compiler release (7.2) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - fix the asm-labels problem (github issue #62) -- find a way to optimize if-statement codegen so that "if var & %10000" doesn't use stack & subroutine call, but also that the simple case "if X {...}" remains fast +- find a way to optimize asm-subroutine param passing where it now sometimes uses the evalstack? Blocked by Commander-x16 v39 release @@ -20,6 +20,7 @@ Future - replace certain uses of inferredType.getOr(DataType.UNDEFINED) by i.getOrElse({ errorhandler }) - see if we can remove more "[InferredType].getOr(DataType.UNDEFINED)" - use more of Result<> and Either<> to handle errors/ nulls better +- [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 - can we get rid of pieces of asmgen.AssignmentAsmGen by just reusing the AugmentableAssignment ? generated code should not suffer - c64: make the graphics.BITMAP_ADDRESS configurable (VIC banking) - optimize several inner loops in gfx2 even further? diff --git a/examples/test.p8 b/examples/test.p8 index 203e1b84f..90e3c76a2 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -4,52 +4,40 @@ main { sub start() { - word xx - - xx=-$7f00 - - txt.print_w(xx) - txt.nl() - if xx>=0 { - txt.print(">=0\n") - } else { - txt.print("<0\n") - } - if xx<=0 { - txt.print("<=0\n") - } else { - txt.print(">0\n") - } - - return + ubyte xx = 100 + ubyte cv - if xx { ; doesn't use stack... - xx++ - } + sys.memset($1000+xx, 10, 255) ; TODO uses stack eval now to precalc parameters - xx = xx+1 ; doesn't use stack... + xx = xx & %0001 ; doesn't use stack... because it uses AugmentableAssignmentAsmGen + ;yy = xx & %0001 ; doesn't use stack... because it uses AugmentableAssignmentAsmGen - if 8