From fea9257e843e3b9f8cfe4a36f1b47e98c598caf3 Mon Sep 17 00:00:00 2001
From: Bradford Lamson-Scribner <brad.lamson@gmail.com>
Date: Sun, 31 May 2020 22:20:08 -0600
Subject: [PATCH] internal/vm/{exec_funcs,vm}: refactor flag get/set/clears

Hit me like a brick wall, remove all the specific flag get/set/clear funcs
in favor of a single getFlag, setFlag, and clearFlag funcs that take a flag
(byte) as its argument.
---
 internal/vm/exec_funcs.go | 186 +++++++++++++++++++-------------------
 internal/vm/vm.go         |  98 ++++++--------------
 2 files changed, 119 insertions(+), 165 deletions(-)

diff --git a/internal/vm/exec_funcs.go b/internal/vm/exec_funcs.go
index d97d129..217938d 100644
--- a/internal/vm/exec_funcs.go
+++ b/internal/vm/exec_funcs.go
@@ -40,8 +40,8 @@ func execDEC(a *Appleone, o op) error {
 	b := a.mem[addr]
 	b--
 	a.mem[addr] = b
-	a.setZeroIfNeeded(b)
-	a.setNegativeIfOverflow(b)
+	a.maybeSetFlagZero(b)
+	a.maybeHandleOverflow(b)
 	return nil
 }
 
@@ -55,8 +55,8 @@ func execINC(a *Appleone, o op) error {
 	b := a.mem[addr]
 	b++
 	a.mem[addr] = b
-	a.setZeroIfNeeded(b)
-	a.setNegativeIfOverflow(b)
+	a.maybeSetFlagZero(b)
+	a.maybeHandleOverflow(b)
 	return nil
 }
 
