Rewrite status checks with macros

This also forces the result of operations to be considered purely as
8-bit results in _some_ cases, but not in others. We were missing some
times when an overflow is the expected behavior; because SET_RESULT()
and mos6502_modify_status() use ints, this overflow was not properly
observed.

include/mos6502.h

@@ -53,6 +53,37 @@
 #define DECL_INST(x) \
     extern void mos6502_handle_##x (mos6502 *, vm_8bit)
 
+#define MOS_CHECK_Z(result) \
+    cpu->P &= ~MOS_ZERO; \
+    if ((vm_8bit)(result) == 0) cpu->P |= MOS_ZERO
+
+#define MOS_CHECK_N(result) \
+    cpu->P &= ~MOS_NEGATIVE; \
+    if ((vm_8bit)(result) & 0x80) cpu->P |= MOS_NEGATIVE
+
+#define MOS_CHECK_V(orig, result) \
+    cpu->P &= ~MOS_OVERFLOW; \
+    do { \
+        vm_8bit r = result; \
+        vm_8bit o = orig; \
+        if ((o & 0x80) ^ (r & 0x80)) { \
+            cpu->P |= MOS_OVERFLOW; \
+        } \
+    } while (0)
+
+#define MOS_CHECK_NV(orig, result) \
+    MOS_CHECK_N(result); \
+    MOS_CHECK_V(orig, result)
+
+#define MOS_CHECK_NZ(result) \
+    MOS_CHECK_N(result); \
+    MOS_CHECK_Z(result)
+
+#define MOS_CHECK_NVZ(orig, result) \
+    MOS_CHECK_N(result); \
+    MOS_CHECK_V(orig, result); \
+    MOS_CHECK_Z(result)
+
 typedef struct {
     /*
      * There are two different segment pointers for reading and writing,

src/mos6502.arith.c

@@ -21,11 +21,19 @@ DEFINE_INST(adc)
     }
 
     MOS_CARRY_BIT();
-    SET_RESULT(cpu->A + oper + carry);
 
-    mos6502_modify_status(cpu, MOS_NVZC, cpu->A, result);
+    vm_8bit result = cpu->A + oper + carry;
+    MOS_CHECK_NVZ(cpu->A, result);
 
-    cpu->A = result & 0xff;
+    // Carry has different meanings in different contexts... in ADC,
+    // carry is set if the result requires a ninth bit (the carry bit!)
+    // to be set.
+    cpu->P &= ~MOS_CARRY;
+    if ((cpu->A + oper + carry) > 0xff) {
+        cpu->P |= MOS_CARRY;
+    }
+    
+    cpu->A = result;
 }
 
 /*
@@ -65,17 +73,16 @@ DEFINE_INST(adc_dec)
     // And the final result has to be ported back into a "hexadecimal"
     // number; you see, BCD values are not just literally decimal, they
     // are decimal in hexadecimal form.
-    SET_RESULT(((modsum / 10) << 4) | (modsum % 10));
+    vm_8bit result = ((modsum / 10) << 4) | (modsum % 10);
 
-    // Because the rules for carry is a bit different, we'll handle it
+    // As you can see, decimal comports a different meaning for the
-    // here.
+    // carry bit than its binary version
     cpu->P &= ~MOS_CARRY;
     if (sum > 100) {
         cpu->P |= MOS_CARRY;
     }
 
-    // We'll check zero, but not negative or overflow.
+    MOS_CHECK_Z(result);
-    mos6502_modify_status(cpu, MOS_ZERO, cpu->A, sum);
 
     cpu->A = result;
 }
@@ -89,10 +96,10 @@ DEFINE_INST(adc_dec)
  */
 DEFINE_INST(cmp)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->A, cpu->A - oper);
+    MOS_CHECK_NZ(cpu->A - oper);
 
-    // Carry works slightly different with the cmp-style instructions;
+    // With CMP, carry is set if the difference between A and oper is
-    // it's set if A >= oper.
+    // not zero.
     cpu->P &= ~MOS_CARRY;
     if (cpu->A >= oper) {
         cpu->P |= MOS_CARRY;
@@ -105,7 +112,7 @@ DEFINE_INST(cmp)
  */
 DEFINE_INST(cpx)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->X, cpu->X - oper);
