Menu

#444 moduschar for sm83

None
closed-fixed
None
5
2022-12-13
2022-10-29
Job Bolle
No

moduschar for sm83 is not correct:
The dividend and divisor are passed in A and E respectively,
but E gets overwritten.
Changing the setup of __moduschar to be the same as in __divuschar fixes it:

--- a/device/lib/sm83/divint.s
+++ b/device/lib/sm83/divint.s
@@ -72,10 +72,9 @@ __divuschar:
        jp      .div16

 __moduschar:
-       ld      e, a
        ld      d, #0

-        ld      a,c             ; Sign extend
+        ld      c,a             ; Sign extend
         rlca
         sbc     a
         ld      b,a

Interestingly, none of the regression tests fail on this. Not even onebyte.
The following extra test will make onebyte fail:

--- a/support/regression/tests/onebyte.c.in
+++ b/support/regression/tests/onebyte.c.in
@@ -119,6 +119,8 @@ testMod (void)
    cL =  127;  cR =   -5; r16 =  cL %  cR; ASSERT (r16 ==  2);
    r16 = -1;
    cL =  127; ucR =    5; r16 =  cL % ucR; ASSERT (r16 ==  2);
+   r16 = -1;
+   cL =  127; ucR =    6; r16 =  cL % ucR; ASSERT (r16 ==  1);

    cL = -128;  cR =    5; r16 =  cL %  cR; ASSERT (r16 == -3);
    r16 = 0;

The following extra test will make muldiv fail:

--- a/support/regression/tests/muldiv.orig.c.in
+++ b/support/regression/tests/muldiv.c.in
@@ -148,6 +148,7 @@ testMod(void)
     i = 100;
     //    LOG(("i%%17 == 15 = %u\n", (int)(i%9)));
     ASSERT(i%17 == 15);
+    ASSERT(i%(unsigned char)19 == 5);

     //    LOG(("i%%-7 == 2 = %u\n", (int)i%-7));
     ASSERT(i%-7 == 2);

The muldiv test is the one that started failing for me when I made changes to the compiler to use 8 but arithmetic instead of 16 bit when possible, and that prompted me to look into this.

Another interesting thing in muldiv is that if I use the same numbers in this new test as in the previous test (17 and 15 instead of 19 and 5) then the test does not fail, but if I move it up to before the previous test, it does fail.

Discussion

  • Philipp Klaus Krause

    • assigned_to: Philipp Klaus Krause
    • Group: -->
     
  • Philipp Klaus Krause

    • status: open --> closed-fixed
     
  • Philipp Klaus Krause

    Thanks. Fixed in [r13764] via your proposed patch.

     

    Related

    Commit: [r13764]


Log in to post a comment.