From d26527114842397649b28db0efedc671c27e2776 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Mon, 20 Mar 2023 21:43:53 +0100 Subject: [PATCH] fix rpn variable depth clobber and type error --- codeCore/src/prog8/code/ast/AstExpressions.kt | 11 +++ codeCore/src/prog8/code/ast/AstPrinter.kt | 4 +- .../src/prog8/codegen/cpu6502/AsmGen.kt | 28 +++---- .../codegen/cpu6502/ExpressionsAsmGen.kt | 14 ++-- .../cpu6502/assignment/AssignmentAsmGen.kt | 51 ++++++------- .../src/prog8/optimizer/BinExprSplitter.kt | 2 - docs/source/todo.rst | 4 +- examples/test.p8 | 75 ++++++++++++------- 8 files changed, 103 insertions(+), 86 deletions(-) diff --git a/codeCore/src/prog8/code/ast/AstExpressions.kt b/codeCore/src/prog8/code/ast/AstExpressions.kt index 5db58c38a..d218fc36a 100644 --- a/codeCore/src/prog8/code/ast/AstExpressions.kt +++ b/codeCore/src/prog8/code/ast/AstExpressions.kt @@ -270,6 +270,17 @@ class PtRpn(type: DataType, position: Position): PtExpression(type, position) { fun finalLeftOperand() = children[children.size-3] fun finalRightOperand() = children[children.size-2] fun finalOperation() = Triple(finalLeftOperand(), finalOperator(), finalRightOperand()) + fun truncateLastOperator(): PtRpn { + // NOTE: this is a destructive operation! + children.removeLast() + children.removeLast() + val finalOper = finalOperator() + if(finalOper.type==type) return this + val typeAdjusted = PtRpn(finalOper.type, this.position) + typeAdjusted.children.addAll(children) + typeAdjusted.parent = parent + return typeAdjusted + } } class PtRpnOperator(val operator: String, val type: DataType, val leftType: DataType, val rightType: DataType, position: Position): PtNode(position) { diff --git a/codeCore/src/prog8/code/ast/AstPrinter.kt b/codeCore/src/prog8/code/ast/AstPrinter.kt index 8da3affba..1561d9f68 100644 --- a/codeCore/src/prog8/code/ast/AstPrinter.kt +++ b/codeCore/src/prog8/code/ast/AstPrinter.kt @@ -19,8 +19,8 @@ fun printAst(root: PtNode, skipLibraries: Boolean, output: (text: String) -> Uni is PtArray -> "array len=${node.children.size} ${type(node.type)}" is PtArrayIndexer -> " ${type(node.type)}" is PtBinaryExpression -> " ${node.operator} ${type(node.type)}" - is PtRpn -> "" - is PtRpnOperator -> node.operator + is PtRpn -> " ${type(node.type)}" + is PtRpnOperator -> "${node.operator} ${type(node.type)}" is PtBuiltinFunctionCall -> { val str = if(node.void) "void " else "" str + node.name + "()" diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt index 4e76c43e7..19fda1f69 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt @@ -1023,15 +1023,14 @@ $repeatLabel lda $counterVar || (rightmostOperand is PtTypeCast && rightmostOperand.value.type == DataType.UBYTE) ) { // split up the big expression in 2 parts so that we CAN use ZP,Y indexing after all - pointerOffsetExpr.children.removeLast() - pointerOffsetExpr.children.removeLast() + val truncatedExpr = pointerOffsetExpr.truncateLastOperator() val tempvar = getTempVarName(DataType.UWORD) - assignExpressionToVariable(pointerOffsetExpr, tempvar, DataType.UWORD) - val smallExpr = PtRpn(DataType.UWORD, pointerOffsetExpr.position) - smallExpr.addRpnNode(PtIdentifier(tempvar, DataType.UWORD, pointerOffsetExpr.position)) + assignExpressionToVariable(truncatedExpr, tempvar, DataType.UWORD) + val smallExpr = PtRpn(DataType.UWORD, truncatedExpr.position) + smallExpr.addRpnNode(PtIdentifier(tempvar, DataType.UWORD, truncatedExpr.position)) smallExpr.addRpnNode(rightmostOperand) smallExpr.addRpnNode(rightmostOperator) - smallExpr.parent = pointerOffsetExpr.parent + smallExpr.parent = truncatedExpr.parent val result = pointerViaIndexRegisterPossible(smallExpr) require(result != null) return result @@ -1178,11 +1177,9 @@ $repeatLabel lda $counterVar val (leftRpn, oper, right) = expr.finalOperation() if(oper.operator !in ComparisonOperators) throw AssemblyError("must be comparison expression") - val left: PtExpression = if(expr.children.size>3 || leftRpn !is PtExpression) { - expr.children.removeLast() - expr.children.removeLast() - expr - } else + val left: PtExpression = if(expr.children.size>3 || leftRpn !is PtExpression) + expr.truncateLastOperator() + else leftRpn // invert the comparison, so we can reuse the JumpIfFalse code generation routines @@ -1208,11 +1205,9 @@ $repeatLabel lda $counterVar private fun translateCompareAndJumpIfFalseRPN(expr: PtRpn, jumpIfFalseLabel: String) { val (leftRpn, oper, right) = expr.finalOperation() - val left: PtExpression = if(expr.children.size>3 || leftRpn !is PtExpression) { - expr.children.removeLast() - expr.children.removeLast() - expr - } else + val left: PtExpression = if(expr.children.size>3 || leftRpn !is PtExpression) + expr.truncateLastOperator() + else leftRpn require(right is PtExpression) @@ -3210,6 +3205,7 @@ internal class SubroutineExtraAsmInfo { var usedRegsaveY = false var usedFloatEvalResultVar1 = false var usedFloatEvalResultVar2 = false + var rpnDepth = 0 // 'depth' tracking of the RPN expression evaluator val extraVars = mutableListOf>() } \ No newline at end of file diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt index 5471ece68..9993aca40 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt @@ -263,9 +263,8 @@ internal class ExpressionsAsmGen(private val program: PtProgram, translateComparisonWithZero(left, leftDt, oper.operator) } is PtRpnOperator -> { - expr.children.removeLast() - expr.children.removeLast() - translateComparisonWithZero(expr, leftDt, oper.operator) + val truncated = expr.truncateLastOperator() + translateComparisonWithZero(truncated, leftDt, oper.operator) } else -> throw AssemblyError("weird rpn node") } @@ -282,7 +281,8 @@ internal class ExpressionsAsmGen(private val program: PtProgram, || (leftDt in WordDatatypes && rightDt !in WordDatatypes)) throw AssemblyError("operator ${oper.operator} left/right dt not identical: $leftDt $rightDt right=${expr.finalRightOperand()}") - var depth=0 + val asmExtra = asmgen.subroutineExtra(expr.definingISub()!!) + val startDepth = asmExtra.rpnDepth expr.children.forEach { if(it is PtRpnOperator) { when(it.leftType) { @@ -297,13 +297,13 @@ internal class ExpressionsAsmGen(private val program: PtProgram, } else -> throw AssemblyError("non-numerical datatype ${it.leftType}") } - depth-- + asmExtra.rpnDepth-- } else { translateExpressionInternal(it as PtExpression) - depth++ + asmExtra.rpnDepth++ } } - require(depth==1) { "unbalanced RPN: $depth ${expr.position}" } + require(asmExtra.rpnDepth-startDepth==1) { "unbalanced RPN: ${expr.position}" } } private fun translateExpression(expr: PtBinaryExpression) { diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt index adbdd93b0..d693ad72f 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt @@ -365,6 +365,8 @@ internal class AssignmentAsmGen(private val program: PtProgram, private fun attemptAssignOptimizedExprRPN(assign: AsmAssignment, scope: IPtSubroutine): Boolean { val value = assign.source.expression as PtRpn val (left, oper, right) = value.finalOperation() + if(oper.type != value.type) + throw AssemblyError("rpn node type error, expected ${value.type} got ${oper.type}") if(oper.operator in ComparisonOperators) { assignRPNComparison(assign, value) @@ -420,17 +422,15 @@ internal class AssignmentAsmGen(private val program: PtProgram, return name } - asmgen.out(" ; rpn expression @ ${value.position} ${value.children.size} nodes") // TODO - var depth=0 + val startDepth = asmExtra.rpnDepth value.children.forEach { when (it) { is PtRpnOperator -> { - asmgen.out(" ; rpn child node ${it.operator}") // TODO val rightvar = evalVars.getValue(getVarDt(it.rightType)).pop() val leftvar = evalVars.getValue(getVarDt(it.leftType)).pop() - depth-=2 - val resultVarname = evalVarName(it.type, depth) - depth++ + asmExtra.rpnDepth -= 2 + val resultVarname = evalVarName(it.type, asmExtra.rpnDepth) + asmExtra.rpnDepth++ symbolTable.resetCachedFlat() if(it.operator in ComparisonOperators) { require(it.type == DataType.UBYTE) @@ -445,8 +445,11 @@ internal class AssignmentAsmGen(private val program: PtProgram, val normalAssign = AsmAssignment(src, target, program.memsizer, assign.position) assignRPNComparison(normalAssign, comparison) } else { - require(resultVarname==leftvar) { - "expected result $resultVarname == leftvar $leftvar" + if(leftvar!=resultVarname) { + val scopeName = (scope as PtNamedNode).scopedName + val leftVarPt = PtIdentifier("$scopeName.$leftvar", it.leftType, it.position) + leftVarPt.parent=scope + assignExpressionToVariable(leftVarPt, resultVarname, it.type) } val src = AsmAssignSource(SourceStorageKind.VARIABLE, program, asmgen, it.rightType, variableAsmName = rightvar) val target = AsmAssignTarget(TargetStorageKind.VARIABLE, asmgen, it.type, scope, assign.position, variableAsmName = resultVarname) @@ -455,20 +458,20 @@ internal class AssignmentAsmGen(private val program: PtProgram, } } is PtExpression -> { - asmgen.out(" ; rpn child node expr ${it}") // TODO - val varname = evalVarName(it.type, depth) + val varname = evalVarName(it.type, asmExtra.rpnDepth) assignExpressionToVariable(it, varname, it.type) - depth++ + asmExtra.rpnDepth++ } else -> throw AssemblyError("weird rpn node") } } - asmgen.out(" ; DONE rpn expression @ ${value.position}") // TODO + require(asmExtra.rpnDepth-startDepth == 1) { + "unbalanced RPN ${value.position}" + } - require(depth==1) { "unbalanced RPN: $depth ${value.position}" } - asmgen.out(" ; assign rpn result to target") // TODO val resultVariable = evalVars.getValue(getVarDt(value.type)).pop() - if(assign.target.datatype != value.type) { + asmExtra.rpnDepth-- + if(!(assign.target.datatype equalsSize value.type)) { // we only allow for transparent byte -> word / ubyte -> uword assignments // any other type difference is an error if(assign.target.datatype in WordDatatypes && value.type in ByteDatatypes) { @@ -486,8 +489,6 @@ internal class AssignmentAsmGen(private val program: PtProgram, else -> throw AssemblyError("weird dt") } } - asmgen.out(" ; DONE assign rpn result to target") // TODO - require(evalVars.all { it.value.isEmpty() }) { "invalid rpn evaluation" } return true @@ -575,11 +576,9 @@ internal class AssignmentAsmGen(private val program: PtProgram, } } - val left: PtExpression = if(comparison.children.size>3 || leftRpn !is PtExpression) { - comparison.children.removeLast() - comparison.children.removeLast() - comparison - } else + val left: PtExpression = if(comparison.children.size>3 || leftRpn !is PtExpression) + comparison.truncateLastOperator() + else leftRpn val leftNum = left as? PtNumber @@ -1011,11 +1010,9 @@ internal class AssignmentAsmGen(private val program: PtProgram, private fun attemptAssignToByteCompareZeroRPN(expr: PtRpn, assign: AsmAssignment): Boolean { val (leftRpn, oper, right) = expr.finalOperation() - val left = if(expr.children.size!=3 || leftRpn !is PtExpression) { - expr.children.removeLast() - expr.children.removeLast() - expr - } else + val left = if(expr.children.size!=3 || leftRpn !is PtExpression) + expr.truncateLastOperator() + else leftRpn when (oper.operator) { "==" -> { diff --git a/codeOptimizers/src/prog8/optimizer/BinExprSplitter.kt b/codeOptimizers/src/prog8/optimizer/BinExprSplitter.kt index 61aa7e994..de5f0dae2 100644 --- a/codeOptimizers/src/prog8/optimizer/BinExprSplitter.kt +++ b/codeOptimizers/src/prog8/optimizer/BinExprSplitter.kt @@ -21,8 +21,6 @@ class BinExprSplitter(private val program: Program, private val options: Compila if(options.compTarget.name == VMTarget.NAME) return noModifications // don't split expressions when targeting the vm codegen, it handles nested expressions well - if(options.useRPN) // TODO RPN does this make a difference? - return noModifications if(assignment.value.inferType(program) istype DataType.FLOAT && !options.optimizeFloatExpressions) return noModifications diff --git a/docs/source/todo.rst b/docs/source/todo.rst index e457f6b18..6d562fcfb 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,15 +1,13 @@ TODO ==== -RPN: examples/maze crashes +RPN: cx16/mandelbrot-gfx-colors half display is wrong RPN: Fix the TODO RPN routines to be optimized assembly in RpnExpressionAsmGen.kt -RPN: check BinExprSplitter disablement any effect for RPN? then: RPN: examples/bsieve,charset compilation crash (bit shift expression) RPN: cube3d-float is massive and slow RPN: mandelbrot is big, but seems faster RPN: swirl is MUCH slower, wizzine is slower then: -RPN: check BinExprSplitter disablement any effect for RPN? RPN: Implement RPN codegen for IR. diff --git a/examples/test.p8 b/examples/test.p8 index 94a34a87c..7b9e0ef51 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,37 +1,54 @@ %import textio +%import floats %zeropage basicsafe main { - const ubyte numCellsHoriz = 15 ;(screenwidth-1) / 2 - const ubyte numCellsVert = 7 ; (screenheight-1) / 2 - - ; cell properties - const ubyte RIGHT = 2 - ubyte[256] cells = 0 - - sub generate() { - ubyte cx = 0 - cells[0] = 255 - repeat 40 { - bool fits = cx uword { - return &cells+cx - } + const uword width = 256 + const uword height = 240 + const ubyte max_iter = 16 ; 32 actually looks pretty nice but takes longer sub start() { - generate() - txt.nl() - txt.nl() + void cx16.screen_mode($80, false) + cx16.r0=0 + cx16.FB_init() + mandel() + } + + sub mandel() { + const float XL=-2.200 + const float XU=0.800 + const float YL=-1.300 + const float YU=1.300 + float dx = (XU-XL)/width + float dy = (YU-YL)/height + ubyte pixelx + ubyte pixely + + for pixely in 0 to height-1 { + float yy = YL+dy*(pixely as float) + + cx16.FB_cursor_position(0, pixely) + + for pixelx in 0 to width-1 { + float xx = XL+dx*(pixelx as float) + + float xsquared = 0.0 + float ysquared = 0.0 + float x = 0.0 + float y = 0.0 + ubyte iter = 0 + + while xsquared+ysquared<4.0 { + y = x*y*2.0 + yy + x = xsquared - ysquared + xx + xsquared = x*x + ysquared = y*y + iter++ + if iter>16 + break + } + cx16.FB_set_pixel(iter) + } + } } } -