6502: Fix word division and byte multiplication

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:

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

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

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)
+    }
+  }
 }

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)
+    divisionCase1(420, 11)
-    divisionCase2(1210, 11)
+    divisionCase1(1210, 11)
-    divisionCase2(35000, 45)
+    divisionCase1(35000, 45)
-    divisionCase2(51462, 1)
+    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)
+    divisionCase4(9039, 247)
-    divisionCase2(2534, 32)
-    divisionCase2(35000, 2)
-    divisionCase2(51462, 4)
-    divisionCase2(51462, 1)
   }
 
   private def divisionCase4(x: Int, y: Int): Unit = {