From 34254314a60165527ca70718e034db418bf9927f Mon Sep 17 00:00:00 2001 From: Karol Stasiak Date: Sat, 3 Aug 2019 23:58:47 +0200 Subject: [PATCH] 6502: Fix word division and byte multiplication --- CHANGELOG.md | 4 ++ include/zp_reg.mfk | 29 ++++++++++++- .../compiler/mos/PseudoregisterBuiltIns.scala | 6 +-- .../scala/millfork/test/ByteMathSuite.scala | 19 +++++++++ .../scala/millfork/test/WordMathSuite.scala | 42 +++++++++++++++---- 5 files changed, 86 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db3a02b2..54aa92bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ This matches both the CC65 behaviour and the return values from `readkey()`. * 6502: Fixed variable bit shifting. +* 6502: Fixed word division by a divisor larger than 127. + +* 6502: Fixed byte multiplication. + * 8080/Z80: Fixed byte division. * Fixed many optimization bugs: diff --git a/include/zp_reg.mfk b/include/zp_reg.mfk index 33e77afe..dc9827c4 100644 --- a/include/zp_reg.mfk +++ b/include/zp_reg.mfk @@ -68,12 +68,17 @@ __mul_u16u8u16_start: } // divide (__reg[1]:__reg[0])/__reg[2] +// remainder in A, quotient in (__reg[1]:__reg[0]) noinline asm byte __mod_u16u8u16u8() { - ? LDA #0 ? LDX #15 ? CLC -__divmod_u16u8u16u8_start: +// TODO: decide when +#if OPTIMIZE_FOR_SPEED + ? LDA __reg+2 + ? BMI __divmod_u16u8u16u8_variant2 + ? LDA #0 +__divmod_u16u8u16u8_start: // for divisors <= 127 ? ROL __reg ? ROL __reg+1 ? ROL @@ -86,6 +91,25 @@ __divmod_u16u8u16u8_skip: ? ROL __reg ? ROL __reg+1 ? RTS +#endif +__divmod_u16u8u16u8_variant2: + ? LDA #0 +__divmod_u16u8u16u8_start2: // for divisors >= 128 + ? ROL __reg + ? ROL __reg+1 + ? ROL + ? BCS __divmod_u16u8u16u8_sub2 + ? CMP __reg+2 + ? BCC __divmod_u16u8u16u8_skip2 +__divmod_u16u8u16u8_sub2: + ? SBC __reg+2 + ? SEC +__divmod_u16u8u16u8_skip2: + ? DEX + ? BPL __divmod_u16u8u16u8_start2 + ? ROL __reg + ? ROL __reg+1 + ? RTS } asm word __div_u16u8u16u8() { @@ -93,6 +117,7 @@ asm word __div_u16u8u16u8() { ? LDA __reg ? LDX __reg+1 ? RTS + } #endif diff --git a/src/main/scala/millfork/compiler/mos/PseudoregisterBuiltIns.scala b/src/main/scala/millfork/compiler/mos/PseudoregisterBuiltIns.scala index 913a3fda..ddea5935 100644 --- a/src/main/scala/millfork/compiler/mos/PseudoregisterBuiltIns.scala +++ b/src/main/scala/millfork/compiler/mos/PseudoregisterBuiltIns.scala @@ -699,19 +699,19 @@ object PseudoregisterBuiltIns { constPart } - def usesRegLo(code: List[AssemblyLine]): Boolean = code.forall{ + def usesRegLo(code: List[AssemblyLine]): Boolean = code.exists{ case AssemblyLine0(JSR | BSR | TCD | TDC, _, _) => true case AssemblyLine0(_, _, MemoryAddressConstant(th)) if th.name == "__reg" => true case _ => false } - def usesRegHi(code: List[AssemblyLine]): Boolean = code.forall{ + def usesRegHi(code: List[AssemblyLine]): Boolean = code.exists{ case AssemblyLine0(JSR | BSR | TCD | TDC, _, _) => true case AssemblyLine0(_, _, CompoundConstant(MathOperator.Plus, MemoryAddressConstant(th), NumericConstant(1, _))) if th.name == "__reg" => true case _ => false } - def usesReg2(code: List[AssemblyLine]): Boolean = code.forall{ + def usesReg2(code: List[AssemblyLine]): Boolean = code.exists{ case AssemblyLine0(JSR | BSR | TCD | TDC, _, _) => true case AssemblyLine0(_, _, CompoundConstant(MathOperator.Plus, MemoryAddressConstant(th), NumericConstant(2, _))) if th.name == "__reg" => true case _ => false diff --git a/src/test/scala/millfork/test/ByteMathSuite.scala b/src/test/scala/millfork/test/ByteMathSuite.scala index dd481588..1f69dbc6 100644 --- a/src/test/scala/millfork/test/ByteMathSuite.scala +++ b/src/test/scala/millfork/test/ByteMathSuite.scala @@ -442,4 +442,23 @@ class ByteMathSuite extends FunSuite with Matchers with AppendedClues { m.readByte(0xc000) should equal(93) } } + + test("Multiplication bug repro") { + EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80)( + """ + | import zp_reg + | byte output @$c000 + | array zeroes[256] = [for i,0,until,256 [0]] + | void main () { + | byte a,b + | a = 5 + | b = 5 + | memory_barrier() + | output = a * (b * a) + | } + """. + stripMargin) { m => + m.readByte(0xc000) should equal(125) + } + } } diff --git a/src/test/scala/millfork/test/WordMathSuite.scala b/src/test/scala/millfork/test/WordMathSuite.scala index 7afa7266..6c9cd0ef 100644 --- a/src/test/scala/millfork/test/WordMathSuite.scala +++ b/src/test/scala/millfork/test/WordMathSuite.scala @@ -470,10 +470,20 @@ class WordMathSuite extends FunSuite with Matchers with AppendedClues { divisionCase1(1, 1) divisionCase1(1, 5) divisionCase1(6, 5) - divisionCase2(420, 11) - divisionCase2(1210, 11) - divisionCase2(35000, 45) - divisionCase2(51462, 1) + divisionCase1(420, 11) + divisionCase1(1210, 11) + divisionCase1(35000, 45) + divisionCase1(127, 127) + divisionCase1(128, 127) + divisionCase1(51462, 1) + } + + test("Word division 1 – large divisor") { + divisionCase1(127, 128) + divisionCase1(128, 128) + divisionCase1(45323, 250) + divisionCase1(4530, 200) + divisionCase1(9039, 247) } private def divisionCase1(x: Int, y: Int): Unit = { @@ -515,6 +525,8 @@ class WordMathSuite extends FunSuite with Matchers with AppendedClues { divisionCase2(6, 5) divisionCase2(73, 5) divisionCase2(73, 8) + divisionCase2(127, 127) + divisionCase2(128, 127) divisionCase2(75, 5) divisionCase2(42, 11) divisionCase2(420, 11) @@ -525,6 +537,14 @@ class WordMathSuite extends FunSuite with Matchers with AppendedClues { divisionCase2(51462, 1) } + test("Word division 2 – large divisor") { + divisionCase2(127, 128) + divisionCase2(128, 128) + divisionCase2(45323, 250) + divisionCase2(4530, 200) + divisionCase2(9039, 247) + } + private def divisionCase2(x: Int, y: Int): Unit = { EmuCrossPlatformBenchmarkRun(Cpu.Mos, Cpu.Z80, Cpu.Intel8080, Cpu.Sharp, Cpu.Intel8086)( s""" @@ -566,14 +586,18 @@ class WordMathSuite extends FunSuite with Matchers with AppendedClues { divisionCase4(1, 4) divisionCase4(6, 8) divisionCase4(73, 16) + divisionCase4(2534, 2) + divisionCase4(2534, 32) + divisionCase4(35000, 2) + divisionCase4(51462, 4) + divisionCase4(51462, 1) + } + + test("Word division 4 – large divisor") { divisionCase4(75, 128) divisionCase4(42, 128) divisionCase4(142, 128) - divisionCase2(2534, 2) - divisionCase2(2534, 32) - divisionCase2(35000, 2) - divisionCase2(51462, 4) - divisionCase2(51462, 1) + divisionCase4(9039, 247) } private def divisionCase4(x: Int, y: Int): Unit = {