+    MOS_CHECK_NZ(cpu->X - oper);
 
     cpu->P &= ~MOS_CARRY;
     if (cpu->X >= oper) {
@@ -119,7 +126,7 @@ DEFINE_INST(cpx)
  */
 DEFINE_INST(cpy)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->Y, cpu->Y - oper);
+    MOS_CHECK_NZ(cpu->Y - oper);
 
     cpu->P &= ~MOS_CARRY;
     if (cpu->Y >= oper) {
@@ -134,7 +141,7 @@ DEFINE_INST(cpy)
 DEFINE_INST(dec) 
 {
     if (cpu->eff_addr) {
-        mos6502_modify_status(cpu, MOS_NZ, oper, oper - 1);
+        MOS_CHECK_NZ(oper - 1);
         mos6502_set(cpu, cpu->eff_addr, oper - 1);
         return;
     }
@@ -149,7 +156,7 @@ DEFINE_INST(dec)
  */
 DEFINE_INST(dex)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->X, cpu->X - 1);
+    MOS_CHECK_NZ(cpu->X - 1);
     cpu->X--;
 }
 
@@ -158,7 +165,7 @@ DEFINE_INST(dex)
  */
 DEFINE_INST(dey)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->Y, cpu->Y - 1);
+    MOS_CHECK_NZ(cpu->Y - 1);
     cpu->Y--;
 }
 
@@ -168,7 +175,7 @@ DEFINE_INST(dey)
 DEFINE_INST(inc)
 {
     if (cpu->eff_addr) {
-        mos6502_modify_status(cpu, MOS_NZ, oper, oper + 1);
+        MOS_CHECK_NZ(oper + 1);
         mos6502_set(cpu, cpu->eff_addr, oper + 1);
         return;
     }
@@ -181,7 +188,7 @@ DEFINE_INST(inc)
  */
 DEFINE_INST(inx)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->X, cpu->X + 1);
+    MOS_CHECK_NZ(cpu->X + 1);
     cpu->X++;
 }
 
@@ -190,7 +197,7 @@ DEFINE_INST(inx)
  */
 DEFINE_INST(iny)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->Y, cpu->Y + 1);
+    MOS_CHECK_NZ(cpu->Y + 1);
     cpu->Y++;
 }
 
@@ -209,7 +216,9 @@ DEFINE_INST(sbc)
     }
 
     MOS_CARRY_BIT();
-    SET_RESULT(cpu->A - oper - (carry ? 0 : 1));
+
+    vm_8bit result = cpu->A - oper - (carry ? 0 : 1);
+    MOS_CHECK_NVZ(cpu->A, result);
 
     // Carry is handled slightly differently in SBC; it's set if the
     // value is non-negative, and unset if negative. (It's essentially a
@@ -219,8 +228,7 @@ DEFINE_INST(sbc)
         cpu->P &= ~MOS_CARRY;
     }
 
-    mos6502_modify_status(cpu, MOS_NVZ, cpu->A, result);
+    cpu->A = result;
-    cpu->A = result & 0xff;
 }
 
 /*
@@ -267,9 +275,8 @@ DEFINE_INST(sbc_dec)
     // And the final result has to be ported back into a "hexadecimal"
     // number; you see, BCD values are not just literally decimal, they
     // are decimal in hexadecimal form.
-    SET_RESULT(((diff / 10) << 4) | (diff % 10));
+    vm_8bit result = ((diff / 10) << 4) | (diff % 10);
-
+    MOS_CHECK_Z(result);
-    mos6502_modify_status(cpu, MOS_ZERO, cpu->A, result);
 
     cpu->A = result;
 }

src/mos6502.bits.c

@@ -14,10 +14,8 @@
  */
 DEFINE_INST(and)
 {
-    SET_RESULT(cpu->A & oper);
+    MOS_CHECK_NZ(cpu->A & oper);
-
+    cpu->A &= oper;
-    mos6502_modify_status(cpu, MOS_NZ, cpu->A, result);
-    cpu->A = result & 0xff;
 }
 
 /*
@@ -33,14 +31,18 @@ DEFINE_INST(and)
  */
 DEFINE_INST(asl)
 {
-    SET_RESULT(oper << 1);
+    vm_8bit result = oper << 1;
 
-    mos6502_modify_status(cpu, MOS_NZC, oper, result);
+    MOS_CHECK_NZ(result);
+    cpu->P &= ~MOS_CARRY;
+    if ((oper << 1) > 0xff) {
+        cpu->P |= MOS_CARRY;
+    }
 
     if (cpu->eff_addr) {
-        mos6502_set(cpu, cpu->eff_addr, result & 0xff);
+        mos6502_set(cpu, cpu->eff_addr, result);
     } else {
-        cpu->A = result & 0xff;
+        cpu->A = result;
     }
 }
 
