#699 [GBZ80] codegen pushes hl for no reason

[Sebastian Riedel]

This code:

//$ sdcc -mgbz80 --fverbose-asm --no-peep -c -o main.rel main.c

#define UINT8   unsigned char

UINT8 level_x;
UINT8 level_y;

void change_level() {}

void teleport_to(const UINT8 lx, const UINT8 ly) {
    level_x = lx;
    level_y = ly;
}

void main(){
    teleport_to(1,2);
}

Generates this asm for teleport_to:

;main.c:10: void teleport_to(const UINT8 lx, const UINT8 ly) {
;   genLabel
;   genFunction
;   ---------------------------------
; Function teleport_to
; ---------------------------------
;   Register assignment is optimal.
; Stack space usage: 0 bytes.
_teleport_to::
;main.c:11: level_x = lx;
;   genAssign
;   AOP_STK for 
    push    hl
    ldhl    sp, #4
    ld  a, (hl)
;fetchLitPair
    ld  hl, #_level_x
    ld  (hl), a
    pop hl
;main.c:12: level_y = ly;
;   genAssign
;   AOP_STK for 
    push    hl
    ldhl    sp, #5
    ld  a, (hl)
;fetchLitPair
    ld  hl, #_level_y
    ld  (hl), a
    pop hl
;   genLabel
00101$:
;main.c:13: }
;   genEndFunction
    ret

Expected generated code:

_teleport_to::
    ldhl    sp, #2
    ld  a, (hl)
    ld  hl, #_level_x
    ld  (hl), a
    ldhl    sp, #3
    ld  a, (hl)
    ld  hl, #_level_y
    ld  (hl), a
00101$:
    ret

Best code:

_teleport_to::
    ldhl    sp, #2
    ld  a, (hl+)
    ld  (#_level_x), a
    ld  a, (hl)
    ld  (#_level_y), a
    ret

[Philipp Klaus Krause]

Ticket moved from /p/sdcc/bugs/3096/

Can't be converted:

• _category: GBZ80

[Philipp Klaus Krause]

Not a bug, since the generated code is correct, though inefficient.

[Sebastian Riedel]

I wasn't really sure where to put it.

[Philipp Klaus Krause]

The attached patch should help with this.
However, on my system gbz80 regression tests currently fail (both with and without the patch), so I should find out about that problem first.

[Sebastian Riedel]

Thanks, that worked nicely in my game

[Philipp Klaus Krause]

Fixed in [r11806].

[Tony Pavlov]

sorry, Philipp, seem that it has some side effects, i can go through walls in my game now. it is ok with 11800, and broken in 11806; and i also use --no-peep, so that is not 11803. i'll try to make a minimal example, but that is not an easy task.

[Tony Pavlov]

I located the problem.

    static UBYTE x, y, z;

     from_coords(src->coords, x, y, z);

where

#define from_coords(coord, x, y, z) (x = ((coord) >> 8u), y = (8u - ((coord) >> 4u) & 0x0f), z = ((coord) & 0x0f))

old asm code is:

    ld  a, (hl)
    inc hl
    ld  h, (hl)
    ld  l, a
    ld  e, h
    ld  d, #0x00
    ld  hl, #_scene_to_map_x_65536_99 ; this is x
    ld  (hl), e

result of ((coord) >> 8u) is allocated in DE; E is transferred into (HL), and HL is initialized by an address of static variable just before.

while the new code is:

    ld  a, (hl)
    inc hl
    ld  h, (hl)
    ld  l, a
    ld  e, #0x00
    ld  hl, #_scene_to_map_x_65536_99
    ld  (hl), h

result of ((coord) >> 8u) is allocated to H; but it tries to load address of a static var into HL, and then issues ld (hl), h -- the high byte of static variable address instead of the result of expression. also, what is E for here? it is not used below at all.

probably, this is how it should be:

    ld    e, h
    ld    hl, #_scene_to_map_x_65536_99
    ld    (hl), e