@@ -64,8 +64,8 @@ func execINC(a *Appleone, o op) error {
 //                                  + + - - - -
 func execINX(a *Appleone, o op) error {
 	a.cpu.x++
-	a.setZeroIfNeeded(a.cpu.x)
-	a.setNegativeIfOverflow(a.cpu.x)
+	a.maybeSetFlagZero(a.cpu.x)
+	a.maybeHandleOverflow(a.cpu.x)
 	return nil
 }
 
@@ -73,8 +73,8 @@ func execINX(a *Appleone, o op) error {
 //                                  + + - - - -
 func execINY(a *Appleone, o op) error {
 	a.cpu.y++
-	a.setZeroIfNeeded(a.cpu.y)
-	a.setNegativeIfOverflow(a.cpu.y)
+	a.maybeSetFlagZero(a.cpu.y)
+	a.maybeHandleOverflow(a.cpu.y)
 	return nil
 }
 
@@ -82,8 +82,8 @@ func execINY(a *Appleone, o op) error {
 //                                  + + - - - -
 func execTAX(a *Appleone, o op) error {
 	a.cpu.x = a.cpu.a
-	a.setZeroIfNeeded(a.cpu.x)
-	a.setNegativeIfOverflow(a.cpu.x)
+	a.maybeSetFlagZero(a.cpu.x)
+	a.maybeHandleOverflow(a.cpu.x)
 	return nil
 }
 
@@ -91,8 +91,8 @@ func execTAX(a *Appleone, o op) error {
 //                                  + + - - - -
 func execTAY(a *Appleone, o op) error {
 	a.cpu.y = a.cpu.a
-	a.setZeroIfNeeded(a.cpu.y)
-	a.setNegativeIfOverflow(a.cpu.y)
+	a.maybeSetFlagZero(a.cpu.y)
+	a.maybeHandleOverflow(a.cpu.y)
 	return nil
 }
 
@@ -100,8 +100,8 @@ func execTAY(a *Appleone, o op) error {
 //                                  + + - - - -
 func execDEX(a *Appleone, o op) error {
 	a.cpu.x--
-	a.setZeroIfNeeded(a.cpu.x)
-	a.setNegativeIfOverflow(a.cpu.x)
+	a.maybeSetFlagZero(a.cpu.x)
+	a.maybeHandleOverflow(a.cpu.x)
 	return nil
 }
 
@@ -109,8 +109,8 @@ func execDEX(a *Appleone, o op) error {
 //                                  + + - - - -
 func execDEY(a *Appleone, o op) error {
 	a.cpu.y--
-	a.setZeroIfNeeded(a.cpu.y)
-	a.setNegativeIfOverflow(a.cpu.y)
+	a.maybeSetFlagZero(a.cpu.y)
+	a.maybeHandleOverflow(a.cpu.y)
 	return nil
 }
 
@@ -122,8 +122,8 @@ func execLDA(a *Appleone, o op) error {
 		return err
 	}
 	a.cpu.a = operand
-	a.setZeroIfNeeded(a.cpu.a)
-	a.setNegativeIfOverflow(a.cpu.a)
+	a.maybeSetFlagZero(a.cpu.a)
+	a.maybeHandleOverflow(a.cpu.a)
 	return nil
 }
 
@@ -135,8 +135,8 @@ func execLDX(a *Appleone, o op) error {
 		return err
 	}
 	a.cpu.x = operand
-	a.setZeroIfNeeded(a.cpu.x)
-	a.setNegativeIfOverflow(a.cpu.x)
+	a.maybeSetFlagZero(a.cpu.x)
+	a.maybeHandleOverflow(a.cpu.x)
 	return nil
 }
 
@@ -148,8 +148,8 @@ func execLDY(a *Appleone, o op) error {
 		return err
 	}
 	a.cpu.y = operand
-	a.setZeroIfNeeded(a.cpu.y)
-	a.setNegativeIfOverflow(a.cpu.y)
+	a.maybeSetFlagZero(a.cpu.y)
+	a.maybeHandleOverflow(a.cpu.y)
 	return nil
 }
 
@@ -162,22 +162,22 @@ func execADC(a *Appleone, o op) error {
 	}
 	operand := uint16(b)
 	regA := uint16(a.cpu.a)
-	sum := regA + operand + uint16(a.getCarry())
+	sum := regA + operand + uint16(a.getFlag(flagCarry))
 	a.cpu.a = byte(sum)
 
-	a.clearCarry()
+	a.clearFlag(flagCarry)
 	if sum > 255 {
-		a.setCarry()
+		a.setFlag(flagCarry)
 	}
 
 	// http://www.righto.com/2012/12/the-6502-overflow-flag-explained.html
-	a.clearOverflow()
+	a.clearFlag(flagOverflow)
 	if (operand^sum)&(regA^sum)&0x80 != 0 {
-		a.setOverflow()
+		a.setFlag(flagOverflow)
 	}
 
-	a.setZeroIfNeeded(a.cpu.a)
-	a.setNegativeIfOverflow(a.cpu.a)
+	a.maybeSetFlagZero(a.cpu.a)
+	a.maybeHandleOverflow(a.cpu.a)
 
 	return nil
 }
@@ -190,29 +190,29 @@ func execSBC(a *Appleone, o op) error {
 		return err
 	}
 
-	carry := uint16(1 - a.getCarry())
+	carry := uint16(1 - a.getFlag(flagCarry))
 	regA := a.cpu.a
 	sum := uint16(regA) - carry - uint16(operand)
 	a.cpu.a = byte(sum)
-	a.clearOverflow()
+	a.clearFlag(flagOverflow)
 
 	if byte(regA)>>7 != a.cpu.a>>7 {
-		a.setOverflow()
+		a.setFlag(flagOverflow)
 	}
 
 	if uint16(sum) < 256 {
-		a.setCarry()
+		a.setFlag(flagCarry)
 	} else {
-		a.clearCarry()
+		a.clearFlag(flagCarry)
 	}
 
-	a.clearOverflow()
+	a.clearFlag(flagOverflow)
 	if ((255-operand)^a.cpu.a)&(regA^a.cpu.a)&0x80 != 0 {
-		a.setOverflow()
+		a.setFlag(flagOverflow)
 	}
 
-	a.setZeroIfNeeded(a.cpu.a)
-	a.setNegativeIfOverflow(a.cpu.a)
+	a.maybeSetFlagZero(a.cpu.a)
+	a.maybeHandleOverflow(a.cpu.a)
 	return nil
 }
 
@@ -252,7 +252,7 @@ func execSTA(a *Appleone, o op) error {
 // branch on Z = 1                  N Z C I D V
 //                                  - - - - - -
 func execBEQ(a *Appleone, o op) error {
-	if a.getZero() == flagZero {
+	if a.getFlag(flagZero) == flagZero {
 		a.branch(o)
 	}
 	return nil
@@ -261,7 +261,7 @@ func execBEQ(a *Appleone, o op) error {
 // branch on Z = 0                  N Z C I D V
 //                                  - - - - - -
 func execBNE(a *Appleone, o op) error {
-	if a.getZero() != flagZero {
+	if a.getFlag(flagZero) != flagZero {
 		a.branch(o)
 	}
 	return nil
@@ -270,7 +270,7 @@ func execBNE(a *Appleone, o op) error {
 // branch on V = 0                  N Z C I D V
 //                                  - - - - - -
 func execBVC(a *Appleone, o op) error {
-	if a.getOverflow() == 0 {
+	if a.getFlag(flagOverflow) == 0 {
 		a.branch(o)
 	}
 	return nil
@@ -279,7 +279,7 @@ func execBVC(a *Appleone, o op) error {
 // branch on V = 1                  N Z C I D V
 //                                  - - - - - -
 func execBVS(a *Appleone, o op) error {
-	if a.getOverflow() != 0 {
+	if a.getFlag(flagOverflow) != 0 {
 		a.branch(o)
 	}
 	return nil
@@ -292,21 +292,21 @@ func execBIT(a *Appleone, o op) error {
 	if err != nil {
 		return err
 	}
-	a.setZeroIfNeeded(a.cpu.a & operand)
-	a.clearOverflow()
+	a.maybeSetFlagZero(a.cpu.a & operand)
+	a.clearFlag(flagOverflow)
 
 	if operand&flagOverflow != 0 {
-		a.setOverflow()
+		a.setFlag(flagOverflow)
 	}
 
-	a.setNegativeIfOverflow(operand)
+	a.maybeHandleOverflow(operand)
 	return nil
 }
 
 // branch on C = 0                  N Z C I D V
 //                                  - - - - - -
 func execBCC(a *Appleone, o op) error {
-	if a.getCarry() == 0 {
+	if a.getFlag(flagCarry) == 0 {
 		a.branch(o)
 	}
 	return nil
@@ -315,7 +315,7 @@ func execBCC(a *Appleone, o op) error {
 // branch on N = 1                  N Z C I D V
 //                                  - - - - - -
 func execBMI(a *Appleone, o op) error {
-	if a.getNegative() == flagNegative {
+	if a.getFlag(flagNegative) == flagNegative {
 		a.branch(o)
 	}
 	return nil
@@ -324,7 +324,7 @@ func execBMI(a *Appleone, o op) error {
 // branch on N = 0                  N Z C I D V
 //                                  - - - - - -
 func execBPL(a *Appleone, o op) error {
-	if a.getNegative() == 0 {
+	if a.getFlag(flagNegative) == 0 {
 		a.branch(o)
 	}
 	return nil
@@ -333,7 +333,7 @@ func execBPL(a *Appleone, o op) error {
 // branch on C = 1                  N Z C I D V
 //                                  - - - - - -
 func execBCS(a *Appleone, o op) error {
-	if a.getCarry() != 0 {
+	if a.getFlag(flagCarry) != 0 {
 		a.branch(o)
 	}
 	return nil
@@ -358,8 +358,8 @@ func execEOR(a *Appleone, o op) error {
 		return err
 	}
 	a.cpu.a ^= operand
-	a.setNegativeIfOverflow(a.cpu.a)
-	a.setZeroIfNeeded(a.cpu.a)
+	a.maybeHandleOverflow(a.cpu.a)
+	a.maybeSetFlagZero(a.cpu.a)
 	return nil
 }
 
@@ -388,49 +388,49 @@ func execCPY(a *Appleone, o op) error {
 // 0 -> C                           N Z C I D V
 //                                  - - 0 - - -
 func execCLC(a *Appleone, o op) error {
-	a.clearCarry()
+	a.clearFlag(flagCarry)
 	return nil
 }
 
 // 0 -> D                           N Z C I D V
 //                                  - - - - 0 -
 func execCLD(a *Appleone, o op) error {
-	a.clearDec()
+	a.clearFlag(flagDecimalMode)
 	return nil
 }
 
 // 0 -> I                           N Z C I D V
 //                                  - - - 0 - -
 func execCLI(a *Appleone, o op) error {
-	a.clearInterrupt()
+	a.clearFlag(flagDisableInterrupts)
 	return nil
 }
 
 // 0 -> V                           N Z C I D V
 //                                  - - - - - 0
 func execCLV(a *Appleone, o op) error {
-	a.clearOverflow()
+	a.clearFlag(flagOverflow)
 	return nil
 }
 
 // 1 -> C                           N Z C I D V
 //                                  - - 1 - - -
 func execSEC(a *Appleone, o op) error {
-	a.setCarry()
+	a.setFlag(flagCarry)
 	return nil
 }
 
 // 1 -> D                           N Z C I D V
 //                                  - - - - 1 -
 func execSED(a *Appleone, o op) error {
-	a.setDec()
+	a.setFlag(flagDecimalMode)
 	return nil
 }
 
 // 1 -> I                           N Z C I D V
 //                                  - - - 1 - -
 func execSEI(a *Appleone, o op) error {
-	a.setInterrupt()
+	a.setFlag(flagDisableInterrupts)
 	return nil
 }
 
@@ -470,8 +470,8 @@ func execPHA(a *Appleone, o op) error {
 //                                  + + - - - -
 func execTXA(a *Appleone, o op) error {
 	a.cpu.a = a.cpu.x
-	a.setNegativeIfOverflow(a.cpu.a)
-	a.setZeroIfNeeded(a.cpu.a)
+	a.maybeHandleOverflow(a.cpu.a)
+	a.maybeSetFlagZero(a.cpu.a)
 	return nil
 }
 
@@ -479,8 +479,8 @@ func execTXA(a *Appleone, o op) error {
 //                                  + + - - - -
 func execTYA(a *Appleone, o op) error {
 	a.cpu.a = a.cpu.y
-	a.setNegativeIfOverflow(a.cpu.a)
-	a.setZeroIfNeeded(a.cpu.a)
+	a.maybeHandleOverflow(a.cpu.a)
+	a.maybeSetFlagZero(a.cpu.a)
 	return nil
 }
 
@@ -488,8 +488,8 @@ func execTYA(a *Appleone, o op) error {
 //                                  + + - - - -
 func execTSX(a *Appleone, o op) error {
 	a.cpu.x = a.cpu.sp
-	a.setNegativeIfOverflow(a.cpu.x)
-	a.setZeroIfNeeded(a.cpu.x)
+	a.maybeHandleOverflow(a.cpu.x)
+	a.maybeSetFlagZero(a.cpu.x)
 	return nil
 }
 
@@ -497,16 +497,16 @@ func execTSX(a *Appleone, o op) error {
 //                                  + + - - - -
 func execPLA(a *Appleone, o op) error {
 	a.cpu.a = a.popStackWord()
-	a.setNegativeIfOverflow(a.cpu.a)
-	a.setZeroIfNeeded(a.cpu.a)
+	a.maybeHandleOverflow(a.cpu.a)
+	a.maybeSetFlagZero(a.cpu.a)
 	return nil
 }
 
 // pull SR from stack                  N Z C I D V
 func execPLP(a *Appleone, o op) error {
 	a.cpu.ps = a.popStackWord() | 0B_00110000
-	a.setNegativeIfOverflow(a.cpu.a)
-	a.setZeroIfNeeded(a.cpu.a)
+	a.maybeHandleOverflow(a.cpu.a)
+	a.maybeSetFlagZero(a.cpu.a)
 	return nil
 }
 
@@ -549,13 +549,13 @@ func execLSR(a *Appleone, o op) error {
 	operand >>= 1
 
 	if bit {
-		a.setCarry()
+		a.setFlag(flagCarry)
 	} else {
-		a.clearCarry()
+		a.clearFlag(flagCarry)
 	}
 
-	a.setZeroIfNeeded(operand)
-	a.setNegativeIfOverflow(operand)
+	a.maybeSetFlagZero(operand)
+	a.maybeHandleOverflow(operand)
 
 	if o.addrMode == accumulator {
 		a.cpu.a = operand
@@ -580,14 +580,14 @@ func execROL(a *Appleone, o op) error {
 	}
 
 	var carry bool
-	if a.getCarry() == flagCarry {
+	if a.getFlag(flagCarry) == flagCarry {
 		carry = true
 	}
 
 	if operand>>7 != 0 {
-		a.setCarry()
+		a.setFlag(flagCarry)
 	} else {
-		a.clearCarry()
+		a.clearFlag(flagCarry)
 	}
 
 	operand <<= 1
@@ -596,8 +596,8 @@ func execROL(a *Appleone, o op) error {
 		operand |= flagCarry
 	}
 
-	a.setZeroIfNeeded(operand)
-	a.setNegativeIfOverflow(operand)
+	a.maybeSetFlagZero(operand)
+	a.maybeHandleOverflow(operand)
 
 	if o.addrMode == accumulator {
 		a.cpu.a = operand
@@ -618,8 +618,8 @@ func execROL(a *Appleone, o op) error {
 func execTXS(a *Appleone, o op) error {
 	a.cpu.sp = a.cpu.x
 	// TODO: needed?
-	// a.setZeroIfNeeded(a.cpu.sp)
-	// a.setNegativeIfOverflow(a.cpu.sp)
+	// a.maybeSetFlagZero(a.cpu.sp)
+	// a.maybeHandleOverflow(a.cpu.sp)
 	return nil
 }
 
@@ -632,14 +632,14 @@ func execROR(a *Appleone, o op) error {
 	}
 
 	var carry bool
-	if a.getCarry() == flagCarry {
+	if a.getFlag(flagCarry) == flagCarry {
 		carry = true
 	}
 
 	if operand&0x01 != 0 {
-		a.setCarry()
+		a.setFlag(flagCarry)
 	} else {
-		a.clearCarry()
+		a.clearFlag(flagCarry)
 	}
 
 	operand >>= 1
@@ -648,8 +648,8 @@ func execROR(a *Appleone, o op) error {
 		operand |= flagNegative
 	}
 
-	a.setZeroIfNeeded(operand)
-	a.setNegativeIfOverflow(operand)
+	a.maybeSetFlagZero(operand)
+	a.maybeHandleOverflow(operand)
 
 	if o.addrMode == accumulator {
 		a.cpu.a = operand
@@ -674,15 +674,15 @@ func execASL(a *Appleone, o op) error {
 	}
 
 	if operand>>7 == 1 {
-		a.setCarry()
+		a.setFlag(flagCarry)
 	} else {
-		a.clearCarry()
+		a.clearFlag(flagCarry)
 	}
 
 	operand <<= 1
 
-	a.setZeroIfNeeded(operand)
-	a.setNegativeIfOverflow(operand)
+	a.maybeSetFlagZero(operand)
+	a.maybeHandleOverflow(operand)
 
 	if o.addrMode == accumulator {
 		a.cpu.a = operand
@@ -706,8 +706,8 @@ func execAND(a *Appleone, o op) error {
 		return err
 	}
 	a.cpu.a &= operand
-	a.setZeroIfNeeded(a.cpu.a)
-	a.setNegativeIfOverflow(a.cpu.a)
+	a.maybeSetFlagZero(a.cpu.a)
+	a.maybeHandleOverflow(a.cpu.a)
 	return nil
 }
 
@@ -719,7 +719,7 @@ func execORA(a *Appleone, o op) error {
 		return err
 	}
 	a.cpu.a |= operand
-	a.setZeroIfNeeded(a.cpu.a)
-	a.setNegativeIfOverflow(a.cpu.a)
+	a.maybeSetFlagZero(a.cpu.a)
+	a.maybeHandleOverflow(a.cpu.a)
 	return nil
 }
diff --git a/internal/vm/vm.go b/internal/vm/vm.go
index 1dde2fb..5eaa672 100644
--- a/internal/vm/vm.go
+++ b/internal/vm/vm.go
@@ -19,6 +19,7 @@ func New() *Appleone {
 	}
 }
 
+// load puts the provided data into the apple1's memory block starting at the provided address
 func (a *Appleone) load(addr uint16, data []byte) {
 	a.mem.load(addr, data)
 	a.cpu.pc = addr
@@ -126,68 +127,35 @@ func (a *Appleone) nextDWord() uint16 {
 	return a.littleEndianToUint16(a.mem[a.cpu.pc-1], a.mem[a.cpu.pc-2])
 }
 
-func (a *Appleone) setZeroIfNeeded(word byte) {
-	a.clearZero()
+// maybeSetFlagZero takes a single word (byte), clears flagZero, and sets flagZero if word is 0
+func (a *Appleone) maybeSetFlagZero(word byte) {
+	a.clearFlag(flagZero)
 	if word == 0 {
-		a.setZero()
+		a.setFlag(flagZero)
 	}
 }
 
-func (a *Appleone) setZero() {
-	a.cpu.ps |= flagZero
+func (a *Appleone) getFlag(flag byte) byte {
+	return a.cpu.ps & flag
 }
 
-func (a *Appleone) clearZero() {
-	a.cpu.sp &^= flagZero
+func (a *Appleone) setFlag(flag byte) {
+	a.cpu.ps |= flag
 }
 
-func (a *Appleone) getNegative() byte {
-	return a.cpu.ps & flagNegative
+func (a *Appleone) clearFlag(flag byte) {
+	a.cpu.ps &^= flag
 }
 
-func (a *Appleone) setNegativeIfOverflow(word byte) {
-	a.clearNegative()
+func (a *Appleone) maybeHandleOverflow(word byte) {
+	a.clearFlag(flagNegative)
 	if word > 127 {
-		a.setNegative()
+		a.setFlag(flagNegative)
 	}
 }
 
-func (a *Appleone) setNegative() {
-	a.cpu.ps |= flagZero
-}
-
-func (a *Appleone) clearNegative() {
-	a.cpu.sp &^= flagZero
-}
-
-func (a *Appleone) getCarry() byte {
-	return a.cpu.sp & flagCarry
-}
-
-func (a *Appleone) setCarry() {
-	a.cpu.sp |= flagCarry
-}
-
-func (a *Appleone) clearCarry() {
-	a.cpu.sp &^= flagCarry
-}
-
-func (a *Appleone) getOverflow() byte {
-	return a.cpu.ps & flagOverflow
-}
-
-func (a *Appleone) setOverflow() {
-	a.cpu.sp |= flagOverflow
-}
-
-func (a *Appleone) clearOverflow() {
-	a.cpu.sp &^= flagOverflow
-}
-
-func (a *Appleone) getZero() byte {
-	return a.cpu.ps & flagZero
-}
-
+// Branch offsets are signed 8-bit values, -128 ... +127, negative offsets in two's
+// complement. Page transitions may occur and add an extra cycle to the exucution
 func (a *Appleone) branch(o op) error {
 	offset, err := a.getOperand(o)
 	if err != nil {
@@ -201,35 +169,21 @@ func (a *Appleone) branch(o op) error {
 	return nil
 }
 
+// compare clears zero, carry, and negative flags, compares the two bytes, and sets the
+// appropriate flags based on the comparison between the bytes.
 func (a *Appleone) compare(b1, b2 byte) {
-	a.clearZero()
-	a.clearCarry()
-	a.clearNegative()
+	a.clearFlag(flagZero)
+	a.clearFlag(flagCarry)
+	a.clearFlag(flagNegative)
 
 	if b1 == b2 {
-		a.setZero()
-		a.setCarry()
+		a.setFlag(flagZero)
+		a.setFlag(flagCarry)
 	}
 	if b1 > b2 {
-		a.setCarry()
+		a.setFlag(flagCarry)
 	}
 
-	sum := byte(uint16(b1) - uint16(b2))
-	a.setNegativeIfOverflow(sum)
-}
-
-func (a *Appleone) setDec() {
-	a.cpu.sp |= flagDecimalMode
-}
-
-func (a *Appleone) clearDec() {
-	a.cpu.sp &^= flagDecimalMode
-}
-
-func (a *Appleone) setInterrupt() {
-	a.cpu.sp |= flagDisableInterrupts
-}
-
-func (a *Appleone) clearInterrupt() {
-	a.cpu.sp &^= flagDisableInterrupts
+	b := byte(uint16(b1) - uint16(b2))
+	a.maybeHandleOverflow(b)
 }