Emulating GameBoy's 0xCB 0x19 Instruction

I am writing a gameboy emulator and I am currently implementing 0xCB 0x19 instruction.

I am using this test rom: https://github.com/retrio/gb-test-roms/blob/master/cpu_instrs/individual/06-ld%20r%2Cr.gb and comparing my logs against https://github.com/wheremyfoodat/Gameboy-logs/blob/master/Blargg6LYStubbed.zip and I am stuck at as according to those logs the instruction 0xCB 0x19 will change the value in register C from 0x47 to 0x23 while my implementation changes it to 0xA3.

Here is my implementations:

    pub fn rr_reg8(&mut self, register_name: RegisterName, register_byte: RegisterByteName) -> u64 {
        self.clear_flag(Flag::HalfCarry);
        self.clear_flag(Flag::Subtraction);

        let shifted_bit = utils::data::check_bit(self.registers[register_name][register_byte], 0);
        if shifted_bit {
            self.set_flag(Flag::Carry);
        } else {
            self.clear_flag(Flag::Carry);
        }

        self.registers[register_name][register_byte] >>= 1;

        if self.registers[register_name][register_byte] == 0 {
            self.set_flag(Flag::Zero);
        } else {
            self.clear_flag(Flag::Zero);
        }

        if self.check_flag(Flag::Carry) {
            utils::data::set_bit(&mut self.registers[register_name][register_byte], 7);
        }

        8
    }

Is this not how rotate through carry should work? Or is there an issue with the logs?

Solution

I couldn't easily find official documentation for the LR35902, but it's mostly Z80 compatible, so I'll assume that they implement this instruction the same.

rr c is effectively a rotation of the 9-bit value formed by the C register together with the carry flag. So the value rotated in to bit 7 of register C should be the old value of the carry flag. Here you are setting it to the new value of the carry flag, which was the old value of register C bit 0. This would be correct behavior for rrc c, opcode 0xcb 0x09, but not for rr c.

(The naming convention of rr/rrc is the opposite of the x86 convention, where rcr al would be the 9-bit rotate through carry, and ror al is the 8-bit rotate of AL alone. See Why are the Intel 8080's rotate instructions called opposite to intuition? on Retrocomputing.SE which puts forward the hypothesis that the c in 8080/Z80 rrc stands for "circular", not "carry".)

If C = 0x47 and the carry flag is clear, then rr c should result in C = 0x23 and the carry flag set. rrc c would result in C = 0xA3 and carry flag set (in that case the original value of the carry flag would be unused).

So you'll want to store the original value of Flag::Carry in a temporary variable, and assign it to bit 7 of register C at the end.

Another bug is that you should set the zero flag based on the final result in register C. So for instance, if C = 0x01 and the carry flag is set, the result should be C = 0x80, carry flag set, and zero flag clear. Your code would improperly leave the zero flag set in this case, since you tested the value of C before setting the high bit.

Also, you seem to have omitted to set the sign flag based on the final value of the high bit of C. (This means the new value of the sign flag will always equal the old value of the carry flag.) You have to set the parity flag properly as well. Unless that's done somewhere else in your code, or the LR35902 left out these flags?