From eda8774e08bc431d4683c97429b62b079223752f Mon Sep 17 00:00:00 2001 From: Sidney Cadot Date: Mon, 16 Dec 2024 16:36:23 +0100 Subject: [PATCH 1/8] Fixed ADC/SBC for the 65C02. The current (before-this-patch) version of sim65.c does not correctly implement the ADC and SBC instructions in 65C02 mode. This PR fixes that. The 6502 and 65C02 behave identically in binary mode; in decimal behavior however they diverge, both in the handling of inputs that are not BCD values, and in the handling of processor flags. This fix restructures the original "ADC" and "SBC" macros in versions that are specific for the 6502 and the 65C02, and updates the opcode tables to ensure that they point to the correct implementations. Considering the ADC instruction for a moment, the original "ADC" macro was changed to two macros ADC_6502 and ADC_65C02. These check the D (decimal mode) bit, and defer their implementation to any of three macros ADC_BINARY_MODE, ADC_DECIMAL_MODE_6502, and ADC_DECIMAL_MODE_65C02. This is a bit verbose but it makes it very clear what's going on. (For the SBC changes, the analogous changes were made.) The correctness of the changes made is ensured as follows: First, an in-depth study was made how ADC and SBC work, both in the original 6502 and the later 65C02 processor. The actual behavior of both processors was captured on hardware (an Atari 800 XL with a 6502 and a Neo6502 equipped with a WDC 65C02 processor), and was analyzed. The results were cross-referenced with internet sources, leading to a C implementation that reproduces the exact result of the hardware processors. See: https://github.com/sidneycadot/6502-test/blob/main/functional_test/adc_sbc/c_and_python_implementations/6502_adc_sbc.c Next, these C implementations of ADC and SBC were fitted into sim65's macro- based implementation scheme, replacing the existing 6502-only implementation. Finally, the new sim65 implementation was tested against the 65x02 testsuite, showing that (1) the 6502 implementation was still correct; and (2) that the 65C02 implementation is now also correct. As an added bonus, this new implementation of ADC/SBC no longer relies on a dirty implementation detail in sim65: the previous implementation relied on the fact that currently, the A register in the simulator is implemented as an "unsigned", with more bits than the actual A register (8 bits). In the future we want to change the register width to 8 bits, and this updated ADC/SBC is a necessary precursor to that change. --- src/sim65/6502.c | 465 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 347 insertions(+), 118 deletions(-) diff --git a/src/sim65/6502.c b/src/sim65/6502.c index cb4e579eb..a424736f3 100644 --- a/src/sim65/6502.c +++ b/src/sim65/6502.c @@ -37,10 +37,11 @@ /* Known bugs and limitations of the 65C02 simulation: * support currently only on the level of 65SC02: BBRx, BBSx, RMBx, SMBx, WAI, and STP are unsupported - * BCD flag handling equals 6502 (unchecked if bug is simulated or wrong for - 6502) */ +#include +#include + #include "memory.h" #include "error.h" #include "6502.h" @@ -555,15 +556,15 @@ static unsigned HaveIRQRequest; /* #imm */ #define ALU_OP_IMM(op) \ unsigned char immediate; \ - MEM_AD_OP_IMM(immediate); \ - Cycles = 2; \ + MEM_AD_OP_IMM(immediate); \ + Cycles = 2; \ op (immediate) /* zp / zp,x / zp,y / abs / abs,x / abs,y / (zp,x) / (zp),y / (zp) */ #define ALU_OP(mode, op) \ unsigned address, operand; \ - Cycles = ALU_CY_##mode; \ - MEM_AD_OP (mode, address, operand); \ + Cycles = ALU_CY_##mode; \ + MEM_AD_OP (mode, address, operand); \ op (operand) /* Store opcode helpers */ @@ -662,40 +663,88 @@ static unsigned HaveIRQRequest; TEST_SF (Regs.AC) -/* ADC */ -#define ADC(v) \ +/* ADC, binary mode (6502 and 65C02) */ +/* TODO: once the Regs fields are properly sized, get rid of the + * "& 0xff" in the Regs.AC asignment. + */ +#define ADC_BINARY_MODE(v) \ do { \ - unsigned old = Regs.AC; \ - unsigned rhs = (v & 0xFF); \ - if (GET_DF ()) { \ - unsigned lo; \ - int res; \ - lo = (old & 0x0F) + (rhs & 0x0F) + GET_CF (); \ - if (lo >= 0x0A) { \ - lo = ((lo + 0x06) & 0x0F) + 0x10; \ - } \ - Regs.AC = (old & 0xF0) + (rhs & 0xF0) + lo; \ - res = (signed char)(old & 0xF0) + \ - (signed char)(rhs & 0xF0) + \ - (signed char)lo; \ - TEST_ZF (old + rhs + GET_CF ()); \ - TEST_SF (Regs.AC); \ - if (Regs.AC >= 0xA0) { \ - Regs.AC += 0x60; \ - } \ - TEST_CF (Regs.AC); \ - SET_OF ((res < -128) || (res > 127)); \ - if (CPU == CPU_65C02) { \ - ++Cycles; \ - } \ + const uint8_t op = v; \ + const uint8_t OldAC = Regs.AC; \ + bool carry = GET_CF(); \ + Regs.AC = (OldAC + op + carry) & 0xff; \ + const bool NV = Regs.AC >= 0x80; \ + carry = OldAC + op + carry >= 0x100; \ + SET_SF(NV); \ + SET_OF(((OldAC >= 0x80) ^ NV) & ((op >= 0x80) ^ NV)); \ + SET_ZF(Regs.AC == 0); \ + SET_CF(carry); \ + } while (0) + +/* ADC, decimal mode (6502 behavior) */ +#define ADC_DECIMAL_MODE_6502(v) \ + do { \ + const uint8_t op = v; \ + const uint8_t OldAC = Regs.AC; \ + bool carry = GET_CF(); \ + const uint8_t binary_result = OldAC + op + carry; \ + uint8_t low_nibble = (OldAC & 15) + (op & 15) + carry; \ + if ((carry = low_nibble > 9)) \ + low_nibble = (low_nibble - 10) & 15; \ + uint8_t high_nibble = (OldAC >> 4) + (op >> 4) + carry; \ + const bool NV = (high_nibble & 8) != 0; \ + if ((carry = high_nibble > 9)) \ + high_nibble = (high_nibble - 10) & 15; \ + Regs.AC = (high_nibble << 4) | low_nibble; \ + SET_SF(NV); \ + SET_OF(((OldAC >= 0x80) ^ NV) & ((op >= 0x80) ^ NV)); \ + SET_ZF(binary_result == 0); \ + SET_CF(carry); \ + } while (0) + +/* ADC, decimal mode (65C02 behavior) */ +#define ADC_DECIMAL_MODE_65C02(v) \ + do { \ + const uint8_t op = v; \ + const uint8_t OldAC = Regs.AC; \ + const bool OldCF = GET_CF(); \ + bool carry = OldCF; \ + uint8_t low_nibble = (OldAC & 15) + (op & 15) + carry; \ + if ((carry = low_nibble > 9)) \ + low_nibble = (low_nibble - 10) & 15; \ + uint8_t high_nibble = (OldAC >> 4) + (op >> 4) + carry; \ + const bool PrematureSF = (high_nibble & 8) != 0; \ + if ((carry = high_nibble > 9)) \ + high_nibble = (high_nibble - 10) & 15; \ + Regs.AC = (high_nibble << 4) | low_nibble; \ + const bool NewZF = Regs.AC == 0; \ + const bool NewSF = Regs.AC >= 0x80; \ + const bool NewOF = ((OldAC >= 0x80) ^ PrematureSF) & \ + ((op >= 0x80) ^ PrematureSF); \ + SET_SF(NewSF); \ + SET_OF(NewOF); \ + SET_ZF(NewZF); \ + SET_CF(carry); \ + ++Cycles; \ + } while (0) + +/* ADC, 6502 version */ +#define ADC_6502(v) \ + do { \ + if (GET_DF()) { \ + ADC_DECIMAL_MODE_6502(v); \ } else { \ - Regs.AC += rhs + GET_CF (); \ - TEST_ZF (Regs.AC); \ - TEST_SF (Regs.AC); \ - TEST_CF (Regs.AC); \ - SET_OF (!((old ^ rhs) & 0x80) && \ - ((old ^ Regs.AC) & 0x80)); \ - Regs.AC &= 0xFF; \ + ADC_BINARY_MODE(v); \ + } \ + } while (0) + +/* ADC, 65C02 version */ +#define ADC_65C02(v) \ + do { \ + if (GET_DF()) { \ + ADC_DECIMAL_MODE_65C02(v); \ + } else { \ + ADC_BINARY_MODE(v); \ } \ } while (0) @@ -737,23 +786,29 @@ static unsigned HaveIRQRequest; /* ROL */ #define ROL(Val) \ - Val <<= 1; \ - if (GET_CF ()) { \ - Val |= 0x01; \ - } \ - TEST_ZF (Val); \ - TEST_SF (Val); \ - TEST_CF (Val) + do { \ + unsigned ShiftOut = (Val & 0x80); \ + Val <<= 1; \ + if (GET_CF ()) { \ + Val |= 0x01; \ + } \ + TEST_ZF (Val); \ + TEST_SF (Val); \ + SET_CF (ShiftOut); \ + } while (0) /* ROR */ #define ROR(Val) \ - if (GET_CF ()) { \ - Val |= 0x100; \ - } \ - SET_CF (Val & 0x01); \ - Val >>= 1; \ - TEST_ZF (Val); \ - TEST_SF (Val) + do { \ + unsigned ShiftOut = (Val & 0x01); \ + Val >>= 1; \ + if (GET_CF ()) { \ + Val |= 0x80; \ + } \ + TEST_ZF (Val); \ + TEST_SF (Val); \ + SET_CF (ShiftOut); \ + } while(0) /* ASL */ #define ASL(Val) \ @@ -816,7 +871,7 @@ static unsigned HaveIRQRequest; } \ SET_CF (Val & 0x01); \ Val >>= 1; \ - ADC (Val) + ADC_6502 (Val) /* BIT */ #define BIT(Val) \ @@ -873,7 +928,7 @@ static unsigned HaveIRQRequest; /* ISC */ #define ISC(Val) \ Val = (Val + 1) & 0xFF; \ - SBC(Val) + SBC_6502(Val) /* ASR */ #define ASR(Val) \ @@ -971,33 +1026,88 @@ static unsigned HaveIRQRequest; TEST_SF (Val); \ TEST_ZF (Val) - -/* SBC */ -#define SBC(v) \ +/* SBC, binary mode (6502 and 65C02) */ +/* TODO: once the Regs fields are properly sized, get rid of the + * "& 0xff" in the Regs.AC asignment. + */ +#define SBC_BINARY_MODE(v) \ do { \ - unsigned r_a = Regs.AC; \ - unsigned src = (v) & 0xFF; \ - unsigned ccc = (Regs.SR & CF) ^ CF; \ - unsigned tmp = r_a - src - ccc; \ - \ - SET_CF(tmp < 0x100); \ - TEST_SF(tmp); \ - TEST_ZF(tmp); \ - SET_OF((r_a ^ tmp) & (r_a ^ src) & 0x80); \ - \ - if (GET_DF ()) { \ - unsigned low = (r_a & 0x0f) - (src & 0x0f) - ccc; \ - tmp = (r_a & 0xf0) - (src & 0xf0); \ - if (low & 0x10) { \ - low -= 6; \ - tmp -= 0x10; \ - } \ - tmp = (low & 0xf) | tmp; \ - if (tmp & 0x100) { \ - tmp -= 0x60; \ - } \ + const uint8_t op = v; \ + const uint8_t OldAC = Regs.AC; \ + const bool borrow = !GET_CF(); \ + Regs.AC = (OldAC - op - borrow) & 0xff; \ + const bool NV = Regs.AC >= 0x80; \ + SET_SF(NV); \ + SET_OF(((OldAC >= 0x80) ^ NV) & ((op < 0x80) ^ NV)); \ + SET_ZF(Regs.AC == 0); \ + SET_CF(OldAC >= op + borrow); \ + } while (0) + +/* SBC, decimal mode (6502 behavior) */ +#define SBC_DECIMAL_MODE_6502(v) \ + do { \ + const uint8_t op = v; \ + const uint8_t OldAC = Regs.AC; \ + bool borrow = !GET_CF(); \ + const uint8_t binary_result = OldAC - op - borrow; \ + const bool NV = binary_result >= 0x80; \ + uint8_t low_nibble = (OldAC & 15) - (op & 15) - borrow; \ + if ((borrow = low_nibble >= 0x80)) \ + low_nibble = (low_nibble + 10) & 15; \ + uint8_t high_nibble = (OldAC >> 4) - (op >> 4) - borrow;\ + if ((borrow = high_nibble >= 0x80)) \ + high_nibble = (high_nibble + 10) & 15; \ + Regs.AC = (high_nibble << 4) | low_nibble; \ + SET_SF(NV); \ + SET_OF(((OldAC >= 0x80) ^ NV) & ((op < 0x80) ^ NV)); \ + SET_ZF(binary_result == 0); \ + SET_CF(!borrow); \ + } while (0) + +/* SBC, decimal mode (65C02 behavior) */ +#define SBC_DECIMAL_MODE_65C02(v) \ + do { \ + const uint8_t op = v; \ + const uint8_t OldAC = Regs.AC; \ + bool borrow = !GET_CF(); \ + uint8_t low_nibble = (OldAC & 15) - (op & 15) - borrow; \ + if ((borrow = low_nibble >= 0x80)) \ + low_nibble += 10; \ + const bool low_nibble_still_negative = \ + (low_nibble >= 0x80); \ + low_nibble &= 15; \ + uint8_t high_nibble = (OldAC >> 4) - (op >> 4) - borrow;\ + const bool PN = (high_nibble & 8) != 0; \ + if ((borrow = high_nibble >= 0x80)) \ + high_nibble += 10; \ + high_nibble -= low_nibble_still_negative; \ + high_nibble &= 15; \ + Regs.AC = (high_nibble << 4) | low_nibble; \ + SET_SF(Regs.AC >= 0x80); \ + SET_OF(((OldAC >= 0x80) ^ PN) & ((op < 0x80) ^ PN)); \ + SET_ZF(Regs.AC == 0x00); \ + SET_CF(!borrow); \ + ++Cycles; \ + } while (0) + +/* SBC, 6502 version */ +#define SBC_6502(v) \ + do { \ + if (GET_DF()) { \ + SBC_DECIMAL_MODE_6502(v); \ + } else { \ + SBC_BINARY_MODE(v); \ + } \ + } while (0) + +/* SBC, 65C02 version */ +#define SBC_65C02(v) \ + do { \ + if (GET_DF()) { \ + SBC_DECIMAL_MODE_65C02(v); \ + } else { \ + SBC_BINARY_MODE(v); \ } \ - Regs.AC = tmp & 0xFF; \ } while (0) @@ -1881,11 +1991,17 @@ static void OPC_6502_60 (void) static void OPC_6502_61 (void) /* Opcode $61: ADC (zp,x) */ { - ALU_OP (ZPXIND, ADC); + ALU_OP (ZPXIND, ADC_6502); } +static void OPC_65C02_61 (void) +/* Opcode $61: ADC (zp,x) */ +{ + ALU_OP (ZPXIND, ADC_65C02); +} + static void OPC_6502_63 (void) /* Opcode $63: RRA (zp,x) */ { @@ -1905,7 +2021,15 @@ static void OPC_65SC02_64 (void) static void OPC_6502_65 (void) /* Opcode $65: ADC zp */ { - ALU_OP (ZP, ADC); + ALU_OP (ZP, ADC_6502); +} + + + +static void OPC_65C02_65 (void) +/* Opcode $65: ADC zp */ +{ + ALU_OP (ZP, ADC_65C02); } @@ -1941,11 +2065,17 @@ static void OPC_6502_68 (void) static void OPC_6502_69 (void) /* Opcode $69: ADC #imm */ { - ALU_OP_IMM (ADC); + ALU_OP_IMM (ADC_6502); } +static void OPC_65C02_69 (void) +/* Opcode $69: ADC #imm */ +{ + ALU_OP_IMM (ADC_65C02); +} + static void OPC_6502_6A (void) /* Opcode $6A: ROR a */ { @@ -2004,7 +2134,15 @@ static void OPC_65C02_6C (void) static void OPC_6502_6D (void) /* Opcode $6D: ADC abs */ { - ALU_OP (ABS, ADC); + ALU_OP (ABS, ADC_6502); +} + + + +static void OPC_65C02_6D (void) +/* Opcode $6D: ADC abs */ +{ + ALU_OP (ABS, ADC_65C02); } @@ -2036,15 +2174,23 @@ static void OPC_6502_70 (void) static void OPC_6502_71 (void) /* Opcode $71: ADC (zp),y */ { - ALU_OP (ZPINDY, ADC); + ALU_OP (ZPINDY, ADC_6502); } -static void OPC_65SC02_72 (void) +static void OPC_65C02_71 (void) +/* Opcode $71: ADC (zp),y */ +{ + ALU_OP (ZPINDY, ADC_65C02); +} + + + +static void OPC_65C02_72 (void) /* Opcode $72: ADC (zp) */ { - ALU_OP (ZPIND, ADC); + ALU_OP (ZPIND, ADC_65C02); } @@ -2068,11 +2214,17 @@ static void OPC_65SC02_74 (void) static void OPC_6502_75 (void) /* Opcode $75: ADC zp,x */ { - ALU_OP (ZPX, ADC); + ALU_OP (ZPX, ADC_6502); } +static void OPC_65C02_75 (void) +/* Opcode $75: ADC zp,x */ +{ + ALU_OP (ZPX, ADC_65C02); +} + static void OPC_6502_76 (void) /* Opcode $76: ROR zp,x */ { @@ -2102,7 +2254,15 @@ static void OPC_6502_78 (void) static void OPC_6502_79 (void) /* Opcode $79: ADC abs,y */ { - ALU_OP (ABSY, ADC); + ALU_OP (ABSY, ADC_6502); +} + + + +static void OPC_65C02_79 (void) +/* Opcode $79: ADC abs,y */ +{ + ALU_OP (ABSY, ADC_65C02); } @@ -2144,7 +2304,15 @@ static void OPC_65SC02_7C (void) static void OPC_6502_7D (void) /* Opcode $7D: ADC abs,x */ { - ALU_OP (ABSX, ADC); + ALU_OP (ABSX, ADC_6502); +} + + + +static void OPC_65C02_7D (void) +/* Opcode $7D: ADC abs,x */ +{ + ALU_OP (ABSX, ADC_65C02); } @@ -2986,11 +3154,17 @@ static void OPC_6502_E0 (void) static void OPC_6502_E1 (void) /* Opcode $E1: SBC (zp,x) */ { - ALU_OP (ZPXIND, SBC); + ALU_OP (ZPXIND, SBC_6502); } +static void OPC_65C02_E1 (void) +/* Opcode $E1: SBC (zp,x) */ +{ + ALU_OP (ZPXIND, SBC_65C02); +} + static void OPC_6502_E3 (void) /* Opcode $E3: ISC (zp,x) */ { @@ -3010,7 +3184,15 @@ static void OPC_6502_E4 (void) static void OPC_6502_E5 (void) /* Opcode $E5: SBC zp */ { - ALU_OP (ZP, SBC); + ALU_OP (ZP, SBC_6502); +} + + + +static void OPC_65C02_E5 (void) +/* Opcode $E5: SBC zp */ +{ + ALU_OP (ZP, SBC_65C02); } @@ -3041,17 +3223,23 @@ static void OPC_6502_E8 (void) -/* Aliases of opcode $EA */ +/* Aliases of opcode $E9 */ #define OPC_6502_EB OPC_6502_E9 static void OPC_6502_E9 (void) /* Opcode $E9: SBC #imm */ { - ALU_OP_IMM (SBC); + ALU_OP_IMM (SBC_6502); } +static void OPC_65C02_E9 (void) +/* Opcode $E9: SBC #imm */ +{ + ALU_OP_IMM (SBC_65C02); +} + /* Aliases of opcode $EA */ #define OPC_6502_1A OPC_6502_EA #define OPC_6502_3A OPC_6502_EA @@ -3117,7 +3305,15 @@ static void OPC_6502_EC (void) static void OPC_6502_ED (void) /* Opcode $ED: SBC abs */ { - ALU_OP (ABS, SBC); + ALU_OP (ABS, SBC_6502); +} + + + +static void OPC_65C02_ED (void) +/* Opcode $ED: SBC abs */ +{ + ALU_OP (ABS, SBC_65C02); } @@ -3148,15 +3344,24 @@ static void OPC_6502_F0 (void) static void OPC_6502_F1 (void) /* Opcode $F1: SBC (zp),y */ { - ALU_OP (ZPINDY, SBC); + ALU_OP (ZPINDY, SBC_6502); } -static void OPC_65SC02_F2 (void) + +static void OPC_65C02_F1 (void) +/* Opcode $F1: SBC (zp),y */ +{ + ALU_OP (ZPINDY, SBC_65C02); +} + + + +static void OPC_65C02_F2 (void) /* Opcode $F2: SBC (zp) */ { - ALU_OP (ZPIND, SBC); + ALU_OP (ZPIND, SBC_65C02); } @@ -3172,7 +3377,15 @@ static void OPC_6502_F3 (void) static void OPC_6502_F5 (void) /* Opcode $F5: SBC zp,x */ { - ALU_OP (ZPX, SBC); + ALU_OP (ZPX, SBC_6502); +} + + + +static void OPC_65C02_F5 (void) +/* Opcode $F5: SBC zp,x */ +{ + ALU_OP (ZPX, SBC_65C02); } @@ -3206,7 +3419,15 @@ static void OPC_6502_F8 (void) static void OPC_6502_F9 (void) /* Opcode $F9: SBC abs,y */ { - ALU_OP (ABSY, SBC); + ALU_OP (ABSY, SBC_6502); +} + + + +static void OPC_65C02_F9 (void) +/* Opcode $F9: SBC abs,y */ +{ + ALU_OP (ABSY, SBC_65C02); } @@ -3234,7 +3455,15 @@ static void OPC_6502_FB (void) static void OPC_6502_FD (void) /* Opcode $FD: SBC abs,x */ { - ALU_OP (ABSX, SBC); + ALU_OP (ABSX, SBC_6502); +} + + + +static void OPC_65C02_FD (void) +/* Opcode $FD: SBC abs,x */ +{ + ALU_OP (ABSX, SBC_65C02); } @@ -3884,35 +4113,35 @@ static const OPFunc OP65C02Table[256] = { OPC_65C02_5E, OPC_Illegal, // $5F: BBR5 currently unsupported OPC_6502_60, - OPC_6502_61, + OPC_65C02_61, OPC_65C02_NOP22, // $62 OPC_65C02_NOP11, // $63 OPC_65SC02_64, - OPC_6502_65, + OPC_65C02_65, OPC_6502_66, OPC_Illegal, // $67: RMB6 currently unsupported OPC_6502_68, - OPC_6502_69, + OPC_65C02_69, OPC_6502_6A, OPC_65C02_NOP11, // $6B OPC_65C02_6C, - OPC_6502_6D, + OPC_65C02_6D, OPC_6502_6E, OPC_Illegal, // $6F: BBR6 currently unsupported OPC_6502_70, - OPC_6502_71, - OPC_65SC02_72, + OPC_65C02_71, + OPC_65C02_72, OPC_65C02_NOP11, // $73 OPC_65SC02_74, - OPC_6502_75, + OPC_65C02_75, OPC_6502_76, OPC_Illegal, // $77: RMB7 currently unsupported OPC_6502_78, - OPC_6502_79, + OPC_65C02_79, OPC_65SC02_7A, OPC_65C02_NOP11, // $7B OPC_65SC02_7C, - OPC_6502_7D, + OPC_65C02_7D, OPC_65C02_7E, OPC_Illegal, // $7F: BBR7 currently unsupported OPC_65SC02_80, @@ -4012,35 +4241,35 @@ static const OPFunc OP65C02Table[256] = { OPC_6502_DE, OPC_Illegal, // $DF: BBS5 currently unsupported OPC_6502_E0, - OPC_6502_E1, + OPC_65C02_E1, OPC_65C02_NOP22, // $E2 OPC_65C02_NOP11, // $E3 OPC_6502_E4, - OPC_6502_E5, + OPC_65C02_E5, OPC_6502_E6, OPC_Illegal, // $E7: SMB6 currently unsupported OPC_6502_E8, - OPC_6502_E9, + OPC_65C02_E9, OPC_6502_EA, OPC_65C02_NOP11, // $EB OPC_6502_EC, - OPC_6502_ED, + OPC_65C02_ED, OPC_6502_EE, OPC_Illegal, // $EF: BBS6 currently unsupported OPC_6502_F0, - OPC_6502_F1, - OPC_65SC02_F2, + OPC_65C02_F1, + OPC_65C02_F2, OPC_65C02_NOP11, // $F3 OPC_65C02_NOP24, // $F4 - OPC_6502_F5, + OPC_65C02_F5, OPC_6502_F6, OPC_Illegal, // $F7: SMB7 currently unsupported OPC_6502_F8, - OPC_6502_F9, + OPC_65C02_F9, OPC_65SC02_FA, OPC_65C02_NOP11, // $FB OPC_65C02_NOP34, // $FC - OPC_6502_FD, + OPC_65C02_FD, OPC_6502_FE, OPC_Illegal, // $FF: BBS7 currently unsupported }; From fb6745573eb0a063f61417e87807becee03348db Mon Sep 17 00:00:00 2001 From: Sidney Cadot Date: Mon, 16 Dec 2024 16:55:26 +0100 Subject: [PATCH 2/8] Fixed whitespace. --- src/sim65/6502.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sim65/6502.c b/src/sim65/6502.c index a424736f3..c114c0c89 100644 --- a/src/sim65/6502.c +++ b/src/sim65/6502.c @@ -556,15 +556,15 @@ static unsigned HaveIRQRequest; /* #imm */ #define ALU_OP_IMM(op) \ unsigned char immediate; \ - MEM_AD_OP_IMM(immediate); \ - Cycles = 2; \ + MEM_AD_OP_IMM(immediate); \ + Cycles = 2; \ op (immediate) /* zp / zp,x / zp,y / abs / abs,x / abs,y / (zp,x) / (zp),y / (zp) */ #define ALU_OP(mode, op) \ unsigned address, operand; \ - Cycles = ALU_CY_##mode; \ - MEM_AD_OP (mode, address, operand); \ + Cycles = ALU_CY_##mode; \ + MEM_AD_OP (mode, address, operand); \ op (operand) /* Store opcode helpers */ From 86ccf25e81032bcc9a221c775a7dd26b29f132a6 Mon Sep 17 00:00:00 2001 From: Sidney Cadot Date: Mon, 16 Dec 2024 17:12:07 +0100 Subject: [PATCH 3/8] CPU registers can be accessed from outside 6502.c. The linkage of the 'Regs' variable in 6502.c was changed from static to extern. This makes the Regs type visible (and even alterable) from the outside. This change helps tools to inspect the CPU state. In particular, it was implemented to facilitate a tool that verifies opcode functionality using the '65x02' testsuite. But the change is also potentially useful for e.g. an online debugger that wants to inspect the CPU state while the 6502 is neing simulated. --- src/sim65/6502.c | 2 +- src/sim65/6502.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/sim65/6502.c b/src/sim65/6502.c index 48a1d560d..9d857a8ed 100644 --- a/src/sim65/6502.c +++ b/src/sim65/6502.c @@ -391,7 +391,7 @@ CPUType CPU; typedef void (*OPFunc) (void); /* The CPU registers */ -static CPURegs Regs; +CPURegs Regs; /* Cycles for the current insn */ static unsigned Cycles; diff --git a/src/sim65/6502.h b/src/sim65/6502.h index cab734c6a..b6b621823 100644 --- a/src/sim65/6502.h +++ b/src/sim65/6502.h @@ -65,6 +65,9 @@ struct CPURegs { unsigned PC; /* Program counter */ }; +/* Current CPU registers */ +extern CPURegs Regs; + /* Status register bits */ #define CF 0x01 /* Carry flag */ #define ZF 0x02 /* Zero flag */ From ceac9f87ba639c83139422d5cd93623f774826fb Mon Sep 17 00:00:00 2001 From: Sidney Cadot Date: Tue, 17 Dec 2024 21:34:05 +0100 Subject: [PATCH 4/8] Temporary fix for fgets() not using target-specific newline. This patch provides a temporary fix for the issue where the fgets() function did not use the target-specific newline character to decide if it has reached the end of the line. It defaulted to the value $0a, which is the newline character on only some targets. The Atari, for example, has newline character $9b instead. This patch is ugly, because the ca65 assembler that is used for fgets doesn't currently accept C-type character escape sequences as values. Ideally we'd be able to write: cmp #'\n' And this would end up being translated to a compare-immediate to the target-specific newline character. Since that is impossible, this patch substitutes the equivalent, but ugly, code: .byte $c9, "\n" This works because $c9 is the opcode for cmp #imm, and the "\n" string /is/ translated to the platform-specific newline character, at least when the 'string_escapes' feature is enabled. --- libsrc/common/fgets.s | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/libsrc/common/fgets.s b/libsrc/common/fgets.s index 172ca10dd..671136be2 100644 --- a/libsrc/common/fgets.s +++ b/libsrc/common/fgets.s @@ -8,6 +8,8 @@ .import _fgetc, popptr1, pushptr1, popax, pushax, return0, ___errno .importzp ptr1, ptr4 + .feature string_escapes + .include "errno.inc" .include "stdio.inc" .include "_file.inc" @@ -88,7 +90,22 @@ read_loop: bne :+ inc ptr4+1 -: cmp #$0A ; Stop at \n + ; The next code line: + ; + ; .byte $c9, "\n" + ; + ; corresponds to a CMP #imm with the target-specific newline value as its operand. + ; This works because (with the 'string_escapes' feature enabled), the "\n" string + ; assembles to the target-specific value for the newline character. + ; + ; It would be better if we could just write: + ; + ; cmp #'\n' + ; + ; Unfortunately, ca65 doesn't currently handle escape characters in character + ; constants. In the longer term, fixing that would be the preferred solution. + +: .byte $c9, "\n" ; cmp #'\n' beq done bne read_loop From 8ee93f7e5f726c0a39aa9a4ee396955718c0b6df Mon Sep 17 00:00:00 2001 From: Sidney Cadot Date: Wed, 18 Dec 2024 09:04:20 +0100 Subject: [PATCH 5/8] Fixed indentation inside comment. --- libsrc/common/fgets.s | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsrc/common/fgets.s b/libsrc/common/fgets.s index 671136be2..d5ea900d0 100644 --- a/libsrc/common/fgets.s +++ b/libsrc/common/fgets.s @@ -100,7 +100,7 @@ read_loop: ; ; It would be better if we could just write: ; - ; cmp #'\n' + ; cmp #'\n' ; ; Unfortunately, ca65 doesn't currently handle escape characters in character ; constants. In the longer term, fixing that would be the preferred solution. From bad2f54f75e6b4688b3ffd20ee4c77115d1feac3 Mon Sep 17 00:00:00 2001 From: Sidney Cadot Date: Thu, 19 Dec 2024 22:35:15 +0100 Subject: [PATCH 6/8] Fix memory access order of the JSR instruction. The obvious way to implement JSR for the 6502 is to (a) read the target address, and then (b) push the return address minus one. Or do (b) first, then (a). However, there is a non-obvious case where this conflicts with the actual order of operations that the 6502 does, which is: (a) Load the LSB of the target address. (b) Push the MSB of the return address, minus one. (c) Push the LSB of the return address, minus one. (d) Load the MSB of the target address. This can make a difference in a pretty esoteric case, if the JSR target is located, wholly or in part, inside the stack page (!). This won't happen in normal code but it can happen in specifically constructed examples. To deal with this, we load the LSB and MSB of the target address separately, with the pushing of the return address sandwiched in between, to mimic the order of the bus operations on a real 6502. --- src/sim65/6502.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/sim65/6502.c b/src/sim65/6502.c index cb3270185..bae23d9c0 100644 --- a/src/sim65/6502.c +++ b/src/sim65/6502.c @@ -1456,13 +1456,37 @@ static void OPC_6502_1F (void) static void OPC_6502_20 (void) /* Opcode $20: JSR */ { - unsigned Addr; + /* The obvious way to implement JSR for the 6502 is to (a) read the target address, + * and then (b) push the return address minus one. Or do (b) first, then (a). + * + * However, there is a non-obvious case where this conflicts with the actual order + * of operations that the 6502 does, which is: + * + * (a) Load the LSB of the target address. + * (b) Push the MSB of the return address, minus one. + * (c) Push the LSB of the return address, minus one. + * (d) Load the MSB of the target address. + * + * This can make a difference in a pretty esoteric case, if the JSR target is located, + * wholly or in part, inside the stack page (!). This won't happen in normal code + * but it can happen in specifically constructed examples. + * + * To deal with this, we load the LSB and MSB of the target address separately, + * with the pushing of the return address sandwiched in between, to mimic + * the order of the bus operations on a real 6502. + */ + + unsigned AddrLo, AddrHi; + Cycles = 6; - Addr = MemReadWord (Regs.PC+1); - Regs.PC += 2; + Regs.PC += 1; + AddrLo = MemReadByte(Regs.PC); + Regs.PC += 1; PUSH (PCH); PUSH (PCL); - Regs.PC = Addr; + AddrHi = MemReadByte(Regs.PC); + + Regs.PC = AddrLo + (AddrHi << 8); ParaVirtHooks (&Regs); } From b14f883e7339a0edbd734939cf910b2a5838f8b2 Mon Sep 17 00:00:00 2001 From: Sidney Cadot Date: Thu, 19 Dec 2024 22:58:42 +0100 Subject: [PATCH 7/8] sim65: changes constant of the unstable "ANE" instruction to comply with 65x02 test suite. ANE (0x8b) is an unstable illegal opcode that depends on a "constant" value that isn't really constant. It varies between machines, with temperature, and so on. Original sim65 behavior was to use the constant value 0xEF. To get the behavior in line with the 65x02 testsuite, we now use the value 0xEE instead, which is also a reasonable choice that can be observed in practice. --- src/sim65/6502.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/sim65/6502.c b/src/sim65/6502.c index cb3270185..166d54e2e 100644 --- a/src/sim65/6502.c +++ b/src/sim65/6502.c @@ -969,8 +969,14 @@ static unsigned HaveIRQRequest; } while (0); /* ANE */ +/* An "unstable" illegal opcode that depends on a "constant" value that isn't + * really constant. It varies between machines, with temperature, and so on. + * Original sim65 behavior was to use the constant 0xEF here. To get behavior + * in line with the 65x02 testsuite, we now use the value 0xEE instead, + * which is also a reasonable choice that can be observed in practice. + */ #define ANE(Val) \ - Val = (Regs.AC | 0xEF) & Regs.XR & Val; \ + Val = (Regs.AC | 0xEE) & Regs.XR & Val; \ Regs.AC = Val; \ TEST_SF (Val); \ TEST_ZF (Val) From 8cb941985dec9d7cf275fa29a0ceb0b45da75b3c Mon Sep 17 00:00:00 2001 From: Sidney Cadot Date: Thu, 19 Dec 2024 23:13:20 +0100 Subject: [PATCH 8/8] sim65: tighten 6502 register types After a lot of preparatory work, we are now in position to finally tighten the types of the 6502 registers defined in the CPURegs struct of sim65. All registers were previously defined as bare 'unsigned', leading to subtle bugs where the bits beyond the 8 or 16 "true" bits in the register could become non-zero. Tightening the types of the registers to uint8_t and uint16_t as appropriate gets rid of these subtle bugs once and for all, assisted by the semantics of C when assigning an unsigned value to an unsigned type with less bits: the high-order bits are simply discarded, which is precisely what we'd want to happen. This change cleans up a lot of spurious failures of sim65 against the 65x02 test-set. For the 6502 and 65C02, we're now *functionally* compliant. For timing (i.e., clock cycle counts for each instruction), some work remains. --- src/sim65/6502.c | 10 ++-------- src/sim65/6502.h | 14 ++++++++------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/sim65/6502.c b/src/sim65/6502.c index cb3270185..8b16da2b8 100644 --- a/src/sim65/6502.c +++ b/src/sim65/6502.c @@ -664,15 +664,12 @@ static unsigned HaveIRQRequest; /* ADC, binary mode (6502 and 65C02) */ -/* TODO: once the Regs fields are properly sized, get rid of the - * "& 0xff" in the Regs.AC asignment. - */ #define ADC_BINARY_MODE(v) \ do { \ const uint8_t op = v; \ const uint8_t OldAC = Regs.AC; \ bool carry = GET_CF(); \ - Regs.AC = (OldAC + op + carry) & 0xff; \ + Regs.AC = (OldAC + op + carry); \ const bool NV = Regs.AC >= 0x80; \ carry = OldAC + op + carry >= 0x100; \ SET_SF(NV); \ @@ -1027,15 +1024,12 @@ static unsigned HaveIRQRequest; TEST_ZF (Val) /* SBC, binary mode (6502 and 65C02) */ -/* TODO: once the Regs fields are properly sized, get rid of the - * "& 0xff" in the Regs.AC asignment. - */ #define SBC_BINARY_MODE(v) \ do { \ const uint8_t op = v; \ const uint8_t OldAC = Regs.AC; \ const bool borrow = !GET_CF(); \ - Regs.AC = (OldAC - op - borrow) & 0xff; \ + Regs.AC = (OldAC - op - borrow); \ const bool NV = Regs.AC >= 0x80; \ SET_SF(NV); \ SET_OF(((OldAC >= 0x80) ^ NV) & ((op < 0x80) ^ NV)); \ diff --git a/src/sim65/6502.h b/src/sim65/6502.h index b6b621823..0f4d066d0 100644 --- a/src/sim65/6502.h +++ b/src/sim65/6502.h @@ -37,6 +37,8 @@ #define _6502_H +#include + /*****************************************************************************/ /* Data */ @@ -57,12 +59,12 @@ extern CPUType CPU; /* 6502 CPU registers */ typedef struct CPURegs CPURegs; struct CPURegs { - unsigned AC; /* Accumulator */ - unsigned XR; /* X register */ - unsigned YR; /* Y register */ - unsigned SR; /* Status register */ - unsigned SP; /* Stackpointer */ - unsigned PC; /* Program counter */ + uint8_t AC; /* Accumulator */ + uint8_t XR; /* X register */ + uint8_t YR; /* Y register */ + uint8_t SR; /* Status register */ + uint8_t SP; /* Stackpointer */ + uint16_t PC; /* Program counter */ }; /* Current CPU registers */