mirror of
https://github.com/mre/mos6502.git
synced 2025-01-15 01:32:49 +00:00
Fix Add with Carry (ADC) Implementation
This pull request addresses several issues with the Add with Carry (ADC) operation in our 6502 emulator, ensuring correct behavior in both decimal and non-decimal modes. It is similar to #106, which improved on our SBC operation. My hope is that these changes improve the accuracy of our 6502 emulator, particularly for software that relies on correct decimal mode arithmetic or precise overflow detection. - Implement correct decimal mode addition in ADC operation - Accurate overflow detection for non-decimal mode - Proper carry flag calculation for both modes - Preserve overflow flag in decimal mode (as per 6502 specification) The previous implementation didn't correctly handle Binary-Coded Decimal (BCD) arithmetic. The new implementation: 1. Performs addition on low and high nibbles separately 2. Adjusts results when they exceed 9 (valid BCD range is 0-9) 3. Propagates carries between nibbles 4. Sets the carry flag correctly based on the final BCD result - In non-decimal mode: Implemented using the formula `(!(A ^ M) & (A ^ R)) & 0x80 != 0`, where A is the accumulator, M is the memory operand, and R is the result. - In decimal mode: The overflow flag is preserved, matching the behavior of the actual 6502 processor. - In non-decimal mode: Set if the unsigned result is less than either the accumulator or the memory operand. - In decimal mode: Set based on whether the BCD addition resulted in a value greater than 99.
This commit is contained in:
Matthias
parent
a0919fe7b8
commit
77697dbb99
204
src/cpu.rs
204
src/cpu.rs
@ -865,40 +865,76 @@ impl<M: Bus, V: Variant> CPU<M, V> {
|
||||
);
|
||||
}
|
||||
|
||||
/// Executes the following calculation: A + M + C
|
||||
///
|
||||
/// This algorithm ensures correct behavior for the ADC operation in both
|
||||
/// decimal and non-decimal modes, handling all the intricacies of the 6502
|
||||
/// processor's implementation.
|
||||
fn add_with_carry(&mut self, value: u8) {
|
||||
const fn decimal_adjust(result: u8) -> u8 {
|
||||
let bcd1: u8 = if (result & 0x0f) > 0x09 { 0x06 } else { 0x00 };
|
||||
// Convert the carry flag to a 0 or 1 for addition
|
||||
let carry: u8 = u8::from(self.registers.status.contains(Status::PS_CARRY));
|
||||
let accumulator_before = self.registers.accumulator;
|
||||
|
||||
let bcd2: u8 = if (result.wrapping_add(bcd1) & 0xf0) > 0x90 {
|
||||
0x60
|
||||
// Perform binary addition first
|
||||
let temp_result = accumulator_before.wrapping_add(value).wrapping_add(carry);
|
||||
|
||||
let (final_result, did_carry) = if self.registers.status.contains(Status::PS_DECIMAL_MODE) {
|
||||
// Decimal mode: treat each nibble as a decimal digit (0-9)
|
||||
|
||||
// Add the low nibbles (including carry)
|
||||
let mut low_nibble = (accumulator_before & 0x0f)
|
||||
.wrapping_add(value & 0x0f)
|
||||
.wrapping_add(carry);
|
||||
|
||||
// Add the high nibbles
|
||||
let mut high_nibble = (accumulator_before >> 4).wrapping_add(value >> 4);
|
||||
let mut carry_to_high = false;
|
||||
|
||||
// If low nibble is > 9, we need to adjust it and carry to high nibble
|
||||
// This is because in BCD, each nibble should represent 0-9
|
||||
if low_nibble > 9 {
|
||||
low_nibble = low_nibble.wrapping_sub(10);
|
||||
carry_to_high = true;
|
||||
}
|
||||
|
||||
// Add the carry from low nibble to high nibble if necessary
|
||||
high_nibble = high_nibble.wrapping_add(u8::from(carry_to_high));
|
||||
|
||||
// Adjust high nibble if it's > 9 and set final carry flag
|
||||
let (adjusted_high, did_carry) = if high_nibble > 9 {
|
||||
(high_nibble.wrapping_sub(10), true)
|
||||
} else {
|
||||
0x00
|
||||
(high_nibble, false)
|
||||
};
|
||||
|
||||
result.wrapping_add(bcd1).wrapping_add(bcd2)
|
||||
}
|
||||
|
||||
let a_before: u8 = self.registers.accumulator;
|
||||
let c_before: u8 = u8::from(self.registers.status.contains(Status::PS_CARRY));
|
||||
let a_after: u8 = a_before.wrapping_add(c_before).wrapping_add(value);
|
||||
|
||||
debug_assert_eq!(a_after, a_before.wrapping_add(c_before).wrapping_add(value));
|
||||
|
||||
let result: u8 = if self.registers.status.contains(Status::PS_DECIMAL_MODE) {
|
||||
decimal_adjust(a_after)
|
||||
// Combine adjusted high and low nibbles
|
||||
let result = (adjusted_high << 4) | (low_nibble & 0x0f);
|
||||
(result, did_carry)
|
||||
} else {
|
||||
a_after
|
||||
// Non-decimal mode: use the binary addition result
|
||||
// Carry occurs if the result is less than either operand
|
||||
(
|
||||
temp_result,
|
||||
temp_result < accumulator_before || temp_result < value,
|
||||
)
|
||||
};
|
||||
|
||||
let did_carry = (result) < (a_before)
|
||||
|| (a_after == 0 && c_before == 0x01)
|
||||
|| (value == 0xff && c_before == 0x01);
|
||||
// Calculate overflow
|
||||
// Overflow occurs when both inputs have the same sign bit,
|
||||
// but the result has a different sign bit
|
||||
let calculated_overflow =
|
||||
(!(accumulator_before ^ value) & (accumulator_before ^ final_result)) & 0x80 != 0;
|
||||
|
||||
let did_overflow = (a_before > 127 && value > 127 && a_after < 128)
|
||||
|| (a_before < 128 && value < 128 && a_after > 127);
|
||||
// In decimal mode, the overflow flag is not affected by ADC
|
||||
let did_overflow = if self.registers.status.contains(Status::PS_DECIMAL_MODE) {
|
||||
// Preserve the current overflow flag in decimal mode
|
||||
self.registers.status.contains(Status::PS_OVERFLOW)
|
||||
} else {
|
||||
calculated_overflow
|
||||
};
|
||||
|
||||
// Update carry and overflow flags
|
||||
let mask = Status::PS_CARRY | Status::PS_OVERFLOW;
|
||||
|
||||
self.registers.status.set_with_mask(
|
||||
mask,
|
||||
Status::new(StatusArgs {
|
||||
@ -908,9 +944,8 @@ impl<M: Bus, V: Variant> CPU<M, V> {
|
||||
}),
|
||||
);
|
||||
|
||||
self.load_accumulator(result);
|
||||
|
||||
log::debug!("accumulator: {}", self.registers.accumulator);
|
||||
// Set the accumulator and update zero and negative flags
|
||||
self.load_accumulator(final_result);
|
||||
}
|
||||
|
||||
fn add_with_no_decimal(&mut self, value: u8) {
|
||||
@ -995,8 +1030,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
|
||||
/// Executes the following calculation: A - M - (1 - C)
|
||||
///
|
||||
/// This algorithm ensures correct behavior for the SBC operation in both
|
||||
/// decimal and non-decimal modes, handling all the intricacies of the 6502
|
||||
/// processor's implementation.
|
||||
/// decimal and non-decimal modes
|
||||
fn subtract_with_carry(&mut self, value: u8) {
|
||||
// First, convert the carry flag to a 0 or 1
|
||||
let carry: u8 = u8::from(self.registers.status.contains(Status::PS_CARRY));
|
||||
@ -1306,28 +1340,36 @@ mod tests {
|
||||
#[cfg_attr(feature = "decimal_mode", test)]
|
||||
fn decimal_add_test() {
|
||||
let mut cpu = CPU::new(Ram::new(), Nmos6502);
|
||||
cpu.registers.status.or(Status::PS_DECIMAL_MODE);
|
||||
cpu.registers.status.insert(Status::PS_DECIMAL_MODE);
|
||||
|
||||
cpu.add_with_carry(0x09);
|
||||
assert_eq!(cpu.registers.accumulator, 0x09);
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
// Overflow flag should remain unchanged in decimal mode
|
||||
let overflow_before = cpu.registers.status.contains(Status::PS_OVERFLOW);
|
||||
|
||||
cpu.add_with_carry(0x43);
|
||||
assert_eq!(cpu.registers.accumulator, 0x52);
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
assert_eq!(
|
||||
cpu.registers.status.contains(Status::PS_OVERFLOW),
|
||||
overflow_before
|
||||
);
|
||||
|
||||
cpu.add_with_carry(0x48);
|
||||
assert_eq!(cpu.registers.accumulator, 0x00);
|
||||
assert!(cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
// Overflow flag should still remain unchanged
|
||||
assert_eq!(
|
||||
cpu.registers.status.contains(Status::PS_OVERFLOW),
|
||||
overflow_before
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg_attr(feature = "decimal_mode", test)]
|
||||
@ -1359,20 +1401,22 @@ mod tests {
|
||||
fn add_with_carry_test() {
|
||||
let mut cpu = CPU::new(Ram::new(), Nmos6502);
|
||||
|
||||
cpu.add_with_carry(1);
|
||||
assert_eq!(cpu.registers.accumulator, 1);
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
// Non-decimal mode tests
|
||||
cpu.registers.status.remove(Status::PS_DECIMAL_MODE);
|
||||
|
||||
cpu.add_with_carry(0xff);
|
||||
// Test case 1: 0 + 0 with carry clear
|
||||
cpu.registers.accumulator = 0;
|
||||
cpu.registers.status.remove(Status::PS_CARRY);
|
||||
cpu.add_with_carry(0);
|
||||
assert_eq!(cpu.registers.accumulator, 0);
|
||||
assert!(cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
|
||||
// Test case 2: 0 + 1 with carry set
|
||||
cpu.registers.accumulator = 0;
|
||||
cpu.registers.status.insert(Status::PS_CARRY);
|
||||
cpu.add_with_carry(1);
|
||||
assert_eq!(cpu.registers.accumulator, 2);
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
@ -1380,61 +1424,45 @@ mod tests {
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
|
||||
let mut cpu = CPU::new(Ram::new(), Nmos6502);
|
||||
|
||||
assert_eq!(cpu.registers.accumulator, 0);
|
||||
cpu.add_with_carry(127);
|
||||
assert_eq!(cpu.registers.accumulator, 127);
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
|
||||
// Allow casting from i8 to u8; -127i8 wraps to 129u8, as intended for
|
||||
// two's complement arithmetic.
|
||||
cpu.add_with_carry(-127i8 as u8);
|
||||
assert_eq!(cpu.registers.accumulator, 0);
|
||||
assert!(cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
|
||||
// Test case 3: 0x7F + 0x01 (overflow case)
|
||||
cpu.registers.accumulator = 0x7F;
|
||||
cpu.registers.status.remove(Status::PS_CARRY);
|
||||
cpu.add_with_carry(0x80);
|
||||
assert_eq!(cpu.registers.accumulator, 0x80);
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
|
||||
cpu.add_with_carry(127);
|
||||
assert_eq!(cpu.registers.accumulator, 0xff);
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
|
||||
let mut cpu = CPU::new(Ram::new(), Nmos6502);
|
||||
|
||||
cpu.add_with_carry(127);
|
||||
assert_eq!(cpu.registers.accumulator, 127);
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
|
||||
cpu.add_with_carry(1);
|
||||
cpu.add_with_carry(0x01);
|
||||
assert_eq!(cpu.registers.accumulator, 0x80);
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
assert!(cpu.registers.status.contains(Status::PS_OVERFLOW));
|
||||
|
||||
let mut cpu = CPU::new(Ram::new(), Nmos6502);
|
||||
cpu.registers.status.or(Status::PS_CARRY);
|
||||
cpu.add_with_carry(0xff);
|
||||
assert_eq!(cpu.registers.accumulator, 0);
|
||||
// Decimal mode tests
|
||||
cpu.registers.status.insert(Status::PS_DECIMAL_MODE);
|
||||
|
||||
// Test case 4: 09 + 01 with carry clear (decimal)
|
||||
cpu.registers.accumulator = 0x09;
|
||||
cpu.registers.status.remove(Status::PS_CARRY);
|
||||
cpu.add_with_carry(0x01);
|
||||
assert_eq!(cpu.registers.accumulator, 0x10);
|
||||
assert!(!cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
|
||||
// Test case 5: 50 + 50 with carry clear (decimal)
|
||||
cpu.registers.accumulator = 0x50;
|
||||
cpu.registers.status.remove(Status::PS_CARRY);
|
||||
cpu.add_with_carry(0x50);
|
||||
assert_eq!(cpu.registers.accumulator, 0x00);
|
||||
assert!(cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
|
||||
// Test case 6: 99 + 01 with carry set (decimal)
|
||||
cpu.registers.accumulator = 0x99;
|
||||
cpu.registers.status.insert(Status::PS_CARRY);
|
||||
cpu.add_with_carry(0x01);
|
||||
assert_eq!(cpu.registers.accumulator, 0x01);
|
||||
assert!(cpu.registers.status.contains(Status::PS_CARRY));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_ZERO));
|
||||
assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@ -1491,7 +1519,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn subtract_with_carry_comprehensive_test() {
|
||||
fn subtract_with_carry_test() {
|
||||
let mut cpu = CPU::new(Ram::new(), Nmos6502);
|
||||
|
||||
// Non-decimal mode tests
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user