@@ -96,10 +98,8 @@ DEFINE_INST(bim)
  */
 DEFINE_INST(eor)
 {
-    SET_RESULT(cpu->A ^ oper);
+    MOS_CHECK_NZ(cpu->A ^ oper);
-
+    cpu->A ^= oper;
-    mos6502_modify_status(cpu, MOS_NZ, cpu->A, result);
-    cpu->A = result & 0xff;
 }
 
 /*
@@ -111,27 +111,25 @@ DEFINE_INST(eor)
  */
 DEFINE_INST(lsr)
 {
-    SET_RESULT(oper >> 1);
+    vm_8bit result = oper >> 1;
 
-    // Set ZERO if it should apply; also inspect the NEGATIVE flag, but
+    // The N flag is ALWAYS cleared in LSR, because a zero is always
-    // note that doing so will have the effect of _always_ unsetting the
+    // entered as bit 7
-    // N flag, because literally any byte value shifted right will
+    cpu->P &= ~MOS_NEGATIVE;
-    // immediately lose the sign bit and be non-negative.
-    mos6502_modify_status(cpu, MOS_NZ, oper, result);
 
-    // However, we handle carry a bit differently here. The carry bit
+    MOS_CHECK_Z(result);
-    // should be 1 if oper & 0x1; that is, when we shift right, we want
+
-    // the right-most bit to be captured in carry, in just the same way
+    // Carry is set to the value of the bit we're "losing" in the shift
-    // we want the left-most bit to be captured in carry for ASL.
+    // operation
     cpu->P &= ~MOS_CARRY;
     if (oper & 0x1) {
         cpu->P |= MOS_CARRY;
     }
 
     if (cpu->eff_addr) {
-        mos6502_set(cpu, cpu->eff_addr, result & 0xff);
+        mos6502_set(cpu, cpu->eff_addr, result);
     } else {
-        cpu->A = result & 0xff;
+        cpu->A = result;
     }
 }
 
@@ -141,10 +139,8 @@ DEFINE_INST(lsr)
  */
 DEFINE_INST(ora)
 {
-    SET_RESULT(cpu->A | oper);
+    MOS_CHECK_NZ(cpu->A | oper);
-
+    cpu->A |= oper;
-    mos6502_modify_status(cpu, MOS_NZ, cpu->A, result);
-    cpu->A = result & 0xff;
 }
 
 /*
@@ -154,7 +150,7 @@ DEFINE_INST(ora)
  */
 DEFINE_INST(rol)
 {
-    SET_RESULT(oper << 1);
+    vm_8bit result = oper << 1;
 
     // Rotations are effectively _9-bit_. So we aren't rotating bit 7
     // into bit 0; we're rotating bit 7 into the carry bit, and we're
@@ -168,12 +164,12 @@ DEFINE_INST(rol)
         cpu->P |= MOS_CARRY;
     }
 
-    mos6502_modify_status(cpu, MOS_NZ, oper, result);
+    MOS_CHECK_NZ(result);
 
     if (cpu->eff_addr) {
-        mos6502_set(cpu, cpu->eff_addr, result & 0xff);
+        mos6502_set(cpu, cpu->eff_addr, result);
     } else {
-        cpu->A = result & 0xff;
+        cpu->A = result;
     }
 }
 
