search

ggambetta / libz80

An emulator of the Z80 processor (C library)
145 stars 32 forks source link

Incorrect handling of 16bit ADC #2

Open EtchedPixels opened 6 years ago

EtchedPixels commented 6 years ago

The ADC maths can give a result of 0x10000, in which case the carry is correctly set but Z is not.

This causes a failure running ROMWBW.

The Z test needs to mask off bit 16 when adding a 16bit word.

lvd2 commented 4 years ago

16-bit addition seems to be broken right from the first lines: if(withCarry && GETFLAG(F_C)) a2++; considering that a2 is ushort, it is incorrect (for example when before ADC HL,DE DE is 0xFFFF and carry is set). Workaround might be extending calculations into 32bit values, which also would make it easier to detect a signed overflow.

jjmz commented 4 years ago

I am also testing with ROMWBW - Z flag is not correctly updated in "doAddWord". If not 'withCarry' or 'isSub', it is not even considered.

I have moved VALFLAG(F_Z,... outside of the "if(withCarry || isSub)" block (after). And changed it to : VALFLAG(F_Z, (sum & 0xFFFF) == 0);

djrose80 commented 1 year ago

16-bit addition seems to be broken right from the first lines: if(withCarry && GETFLAG(F_C)) a2++; considering that a2 is ushort, it is incorrect (for example when before ADC HL,DE DE is 0xFFFF and carry is set). Workaround might be extending calculations into 32bit values, which also would make it easier to detect a signed overflow.

I have just tried adjusting the "doAddWord" by removing the increment of the 16 bit a2 and instead adding code to either decrement or increment 'sum' within the 'isSub' if/else block. This ensures that the maths is carried out on a 32 bit value.

Initial tests show that C Flag is conditioned correctly when the value being added/subtracted is 0xFFFF and there is an original carry.