Menu

#4047 adc hl,bc with b containing stale value

closed-fixed
z80 (20)
Z80
5
4 days ago
4 days ago
Under4Mhz
No

The following code produces the wrong result.

/// GPL 2.0 or later
#include <stdio.h>
#include <stdint.h>
#include <stdbool.h>
#include <string.h>

#define COLS 8
#define ROWS 3

typedef struct { int8_t row; int8_t col; uint8_t orientation : 1; } Shape;
typedef struct { uint8_t sprite; } TableItem;

static TableItem table[ROWS][COLS];

TableItem *TableGet( uint8_t col, uint8_t row ) { return &table[row][col]; }

///< Check next position valid. All three conditions are needed to trigger.
int BlockMoveCheck( const Shape *shape ) {

    bool vertical = shape->orientation == 0;
    uint8_t height = vertical;
    uint8_t width = !vertical;

    if ( shape->col < 0 || shape->col + width >= COLS || shape->row + height >= ROWS ) return false;
    else if ( TableGet( shape->col, shape->row )->sprite ) return false;
    else if ( TableGet( shape->col + width, shape->row + height )->sprite ) return false;

    return true;
}

void main(void) {

    memset( &table, 0, sizeof(table) );

    // orientation = 1 -> height = 0, so the last row is legal. Expect 1 1 1.
    for ( int8_t row = 0; row < ROWS; row++ ) {

        Shape shape = { .col = 2, .row = row, .orientation = 1 };

        printf( "row%d=%d\n", row, BlockMoveCheck( &shape ) );
    }
}

#ifdef __SDCC
__sfr __at 0xff sif;
int putchar( int c ) { sif = 'p'; sif = c; return c; }
#endif
//
// sdcc -mz80 --allow-undocumented-instructions --allow-unsafe-read --opt-code-speed --max-allocs-per-node 200000 --std-c23 ./registers_swapped.c -o registers_swapped.ihx && ucsim_z80 -t z80 -I if=outputs[0xff] -e run -e quit registers_swapped.ihx
// gcc ./registers_swapped.c && ./a.out
$ sdcc -mz80 --allow-undocumented-instructions --allow-unsafe-read --opt-code-speed --max-allocs-per-node 200000 --std-c23 ./registers_swapped.c -o registers_swapped.ihx && ucsim_z80 -t z80 -I if=outputs[0xff] -e run -e quit registers_swapped.ihx
uCsim 0.9.9, Copyright (C) 1997 Daniel Drotos.
uCsim comes with ABSOLUTELY NO WARRANTY; for details type `show w'.
This is free software, and you are welcome to redistribute it
under certain conditions; type `show c' for details.
0> 3181 words read from registers_swapped.ihx
Simulation started, PC=0x000000
row0=1
row1=1
row2=0
$ gcc ./registers_swapped.c && ./a.out
row0=1
row1=1
row2=1

looking at the assembler, b is not reset between 90 and 124, so the adc is giving an incorrect result.

90:   xor a, #0x01
91:   ld b, a
...
124:  adc hl, bc
sdcc -v
SDCC : z80/sm83/ez80/z80n/mos6502/mos65c02 4.6.2 #16701 (Linux)
3 Attachments

Discussion

  • Philipp Klaus Krause

    I can reproduce the bug (though I see a slightly different register assignment here):

    ;   genPlus
        add a, h
        ld  h, a
        adc hl, bc
    

    The problem is that SDCC uses adc hl, rr to calculate the result in l, despite h already being in use as the lower byte of the result. Looks like an addition codegen bug.

     
  • Philipp Klaus Krause

    • labels: --> z80
    • assigned_to: Philipp Klaus Krause
     
  • Philipp Klaus Krause

    • status: open --> closed-fixed
     
  • Philipp Klaus Krause

    Fixed in [r16726].

     

    Related

    Commit: [r16726]


Log in to post a comment.

Auth0 Logo