@@ -183,7 +179,7 @@ DEFINE_INST(rol)
  */
 DEFINE_INST(ror)
 {
-    SET_RESULT(oper >> 1);
+    vm_8bit result = oper >> 1;
 
     // See the code for ROL for my note on 9-bit rotation (vs. 8-bit).
     if (cpu->P & MOS_CARRY) {
@@ -195,12 +191,12 @@ DEFINE_INST(ror)
         cpu->P |= MOS_CARRY;
     }
 
-    mos6502_modify_status(cpu, MOS_NZ, oper, result);
+    MOS_CHECK_NZ(result);
 
     if (cpu->eff_addr) {
-        mos6502_set(cpu, cpu->eff_addr, result & 0xff);
+        mos6502_set(cpu, cpu->eff_addr, result);
     } else {
-        cpu->A = result & 0xff;
+        cpu->A = result;
     }
 }
 

src/mos6502.loadstor.c

@@ -14,7 +14,7 @@
  */
 DEFINE_INST(lda)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->A, oper);
+    MOS_CHECK_NZ(oper);
     cpu->A = oper;
 }
 
@@ -23,7 +23,7 @@ DEFINE_INST(lda)
  */
 DEFINE_INST(ldx)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->X, oper);
+    MOS_CHECK_NZ(oper);
     cpu->X = oper;
 }
 
@@ -32,7 +32,7 @@ DEFINE_INST(ldx)
  */
 DEFINE_INST(ldy)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->Y, oper);
+    MOS_CHECK_NZ(oper);
     cpu->Y = oper;
 }
 
@@ -74,9 +74,9 @@ DEFINE_INST(phy)
  */
 DEFINE_INST(pla)
 {
-    SET_RESULT(mos6502_pop_stack(cpu));
+    vm_8bit result = mos6502_pop_stack(cpu);
 
-    mos6502_modify_status(cpu, MOS_NZ, cpu->A, result);
+    MOS_CHECK_NZ(result);
     cpu->A = result;
 }
 
@@ -93,9 +93,9 @@ DEFINE_INST(plp)
  */
 DEFINE_INST(plx)
 {
-    SET_RESULT(mos6502_pop_stack(cpu));
+    vm_8bit result = mos6502_pop_stack(cpu);
 
-    mos6502_modify_status(cpu, MOS_NZ, cpu->X, result);
+    MOS_CHECK_NZ(result);
     cpu->X = result;
 }
 
@@ -104,9 +104,9 @@ DEFINE_INST(plx)
  */
 DEFINE_INST(ply)
 {
-    SET_RESULT(mos6502_pop_stack(cpu));
+    vm_8bit result = mos6502_pop_stack(cpu);
 
-    mos6502_modify_status(cpu, MOS_NZ, cpu->Y, result);
+    MOS_CHECK_NZ(result);
     cpu->Y = result;
 }
 
@@ -149,7 +149,7 @@ DEFINE_INST(stz)
  */
 DEFINE_INST(tax)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->X, cpu->A);
+    MOS_CHECK_NZ(cpu->A);
     cpu->X = cpu->A;
 }
 
@@ -158,7 +158,7 @@ DEFINE_INST(tax)
  */
 DEFINE_INST(tay)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->Y, cpu->A);
+    MOS_CHECK_NZ(cpu->A);
     cpu->Y = cpu->A;
 }
 
@@ -167,7 +167,7 @@ DEFINE_INST(tay)
  */
 DEFINE_INST(tsx)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->X, cpu->S);
+    MOS_CHECK_NZ(cpu->S);
     cpu->X = cpu->S;
 }
 
@@ -176,7 +176,7 @@ DEFINE_INST(tsx)
  */
 DEFINE_INST(txa)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->A, cpu->X);
+    MOS_CHECK_NZ(cpu->X);
     cpu->A = cpu->X;
 }
 
@@ -185,7 +185,7 @@ DEFINE_INST(txa)
  */
 DEFINE_INST(txs)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->S, cpu->X);
+    MOS_CHECK_NZ(cpu->X);
     cpu->S = cpu->X;
 }
 
@@ -194,6 +194,6 @@ DEFINE_INST(txs)
  */
 DEFINE_INST(tya)
 {
-    mos6502_modify_status(cpu, MOS_NZ, cpu->A, cpu->Y);
+    MOS_CHECK_NZ(cpu->Y);
     cpu->A = cpu->Y;
 }