dschmenk / PLASMA

Proto Language AsSeMbler for All (formerly Apple)
MIT License
191 stars 26 forks source link

DIVMOD inconsistent with %? #40

Closed ZornsLemma closed 6 years ago

ZornsLemma commented 6 years ago

Hi Dave,

I haven't been able to test this on the Apple version (I couldn't find a disk image and I was too lazy to install CiderPress...) but based on playing around with my Acorn port, I think there's a small bug here.

Using this test program:

include "inc/cmdsys.plh"
word p = 1
word q = -8
word a, b, c, d
a = p / q
b = p % q
c, d = divmod(p, q)
puti(a); putc(' '); puti(b); putln
puti(c); putc(' '); puti(d); putln
done

I get the following output:

0 -1
0 1

I'd expect the two methods of calculating to give the same result. I think the problem is in DIVMOD:

DIVMOD  JSR     _DIV
        LSR     DVSIGN          ; SIGN(RESULT) = (SIGN(DIVIDEND) + SIGN(DIVISOR)) & 1
        BCC     +
        JSR     _NEG
+       DEX
        LDA     TMPL            ; REMNDRL
        STA     ESTKL,X
        LDA     TMPH            ; REMNDRH
        STA     ESTKH,X
        LDA     DVSIGN          ; REMAINDER IS SIGN OF DIVIDEND 
        BMI     NEG
        JMP     NEXTOP

The 'LDA DVSIGN ; REMAINDER IS SIGN OF DIVIDEND' is loading the value of DVSIGN, but the earlier 'LSR DVSIGN' has shifted it right, so what was in b7 is now in b6. I think the correct version would finish:

        ASL     DVSIGN          ; REMAINDER IS SIGN OF DIVIDEND 
        BMI     NEG
        JMP     NEXTOP

For me at least, this makes the two consistent. The test program then outputs:

0 -1
0 -1

That said - and I could well be getting confused here - it's not clear to me that the result given by % is correct. I think (using the variable names from that sample code above) it should be true that

(a*q)+b==p

but the left hand side works out to be -1 while p is 1.

I hope this makes sense and apologies if this is something specific to my port after all.

Cheers.

Steve

dschmenk commented 6 years ago

Yep, looks like I screwed up. I only tested the case of the dividend being negative. Thanks for checking this, I'll get the fix in later.

dschmenk commented 6 years ago

Hi Steve- Can you send me an email at dschmenk at gmail.com? I want to run some ideas past you. Thanks!

dschmenk commented 6 years ago

So I had it conceptually correct, but my implementation used the sign of the divisor instead of the dividend as the sign of the result. Test case and portable VM also updated.