From 7aa807ec7f06ae6bbde36bc8f8b6438136aae1c8 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 15 Feb 2022 23:16:49 +0100 Subject: [PATCH] proper error if attempting to do a containment check against non const range, and some cleanup in asmgen --- .../codegen/cpu6502/FunctionCallAsmGen.kt | 14 +------ .../cpu6502/assignment/AsmAssignment.kt | 2 +- .../cpu6502/assignment/AssignmentAsmGen.kt | 27 +++++++------- .../optimizer/ConstantFoldingOptimizer.kt | 2 +- .../prog8/optimizer/ExpressionSimplifier.kt | 8 +--- compiler/src/prog8/compiler/Compiler.kt | 9 ----- .../compiler/astprocessing/AstChecker.kt | 4 ++ compiler/test/ast/TestProg8Parser.kt | 1 - docs/source/todo.rst | 3 +- examples/test.p8 | 37 ++++++++++++++----- 10 files changed, 51 insertions(+), 56 deletions(-) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt index 88a8514f7..8208821d7 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt @@ -210,25 +210,15 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg private fun registerArgsViaStackEvaluation(stmt: IFunctionCall, sub: Subroutine) { // this is called when one or more of the arguments are 'complex' and // cannot be assigned to a register easily or risk clobbering other registers. - // TODO find another way to prepare the arguments, without using the eval stack + // TODO find another way to prepare the arguments, without using the eval stack: use a few temporary variables instead, or use push()/pop() like replaceCallAsmSubStatementWithGosub() in the statement reorderer if(sub.parameters.isEmpty()) return - - // 1. load all arguments reversed onto the stack: first arg goes last (is on top). - + // load all arguments reversed onto the stack: first arg goes last (is on top). for (arg in stmt.args.reversed()) asmgen.translateExpression(arg) - // TODO here's an alternative to the above, but for now generates bigger code due to intermediate register steps: -// for (arg in stmt.args.reversed()) { -// // note this stuff below is needed to (eventually) avoid calling asmgen.translateExpression() -// // TODO also This STILL requires the translateNormalAssignment() to be fixed to avoid stack eval for expressions... -// val dt = arg.inferType(program).getOr(DataType.UNDEFINED) -// asmgen.assignExpressionTo(arg, AsmAssignTarget(TargetStorageKind.STACK, program, asmgen, dt, sub)) -// } - var argForCarry: IndexedValue>? = null var argForXregister: IndexedValue>? = null var argForAregister: IndexedValue>? = null diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AsmAssignment.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AsmAssignment.kt index 0f2de3b10..1e654f911 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AsmAssignment.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AsmAssignment.kt @@ -36,7 +36,7 @@ internal class AsmAssignTarget(val kind: TargetStorageKind, val array: ArrayIndexedExpression? = null, val memory: DirectMemoryWrite? = null, val register: RegisterOrPair? = null, - val origAstTarget: AssignTarget? = null + val origAstTarget: AssignTarget? = null // TODO look into removing the need to store this ) { val constMemoryAddress by lazy { memory?.addressExpression?.constValue(program)?.number?.toUInt() ?: 0u} diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt index 9fdba34d5..025d2cb73 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt @@ -308,22 +308,17 @@ internal class AssignmentAsmGen(private val program: Program, asmgen.translate(ifelse) } else { - // no orig ast assign target, can't use the workaround, so fallback to stack eval - fallbackToStackEval(value, assign) + // no orig ast assign target so can't use the workaround, so fallback to stack eval + fallbackToStackEval(assign) } } else { - // Everything else just evaluate via the stack. + // All remaining binary expressions just evaluate via the stack for now. // (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...) - fallbackToStackEval(value, assign) + fallbackToStackEval(assign) } } - else -> { - // Everything else just evaluate via the stack. - // (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...) - fallbackToStackEval(value, assign) - } + else -> throw AssemblyError("weird assignment value type $value") } } SourceStorageKind.REGISTER -> { @@ -336,9 +331,13 @@ internal class AssignmentAsmGen(private val program: Program, } } - private fun fallbackToStackEval(value: Expression, assign: AsmAssignment) { - // TODO DON'T STACK-EVAL THIS... by using a temp var? so that it becomes augmentable assignment expression? - asmgen.translateExpression(value) + private fun fallbackToStackEval(assign: AsmAssignment) { + // TODO DON'T STACK-EVAL... perhaps by using a temp var? so that it becomes augmentable assignment expression? + // or don't try to solve it here in this one case and rather rewrite the whole stack based value evaluation. + // this routine is called for assigning a binaryexpression value: + // - if it's a boolean comparison expression and the workaround isn't possible (no origTarget ast node) + // - for all other binary expressions. + asmgen.translateExpression(assign.source.expression!!) if (assign.target.datatype in WordDatatypes && assign.source.datatype in ByteDatatypes) asmgen.signExtendStackLsb(assign.source.datatype) if (assign.target.kind != TargetStorageKind.STACK || assign.target.datatype != assign.source.datatype) @@ -352,7 +351,7 @@ internal class AssignmentAsmGen(private val program: Program, val constRange = range.toConstantIntegerRange() if(constRange!=null) return containmentCheckIntoA(containment.element, elementDt.getOr(DataType.UNDEFINED), constRange.toList()) - TODO("non-const range containment check ${containment.position}") + throw AssemblyError("non const range containment check not supported") } val variable = (containment.iterable as? IdentifierReference)?.targetVarDecl(program) if(variable!=null) { diff --git a/codeOptimizers/src/prog8/optimizer/ConstantFoldingOptimizer.kt b/codeOptimizers/src/prog8/optimizer/ConstantFoldingOptimizer.kt index cbe62a80e..6c2076cb9 100644 --- a/codeOptimizers/src/prog8/optimizer/ConstantFoldingOptimizer.kt +++ b/codeOptimizers/src/prog8/optimizer/ConstantFoldingOptimizer.kt @@ -427,7 +427,7 @@ class ConstantFoldingOptimizer(private val program: Program) : AstWalker() { { // NOTE: THIS IS ONLY VALID ON FLOATING POINT CONSTANTS - // todo: this implements only a small set of possible reorderings at this time, we could think of more + // TODO: this implements only a small set of possible reorderings at this time, we could think of more if(expr.operator==subExpr.operator) { // both operators are the same. diff --git a/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt b/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt index bc524f88b..509283e4d 100644 --- a/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt +++ b/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt @@ -16,13 +16,7 @@ import kotlin.math.abs import kotlin.math.log2 import kotlin.math.pow -/* - todo add more peephole expression optimizations - - Investigate what optimizations binaryen has, also see https://egorbo.com/peephole-optimizations.html - - */ - +// TODO add more peephole expression optimizations? Investigate what optimizations binaryen has, also see https://egorbo.com/peephole-optimizations.html class ExpressionSimplifier(private val program: Program, private val errors: IErrorReporter) : AstWalker() { private val powersOfTwo = (1..16).map { (2.0).pow(it) }.toSet() diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt index 7d3a6aa82..a4a12a7bd 100644 --- a/compiler/src/prog8/compiler/Compiler.kt +++ b/compiler/src/prog8/compiler/Compiler.kt @@ -275,15 +275,10 @@ fun determineCompilationOptions(program: Program, compTarget: ICompilationTarget } private fun processAst(program: Program, errors: IErrorReporter, compilerOptions: CompilationOptions) { - // perform initial syntax checks and processings println("Analyzing code...") program.preprocessAst(errors) program.checkIdentifiers(errors, compilerOptions) errors.report() - // TODO: turning char literals into UBYTEs via an encoding should really happen in code gen - but for that we'd need DataType.CHAR - // ...but what do we gain from this? We can leave it as it is now: where a char literal is no more than syntactic sugar for an UBYTE value. - // By introduction a CHAR dt, we will also lose the opportunity to do constant-folding on any expression containing a char literal. - // Yes this is different from strings that are only encoded in the code gen phase. program.charLiteralsToUByteLiterals(compilerOptions.compTarget, errors) errors.report() program.constantFold(errors, compilerOptions.compTarget) @@ -303,13 +298,10 @@ private fun processAst(program: Program, errors: IErrorReporter, compilerOptions } private fun optimizeAst(program: Program, compilerOptions: CompilationOptions, errors: IErrorReporter, functions: IBuiltinFunctions, compTarget: ICompilationTarget) { - // optimize the parse tree println("Optimizing...") - val remover = UnusedCodeRemover(program, errors, compTarget) remover.visit(program) remover.applyModifications() - while (true) { // keep optimizing expressions and statements until no more steps remain val optsDone1 = program.simplifyExpressions(errors) @@ -346,7 +338,6 @@ private fun writeAssembly(program: Program, errors: IErrorReporter, compilerOptions: CompilationOptions ): WriteAssemblyResult { - // asm generation directly from the Ast compilerOptions.compTarget.machine.initializeZeropage(compilerOptions) val variables = VariableExtractor().extractVars(program) program.processAstBeforeAsmGeneration(compilerOptions, variables, errors) diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 6363439b9..9ca10d26d 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -1221,6 +1221,10 @@ internal class AstChecker(private val program: Program, if(containment.parent is BinaryExpression) errors.err("containment check is currently not supported in complex expressions", containment.position) + val range = containment.iterable as? RangeExpression + if(range!=null && range.toConstantIntegerRange()==null) + errors.err("containment check requires a constant integer range", range.position) + if(iterableDt.isIterable) { val iterableEltDt = ArrayToElementTypes.getValue(iterableDt.getOr(DataType.UNDEFINED)) val invalidDt = if (elementDt.isBytes) { diff --git a/compiler/test/ast/TestProg8Parser.kt b/compiler/test/ast/TestProg8Parser.kt index a19d32b6e..c9b3fdaa8 100644 --- a/compiler/test/ast/TestProg8Parser.kt +++ b/compiler/test/ast/TestProg8Parser.kt @@ -872,7 +872,6 @@ class TestProg8Parser: FunSpec( { val ff = start.statements[4] as Assignment ff.value shouldBe NumericLiteral(DataType.UBYTE, 255.0, Position.DUMMY) val letter = start.statements[6] as Assignment - // TODO characters should perhaps not be encoded until code generation, like strings... however this will prevent optimizing expressions with characters val encodedletter = PetsciiEncoding.encodePetscii("A", true).getOrElse { fail("petscii error") }.single() letter.value shouldBe NumericLiteral(DataType.UBYTE, encodedletter.toDouble(), Position.DUMMY) } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index d1c85f2fc..10d31a119 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,7 +3,8 @@ TODO For next release ^^^^^^^^^^^^^^^^ -- ... +- attempt to rework registerArgsViaStackEvaluation() to use tempvars or push()/pop() instead of evalstack based evaluation + actually, all function call asmgen code should use the same routine to pass arguments (replaceCallAsmSubStatementWithGosub ?) Need help with diff --git a/examples/test.p8 b/examples/test.p8 index 26dd17bf5..9ecdec712 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,14 +1,31 @@ +%import textio + main { - str string1 = "default" - str string2 = sc:"screencodes" - str string3 = iso:"iso" - str string4 = petscii:"petscii" - - ubyte char1 = 'd' - ubyte char2 = sc:'s' - ubyte char3 = iso:'i' - ubyte char4 = petscii:'p' - sub start() { + ubyte xx = 10 + ubyte yy = 10 + + routine(xx+yy, yy+99, 99, true) + + } + + uword @shared r_arg + ubyte @shared r_arg2 + ubyte @shared r_arg3 + ubyte @shared r_arg4 + + asmsub routine(uword arg @AY, ubyte arg2 @X, ubyte arg3 @R0, ubyte arg4 @Pc) { + %asm {{ + pha + adc #0 + sta r_arg4 + pla + sta r_arg + sty r_arg+1 + stx r_arg2 + lda cx16.r0 + sta r_arg3 + rts + }} } }