Search code examples
assemblyemulationz80gameboygbdk

Bug with power-of-two multiplication in GBDK compiler


I'm currently developing a gameboy emulator and to test the correctness of my emulator I'm using GBDK to compile c programs for my emulator.

I noticed that the compiler (as expected) optimizes multiplications with constants that are a power of two by doing rotates. However it appears that it does not generate the correct amount of rotates for a given power.

For example the following very simple program:

#include <gb/gb.h>

unsigned char one() { return 1; }

void main()
{
    unsigned char r;

    // Force compiler to generate muliplication by deferring the '1'
    r = one() * 32;

    // Store result somewhere
    *((unsigned char*)(0xFFFE)) = r;
}

Generates the following assembly:

___main_start:
_main:
    push    bc
;   reproduce.c 14
; genCall
    call    _one
    ld  c,e
; genLeftShift
    ld  a,c
    rr  a
    rr  a
    rr  a
    and a,#0xE0
    ld  c,a
;   reproduce.c 16
; genAssign
    ld  de,#0xFFFE
; genAssign (pointer)
    ld  a,c
    ld  (de),a
; genLabel
00101$:
; genEndFunction
    pop bc
    ret
___main_end:
    .area _CODE

Which to me appears to be incorrect as the RR instruction actually rotates through the carry flag, effectively making it a 9-bit rotate. This means that there should be an additional rotate to produce the correct result instead of the current (0x40) wrong result.

Visualization:

Start: A = 00000001 Carry = 0
RR A:  A = 00000000 Carry = 1
RR A:  A = 10000000 Carry = 0
RR A:  A = 01000000 Carry = 0 <- WRONG! 

Can anyone verify that this is indeed a bug in the SDCC compiler that comes with GBDK? I'm also interested in the use of the and instruction following the rotates.

Using the latest (3-2.93) version of GBDK for windows from sourceforge.


Solution

  • It's not a bug with your emulator -- other emulators I've tested also give 64 for the following code:

    #include <stdio.h>
    
    unsigned char one() { return 1; }
    
    void main()
    {
        unsigned int r;
    
        // Force compiler to generate multiplication by deferring the '1'
        r = one() * 32;
    
        printf("1 * 32 = %u", r);
    }
    

    Here it is on BGB (version 1.5.1):

    BGB version 1.5.1: 1 * 32 = 64

    And this is on VBA-M (version SVN1149):

    VBA-M version SVN1149: 1 * 32 = 64


    As for why the and a,#0xE0 is included? That's actually simple. It just ensures that overflow doesn't mess up the value.

    First off, suppose that multiplication actually worked right, and 1 * 32 still equals 32. (I'll just add an extra RR A).

    For 1 * 32, this looks like this:

    Start       ; A = 00000001 Carry = 0
    RR  A       ; A = 00000000 Carry = 1
    RR  A       ; A = 10000000 Carry = 0
    RR  A       ; A = 01000000 Carry = 0
    RR  A       ; A = 00100000 Carry = 0
    AND A,0xE0  ; A = 00100000 Carry = 0
    

    Here, the AND has no effect. But, suppose we multiplied by something that would cause overflow, such as 17 * 32:

    Start       ; A = 00010001 Carry = 0
    RR  A       ; A = 00001000 Carry = 1
    RR  A       ; A = 10000100 Carry = 0
    RR  A       ; A = 01000010 Carry = 0
    RR  A       ; A = 00100001 Carry = 1
    AND A,0xE0  ; A = 00100000 Carry = 0
    

    Without the and, we'd have gotten 17 * 32 = 33, rather than the correct answer for 1 byte (32). While neither of those answers are the true answer (544), 32 is the correct value for the first byte of it.