From dcc1f000480d6125dee0741d097ac0987602f580 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 24 Jul 2022 12:21:10 +0200 Subject: [PATCH] fix rounding errors in signed divide by power-of-two The optimized bit-shifting division is removed (for now) --- .../codegen/cpu6502/ExpressionsAsmGen.kt | 14 ++++---- .../src/prog8/codegen/virtual/CodeGen.kt | 8 ++--- .../prog8/optimizer/ExpressionSimplifier.kt | 19 ++-------- docs/source/todo.rst | 8 +++-- examples/test.p8 | 35 +++++++++---------- 5 files changed, 35 insertions(+), 49 deletions(-) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt index 589253a80..f1884290a 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt @@ -519,14 +519,14 @@ internal class ExpressionsAsmGen(private val program: Program, val rightVal = expr.right.constValue(program)?.number?.toInt() if(rightVal!=null && rightVal==2) { translateExpressionInternal(expr.left) - when(leftDt) { - DataType.UBYTE -> asmgen.out(" lsr P8ESTACK_LO+1,x") - DataType.BYTE -> asmgen.out(" lda P8ESTACK_LO+1,x | asl a | ror P8ESTACK_LO+1,x") - DataType.UWORD -> asmgen.out(" lsr P8ESTACK_HI+1,x | ror P8ESTACK_LO+1,x") - DataType.WORD -> asmgen.out(" lda P8ESTACK_HI+1,x | asl a | ror P8ESTACK_HI+1,x | ror P8ESTACK_LO+1,x") - else -> throw AssemblyError("wrong dt") + // shifting only yields the correct rounded result on unsinged numbers + if(leftDt==DataType.UBYTE) { + asmgen.out(" lsr P8ESTACK_LO+1,x") + return + } else if(leftDt==DataType.UWORD) { + asmgen.out(" lsr P8ESTACK_HI+1,x | ror P8ESTACK_LO+1,x") + return } - return } } } diff --git a/codeGenVirtual/src/prog8/codegen/virtual/CodeGen.kt b/codeGenVirtual/src/prog8/codegen/virtual/CodeGen.kt index b97faf3fd..555f5b484 100644 --- a/codeGenVirtual/src/prog8/codegen/virtual/CodeGen.kt +++ b/codeGenVirtual/src/prog8/codegen/virtual/CodeGen.kt @@ -511,14 +511,14 @@ class CodeGen(internal val program: PtProgram, if(factor==1) return code val pow2 = powersOfTwo.indexOf(factor) - if(pow2==1) { + if(pow2==1 && !signed) { // just shift 1 bit code += if(signed) VmCodeInstruction(Opcode.ASR, dt, reg1=reg) else VmCodeInstruction(Opcode.LSR, dt, reg1=reg) } - else if(pow2>=1) { + else if(pow2>=1 &&!signed) { // just shift multiple bits val pow2reg = vmRegisters.nextFree() code += VmCodeInstruction(Opcode.LOAD, dt, reg1=pow2reg, value=pow2) @@ -544,14 +544,14 @@ class CodeGen(internal val program: PtProgram, if(factor==1) return code val pow2 = powersOfTwo.indexOf(factor) - if(pow2==1) { + if(pow2==1 && !signed) { // just shift 1 bit code += if(signed) VmCodeInstruction(Opcode.ASRM, dt, value=address) else VmCodeInstruction(Opcode.LSRM, dt, value=address) } - else if(pow2>=1) { + else if(pow2>=1 && !signed) { // just shift multiple bits val pow2reg = vmRegisters.nextFree() code += VmCodeInstruction(Opcode.LOAD, dt, reg1=pow2reg, value=pow2) diff --git a/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt b/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt index 774a584c8..97fcdf3ce 100644 --- a/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt +++ b/codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt @@ -21,7 +21,6 @@ import kotlin.math.pow class ExpressionSimplifier(private val program: Program) : AstWalker() { private val powersOfTwo = (1..16).map { (2.0).pow(it) }.toSet() - private val negativePowersOfTwo = powersOfTwo.map { -it }.toSet() override fun after(typecast: TypecastExpression, parent: Node): Iterable { val mods = mutableListOf() @@ -469,19 +468,12 @@ class ExpressionSimplifier(private val program: Program) : AstWalker() { } } in powersOfTwo -> { - if (leftDt in IntegerDatatypes) { - // divided by a power of two => shift right + if (leftDt==DataType.UBYTE || leftDt==DataType.UWORD) { + // unsigned number divided by a power of two => shift right val numshifts = log2(cv).toInt() return BinaryExpression(expr.left, ">>", NumericLiteral.optimalInteger(numshifts, expr.position), expr.position) } } - in negativePowersOfTwo -> { - if (leftDt in IntegerDatatypes) { - // divided by a negative power of two => negate, then shift right - val numshifts = log2(-cv).toInt() - return BinaryExpression(PrefixExpression("-", expr.left, expr.position), ">>", NumericLiteral.optimalInteger(numshifts, expr.position), expr.position) - } - } } if (leftDt == DataType.UBYTE) { @@ -537,13 +529,6 @@ class ExpressionSimplifier(private val program: Program) : AstWalker() { return BinaryExpression(expr2.left, "<<", NumericLiteral.optimalInteger(numshifts, expr.position), expr.position) } } - in negativePowersOfTwo -> { - if (leftValue.inferType(program).isInteger) { - // times a negative power of two => negate, then shift left - val numshifts = log2(-cv).toInt() - return BinaryExpression(PrefixExpression("-", expr2.left, expr.position), "<<", NumericLiteral.optimalInteger(numshifts, expr.position), expr.position) - } - } } } // no need to check for left val constant (because of associativity) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 94f87b343..74b886c14 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,9 +3,11 @@ TODO For next release ^^^^^^^^^^^^^^^^ -- what to do with the rouding difference in signed divide by 2 / 4 (double ror)? it rounds towards minus infinity (so -5 / 2 = -3) - while the NON-optimized routine produces -2 . Virtual machine also produces -3? - What rounding do we want? +- Add optimized signed word division for factors of 2 (bit shifting but this time with correct rounding) + CodeGen divideByConst() and divideByConstInplace() + ExpressionsAsmGen translateExpression() + ExpressionSimplifier optimizeDivision() ? + - add item to XyzZeropage that enables an option that if zeropage=FULL or KERNALSAFE, moves the cx16 virtual registers to ZP, same location as on x16 (can be done on C64 only for now) Remove those addresses from the ZP free pool = allocate them in ZP like Cx16Zeropage does Adapt the code in AstPreprocessor that relocates the registers as well. diff --git a/examples/test.p8 b/examples/test.p8 index 86a43eb66..83d836f22 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -3,28 +3,27 @@ %zeropage basicsafe main { + sub derp(word num, ubyte a1, ubyte a2, ubyte a3, ubyte a4) { + txt.print_w(num) + txt.nl() + } + + ; TODO test with new optimized division routines. + sub start() { - word bb = -15 - bb /= 4 - txt.print_w(bb) + byte qq = 1 + byte bb = -51 + derp((bb*qq)/-4, 1,2,3,4) + bb /= -4 + txt.print_b(bb) txt.nl() - bb = 15 - bb /= 4 - txt.print_w(bb) + bb = 51 + bb /= -4 + txt.print_b(bb) txt.nl() - uword ubb = 15 + ubyte ubb = 51 ubb /= 4 - txt.print_uw(ubb) + txt.print_ub(ubb) txt.nl() - - recurse1() - } - - sub recurse1() { - recurse2() - } - - sub recurse2() { - start() } }