From 134fd62da8ff7d391416ba428f803ac2a9ab2d4a Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 21 Mar 2023 02:54:26 +0100 Subject: [PATCH] RPN: better handling of bit shifts --- codeCore/src/prog8/code/ast/AstExpressions.kt | 10 +- .../cpu6502/assignment/AssignmentAsmGen.kt | 92 ++++++++++--------- .../assignment/AugmentableAssignmentAsmGen.kt | 16 +++- .../compiler/astprocessing/AstChecker.kt | 12 ++- .../astprocessing/BeforeAsmTypecastCleaner.kt | 4 + .../compiler/astprocessing/TypecastsAdder.kt | 28 +++--- compiler/test/ProjectConfig.kt | 2 +- compiler/test/ast/TestVariousCompilerAst.kt | 2 +- .../test/codegeneration/TestVariousCodeGen.kt | 21 +++++ docs/source/todo.rst | 17 ++-- 10 files changed, 133 insertions(+), 71 deletions(-) diff --git a/codeCore/src/prog8/code/ast/AstExpressions.kt b/codeCore/src/prog8/code/ast/AstExpressions.kt index d218fc36a..c88f642d6 100644 --- a/codeCore/src/prog8/code/ast/AstExpressions.kt +++ b/codeCore/src/prog8/code/ast/AstExpressions.kt @@ -270,10 +270,16 @@ 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 { + fun truncateLastOperator(): PtExpression { // NOTE: this is a destructive operation! children.removeLast() children.removeLast() + if(children.last() !is PtRpnOperator) { + if(children.size==1 && children[0] is PtExpression) + return children[0] as PtExpression + else + throw IllegalArgumentException("don't know what to do with children: $children") + } val finalOper = finalOperator() if(finalOper.type==type) return this val typeAdjusted = PtRpn(finalOper.type, this.position) @@ -286,7 +292,7 @@ class PtRpn(type: DataType, position: Position): PtExpression(type, position) { class PtRpnOperator(val operator: String, val type: DataType, val leftType: DataType, val rightType: DataType, position: Position): PtNode(position) { init { // NOTE: For now, we require that the types of the operands are the same size as the output type of the operator node. - if(operator !in ComparisonOperators) { + if(operator !in ComparisonOperators && operator != "<<" && operator != ">>") { require(type equalsSize leftType && type equalsSize rightType) { "operand type size(s) differ from operator result type $type: $leftType $rightType oper: $operator" } diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt index 46a83bd44..75cc39bf3 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt @@ -374,16 +374,18 @@ internal class AssignmentAsmGen(private val program: PtProgram, } if(value.children.size==3 && left is PtExpression && right is PtExpression) { + // TODO RPN make this work also when left is not an Expression (in which case size >3) if (simpleEqualityExprRPN(left, oper.operator, right, assign.target)) return true if (simpleLogicalExprRPN(left, oper.operator, right, assign.target)) return true if (simplePlusOrMinusExprRPN(left, oper.operator, right, assign.target)) return true - if (simpleBitshiftExprRPN(left, oper.operator, right, assign.target)) - return true } + if (simpleBitshiftExprRPN(value, oper, right, assign.target)) + return true + val asmExtra = asmgen.subroutineExtra(scope) val evalVars = mutableMapOf ( DataType.UBYTE to Stack(), @@ -564,8 +566,7 @@ internal class AssignmentAsmGen(private val program: PtProgram, return false // expression datatype is BOOL (ubyte) but operands can be anything - if(left.type in ByteDatatypes && right.type in ByteDatatypes && - left.isSimple() && right.isSimple()) { + if(left.type in ByteDatatypes && right.type in ByteDatatypes && left.isSimple() && right.isSimple()) { assignExpressionToRegister(left, RegisterOrPair.A, false) asmgen.saveRegisterStack(CpuRegister.A, false) assignExpressionToVariable(right, "P8ZP_SCRATCH_B1", DataType.UBYTE) @@ -589,8 +590,7 @@ internal class AssignmentAsmGen(private val program: PtProgram, } assignRegisterByte(target, CpuRegister.A) return true - } else if(left.type in WordDatatypes && right.type in WordDatatypes && - left.isSimple() && right.isSimple()) { + } else if(left.type in WordDatatypes && right.type in WordDatatypes && left.isSimple() && right.isSimple()) { assignExpressionToRegister(left, RegisterOrPair.AY, false) asmgen.saveRegisterStack(CpuRegister.A, false) asmgen.saveRegisterStack(CpuRegister.Y, false) @@ -759,59 +759,61 @@ internal class AssignmentAsmGen(private val program: PtProgram, return false } - private fun simpleBitshiftExprRPN(left: PtExpression, operator: String, right: PtExpression, target: AsmAssignTarget): Boolean { - if(operator!="<<" && operator!=">>") + private fun simpleBitshiftExprRPN(expr: PtRpn, operator: PtRpnOperator, rightNode: PtNode, target: AsmAssignTarget): Boolean { + if(operator.operator!="<<" && operator.operator!=">>") + return false + val shifts = (rightNode as? PtExpression)?.asConstInteger() + if(shifts==null || shifts !in 0..7 || operator.leftType !in IntegerDatatypes) return false - val shifts = right.asConstInteger() - if(shifts!=null) { - val dt = left.type - if(dt in ByteDatatypes && shifts in 0..7) { - val signed = dt == DataType.BYTE - assignExpressionToRegister(left, RegisterOrPair.A, signed) - if(operator=="<<") { - repeat(shifts) { - asmgen.out(" asl a") - } + if(operator.leftType in ByteDatatypes) { + val signed = operator.leftType == DataType.BYTE + val left = expr.truncateLastOperator() + assignExpressionToRegister(left, RegisterOrPair.A, signed) + if(operator.operator=="<<") { + repeat(shifts) { + asmgen.out(" asl a") + } + } else { + if(signed && shifts>0) { + asmgen.out(" ldy #$shifts | jsr math.lsr_byte_A") } else { - if(signed && shifts>0) { - asmgen.out(" ldy #$shifts | jsr math.lsr_byte_A") - } else { - repeat(shifts) { - asmgen.out(" lsr a") - } + repeat(shifts) { + asmgen.out(" lsr a") } } - assignRegisterByte(target, CpuRegister.A) - return true - } else if(dt in WordDatatypes && shifts in 0..7) { - val signed = dt == DataType.WORD + } + assignRegisterByte(target, CpuRegister.A) + return true + } else if(operator.leftType in WordDatatypes) { + val signed = operator.leftType == DataType.WORD + if(operator.operator=="<<") { + val left = expr.truncateLastOperator() assignExpressionToRegister(left, RegisterOrPair.AY, signed) - if(operator=="<<") { + if(shifts>0) { + asmgen.out(" sty P8ZP_SCRATCH_B1") + repeat(shifts) { + asmgen.out(" asl a | rol P8ZP_SCRATCH_B1") + } + asmgen.out(" ldy P8ZP_SCRATCH_B1") + } + } else { + if(signed) { + return false // TODO("shift AY >> $shifts signed") + } else { + val left = expr.truncateLastOperator() + assignExpressionToRegister(left, RegisterOrPair.AY, false) if(shifts>0) { asmgen.out(" sty P8ZP_SCRATCH_B1") repeat(shifts) { - asmgen.out(" asl a | rol P8ZP_SCRATCH_B1") + asmgen.out(" lsr P8ZP_SCRATCH_B1 | ror a") } asmgen.out(" ldy P8ZP_SCRATCH_B1") } - } else { - if(signed) { - // TODO("shift AY >> $shifts signed") - return false - } else { - if(shifts>0) { - asmgen.out(" sty P8ZP_SCRATCH_B1") - repeat(shifts) { - asmgen.out(" lsr P8ZP_SCRATCH_B1 | ror a") - } - asmgen.out(" ldy P8ZP_SCRATCH_B1") - } - } } - assignRegisterpairWord(target, RegisterOrPair.AY) - return true } + assignRegisterpairWord(target, RegisterOrPair.AY) + return true } return false diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt index b72284f39..d930fb117 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt @@ -1288,7 +1288,9 @@ internal class AugmentableAssignmentAsmGen(private val program: PtProgram, sta $name+1 """) } - "<<", ">>" -> throw AssemblyError("shift by a word value not supported, max is a byte") + "<<", ">>" -> { + throw AssemblyError("shift by a word variable not supported, max is a byte") + } "&" -> asmgen.out(" lda $name | and $otherName | sta $name | lda $name+1 | and $otherName+1 | sta $name+1") "|" -> asmgen.out(" lda $name | ora $otherName | sta $name | lda $name+1 | ora $otherName+1 | sta $name+1") "^" -> asmgen.out(" lda $name | eor $otherName | sta $name | lda $name+1 | eor $otherName+1 | sta $name+1") @@ -1504,7 +1506,17 @@ internal class AugmentableAssignmentAsmGen(private val program: PtProgram, asmgen.assignExpressionToRegister(value, RegisterOrPair.AY) remainderVarByWordInAY() } - "<<", ">>" -> throw AssemblyError("shift by a word value not supported, max is a byte") + "<<", ">>" -> { + if(value is PtNumber && value.number<=255) { + when (dt) { + in WordDatatypes -> TODO("shift a word var by ${value.number}") + in ByteDatatypes -> TODO("shift a byte var by ${value.number}") + else -> throw AssemblyError("weird dt for shift") + } + } else { + throw AssemblyError("shift by a word value not supported, max is a byte") + } + } "&" -> { asmgen.assignExpressionToRegister(value, RegisterOrPair.AY) asmgen.out(" and $name | sta $name | tya | and $name+1 | sta $name+1") diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 20074a617..7704f3e9c 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -929,6 +929,14 @@ internal class AstChecker(private val program: Program, errors.err("bitwise operator can only be used on integer operands", expr.right.position) } "in" -> throw FatalAstException("in expression should have been replaced by containmentcheck") + "<<", ">>" -> { + if(rightDt in WordDatatypes) { + val shift = expr.right.constValue(program)?.number?.toInt() + if(shift==null || shift > 255) { + errors.err("shift by a word value not supported, max is a byte", expr.position) + } + } + } } if(leftDt !in NumericDatatypes && leftDt != DataType.STR && leftDt != DataType.BOOL) @@ -937,11 +945,13 @@ internal class AstChecker(private val program: Program, errors.err("right operand is not numeric or str", expr.right.position) if(leftDt!=rightDt) { if(leftDt==DataType.STR && rightDt in IntegerDatatypes && expr.operator=="*") { - // only exception allowed: str * constvalue + // exception allowed: str * constvalue if(expr.right.constValue(program)==null) errors.err("can only use string repeat with a constant number value", expr.left.position) } else if(leftDt==DataType.BOOL && rightDt in ByteDatatypes || leftDt in ByteDatatypes && rightDt==DataType.BOOL) { // expression with one side BOOL other side (U)BYTE is allowed; bool==byte + } else if((expr.operator == "<<" || expr.operator == ">>") && (leftDt in WordDatatypes && rightDt in ByteDatatypes)) { + // exception allowed: shifting a word by a byte } else { errors.err("left and right operands aren't the same type", expr.left.position) } diff --git a/compiler/src/prog8/compiler/astprocessing/BeforeAsmTypecastCleaner.kt b/compiler/src/prog8/compiler/astprocessing/BeforeAsmTypecastCleaner.kt index 3eeebfff3..aa561708d 100644 --- a/compiler/src/prog8/compiler/astprocessing/BeforeAsmTypecastCleaner.kt +++ b/compiler/src/prog8/compiler/astprocessing/BeforeAsmTypecastCleaner.kt @@ -139,6 +139,10 @@ internal class BeforeAsmTypecastCleaner(val program: Program, errors.warn("shift always results in 0", expr.position) if(dt.istype(DataType.UWORD) && shifts.number>=16.0) errors.warn("shift always results in 0", expr.position) + if(shifts.number<=255.0 && shifts.type in WordDatatypes) { + val byteVal = NumericLiteral(DataType.UBYTE, shifts.number, shifts.position) + return listOf(IAstModification.ReplaceNode(expr.right, byteVal, expr)) + } } } return noModifications diff --git a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt index 74288107f..b6e1c0c0b 100644 --- a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt +++ b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt @@ -60,15 +60,15 @@ class TypecastsAdder(val program: Program, val options: CompilationOptions, val if (parent is Assignment) { if (parent.target.inferType(program).isWords) { modifications += IAstModification.ReplaceNode(expr.left, leftConstAsWord, expr) - if(rightDt.isBytes) - modifications += IAstModification.ReplaceNode(expr.right, TypecastExpression(expr.right, leftConstAsWord.type, true, expr.right.position), expr) +// if(rightDt.isBytes) +// modifications += IAstModification.ReplaceNode(expr.right, TypecastExpression(expr.right, leftConstAsWord.type, true, expr.right.position), expr) } } else if (parent is TypecastExpression && parent.type == DataType.UWORD && parent.parent is Assignment) { val assign = parent.parent as Assignment if (assign.target.inferType(program).isWords) { modifications += IAstModification.ReplaceNode(expr.left, leftConstAsWord, expr) - if(rightDt.isBytes) - modifications += IAstModification.ReplaceNode(expr.right, TypecastExpression(expr.right, leftConstAsWord.type, true, expr.right.position), expr) +// if(rightDt.isBytes) +// modifications += IAstModification.ReplaceNode(expr.right, TypecastExpression(expr.right, leftConstAsWord.type, true, expr.right.position), expr) } } if(modifications.isNotEmpty()) @@ -140,16 +140,18 @@ class TypecastsAdder(val program: Program, val options: CompilationOptions, val } - // 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.right) - if(toFix!=null) { - val modifications = mutableListOf() - when { - toFix===expr.left -> addTypecastOrCastedValueModification(modifications, expr.left, commonDt, expr) - toFix===expr.right -> addTypecastOrCastedValueModification(modifications, expr.right, commonDt, expr) - else -> throw FatalAstException("confused binary expression side") + if((expr.operator!="<<" && expr.operator!=">>") || !leftDt.isWords || !rightDt.isBytes) { + // 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.right) + if(toFix!=null) { + val modifications = mutableListOf() + when { + toFix===expr.left -> addTypecastOrCastedValueModification(modifications, expr.left, commonDt, expr) + toFix===expr.right -> addTypecastOrCastedValueModification(modifications, expr.right, commonDt, expr) + else -> throw FatalAstException("confused binary expression side") + } + return modifications } - return modifications } } } diff --git a/compiler/test/ProjectConfig.kt b/compiler/test/ProjectConfig.kt index 5e368549f..9c1542120 100644 --- a/compiler/test/ProjectConfig.kt +++ b/compiler/test/ProjectConfig.kt @@ -3,7 +3,7 @@ package prog8tests import io.kotest.core.config.AbstractProjectConfig object ProjectConfig : AbstractProjectConfig() { - override val parallelism = 2 // max(2, Runtime.getRuntime().availableProcessors() / 2) + override val parallelism = kotlin.math.max(1, Runtime.getRuntime().availableProcessors() / 2) // override fun listeners() = listOf(SystemOutToNullListener) } diff --git a/compiler/test/ast/TestVariousCompilerAst.kt b/compiler/test/ast/TestVariousCompilerAst.kt index 05a2645ee..ebdea7e7b 100644 --- a/compiler/test/ast/TestVariousCompilerAst.kt +++ b/compiler/test/ast/TestVariousCompilerAst.kt @@ -134,7 +134,7 @@ main { compileText(C64Target(), optimize=false, src, writeAssembly=false) shouldNotBe null } - test("bitshift left of const byte converted to word") { + test("bitshift left of const byte not converted to word") { val src=""" main { sub start() { diff --git a/compiler/test/codegeneration/TestVariousCodeGen.kt b/compiler/test/codegeneration/TestVariousCodeGen.kt index d2fab5501..2ecdc9ad2 100644 --- a/compiler/test/codegeneration/TestVariousCodeGen.kt +++ b/compiler/test/codegeneration/TestVariousCodeGen.kt @@ -12,6 +12,7 @@ import prog8.code.ast.PtAssignment import prog8.code.ast.PtVariable import prog8.code.core.DataType import prog8.code.target.C64Target +import prog8.code.target.VMTarget import prog8tests.helpers.ErrorReporterForTests import prog8tests.helpers.compileText @@ -144,4 +145,24 @@ main { errors.errors.size shouldBe 1 errors.errors[0] shouldContain "undefined symbol: doesnotexist" } + + test("shifting by word value is ok") { + val text=""" +main { + sub start() { + ubyte c = 1 + @(15000 + c<<${'$'}0003) = 42 + @(15000 + (c<<${'$'}0003)) = 42 + @(15000 + c*${'$'}0008) = 42 ; *8 becomes a shift after opt + + uword @shared qq = 15000 + c<<${'$'}0003 + qq = 15000 + (c<<${'$'}0003) + qq = 16000 + c*${'$'}0008 + } +}""" + compileText(C64Target(), true, text, writeAssembly = true, useRPN = false) shouldNotBe null + compileText(C64Target(), true, text, writeAssembly = true, useRPN = true) shouldNotBe null + compileText(VMTarget(), true, text, writeAssembly = true, useRPN = false) shouldNotBe null + // TODO RPN once IR RPN codegen is done: compileText(VMTarget(), true, text, writeAssembly = true, useRPN = true) shouldNotBe null + } }) \ No newline at end of file diff --git a/docs/source/todo.rst b/docs/source/todo.rst index ea8646649..00998008a 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,14 +1,19 @@ TODO ==== -RPN: cube3d-sprites compiler crash (bit shift too large) -RPN: swirl is bigger and MUCH slower -RPN: wizzine is slower but about equal size +RPN: assem once again is broken with selftest +RPN: optimize RPN in AssignmentAsmGen TODO's +RPN: swirl is MUCH slower +RPN: wizzine is slower + +- Move asmExtra vars into BSS as well, now are .byte 0 allocated + +then: +RPN: swirl is bigger +RPN: petaxian is 900 bytes larger, chess is a lot bigger RPN: charset is larger RPN: cube3d is much larger, but a bit faster RPN: cube3d-float is massive and slow RPN: mandelbrot is bigger, but seems faster - -then: RPN: Implement RPN codegen for IR. @@ -16,7 +21,7 @@ For next minor release ^^^^^^^^^^^^^^^^^^^^^^ - ubyte fits = cx