sdcc generates invalid opcode ld h,iyl

[jorgegv]

Apparently, when compiling some C code with SDCC, an invalid opcode is generated:

make -f Makefile-48 engine/src/bullet.o
Compiling engine/src/bullet.c ...
engine/src/bullet.c:1033: error: syntax error
  ^---- ld h, iyl
engine/src/bullet.c:1039: error: syntax error
  ^---- ld h, iyl
make: *** [Makefile.common:107: engine/src/bullet.o] Error 1

When examining the ASM generated by the C source, the invalid opcode is indeed there:

(snippet from engine/src/bullet.c.asm)
...
        inc     hl
        ld      (hl), d
;engine/src/bullet.c:84: bs->xthresh = BULLET_SPRITE_XTHRESH;
        ld      l, b
        ld      h, iyl
        ld      de,0x0011
        add     hl, de
        ld      (hl),0x04
;engine/src/bullet.c:85: bs->ythresh = BULLET_SPRITE_YTHRESH;
        ld      l, b
        ld      h, iyl
        ld      de,0x0012
        add     hl, de

I know that "ld h, iyl" is an undocumented instruction, but according to this reference, it is not a valid one due to ambiguity. And Z80ASM does not chew it (correctly, it seems).

Compilation options: zcc +zx -compiler=sdcc

Compilation with -SO3 --opt-code-size and without makes no difference.

I'm not sure how to reproduce the bug, since the assignment instruction seems pretty simple, so it looks that this is a combinatioj of factors involved that lead to this error.

Happy to do any tests to hunt this down.

Update: if I compile with the --reserve-regs-iy option, the problem disappears, as expected.

[feilipu]

Not sure it is a bug, as your reference describes it.

edit Not true. I misread instruction issued.

If, however, an instruction contains both H and (HL), or both L and (HL), then only the (HL) part is affected by the addition of DD or FD.

But, afaik and as described in other issues (eg #1504) iy should be avoided in +zx target because it is used and not preserved by an interrupt routine.

[jorgegv]

mmm I'm not sure the lines you mention apply to the instruction which is generated.

In any case, I believe it is a bug because:

• If it's not a valid instruction for Z80ASM, then SDCC should not generate it, but it is doing so.
• If it is indeed a valid instruction, then Z80ASM should assemble it, but it doesn't.

So either it is a Z80ASM bug, or a ZSDCC one.

Don't you agree? ;-)

[suborb]

It looks like if assembled it would act as ld iyh,iyl which is not what is intended from the rest of the snippet.

So I’m backing sdcc rather than z80asm issue.

[feilipu]

If you can reproduce it in a short section of code, targeting +z80, then it would be best to raise directly on the SDCC issue register.

They are closing a stable release window in next days.

[suborb]

I've got a "minimal" example. It's seems related to the size of the structures.

Compile: zcc +zx -clib=sdcc_ix repro.c -a -SO3 --opt-code-size --c-code-in-asm --max-allocs-per-node200000

Removing --max-allocs-per-node also sidesteps the issue.

#include <inttypes.h>



struct sp1_ss
 {
   uint8_t              row;            // +0  current y tile-coordinate
   uint8_t              col;            // +1  current x tile-coordinate
   uint8_t              width;          // +2  width of sprite in tiles
   uint8_t              height;         // +3  height of sprite in tiles

   uint8_t              vrot;           // +4  bit 7 = 1 for 2-byte graphical definition else 1-byte, bits 2:0 = current vertical rotation (0..7)
   uint8_t              hrot;           // +5  current horizontal rotation (0..7)

   uint8_t             *frame;          // +6  current sprite frame address added to graphic pointers

   uint8_t              res0;           // +8  "LD A,n" opcode
   uint8_t              e_hrot;         // +9  effective horizontal rotation = MSB of rotation table to use
   uint8_t              res1;           // +10 "LD BC,nn" opcode
   uint16_t             e_offset;       // +11 effective offset to add to graphic pointers, equals result of vertical rotation + frame addr
   uint8_t              res2;           // +13 "EX DE,HL" opcode
   uint8_t              res3;           // +14 "JP (HL)" opcode


   uint8_t              xthresh;        // +17 hrot must be at least this number of pixels for last column of sprite to be drawn (1 default)
   uint8_t              ythresh;        // +18 vrot must be at least this number of pixels for last row of sprite to be drawn (1 default)
};

struct sp1_ss *sprite_allocate( uint8_t rows, uint8_t cols);

struct position_data_s {
    uint8_t x,y;        // position top, left
    uint8_t xmax,ymax;  // position bottom,right
};


struct bullet_movement_data_s {
    int8_t dx,dy;       // position increments, absolute
    uint8_t delay;      // frames between position updates (inv of speed)
};

struct bullet_state_data_s {
    struct sp1_ss *sprite;
    struct position_data_s position;
    int8_t dx, dy;              // current x and y increments
    uint8_t delay_counter;      // current delay counter
    uint8_t flags;
};

extern struct bullet_state_data_s bullet_state_data[];

#define BULLET_MAX_BULLETS 100
#define BULLET_SPRITE_XTHRESH   4
#define BULLET_SPRITE_YTHRESH   4

struct bullet_info_s;


void bullet_init_sprites(void) {
    struct bullet_info_s *bi;
    struct sp1_ss *bs;
    uint8_t i;

    // SP1 sprite data
    i = BULLET_MAX_BULLETS;
    while ( i-- ) {
        bullet_state_data[i].sprite = bs = sprite_allocate( 1, 1 );
        bs->xthresh = BULLET_SPRITE_XTHRESH;
        bs->ythresh = BULLET_SPRITE_YTHRESH;
    }
}

Need to verify that this also happens in vanilla upstream.

Edit: After fixing the sdcc trunk so that sdcc will build, the issue can be reproduced upstream. Raised bug: https://sourceforge.net/p/sdcc/bugs/3327/

[jorgegv]

Well, thanks so much to all involved, It never ceases to amaze me how I can report a bug at 2pm and have it solved and patched on the same day at 23.30. This is what makes me love free software.

Cheers
J.

[feilipu]

The solution for #1977 is now in the 4.2.0 PR patch, but will be removed once it flows into the 4.2.0 RC.

[zx70]

@feilipu, while restoring the MZ800 MONITOR and BASIC sources I noticed that the (mz-5z009) uses different opcodes than z80asm:

LD IY,DE was originally translated to $d5, $fd, $e1

..while z88dk-z80asm uses:
$fd $62 ; LD HY,D
$fd $6b ; LD LY,E

Obviously the IX prefix should work similarly.

I don't know if the shorter sequence is valid on all the Z80 CPUs or only on the LH-0080, though.
I'm still progressing with the fixes on those sources, I could discover more.

EDIT:
This is the only other difference I found:
defb $fd, $e5, $d1 ;LD DE,IY (Z80ASM uses 4 bytes)
; ** the z80asm output would currently be:
;defb $fd $54 ; LD D,HY
;defb $fd $5d ; LD E,LY

[ped7g]

the shorter sequence is push de : pop iy and it is valid on all Z80 CPUs, but it's tiny bit slower (11+14 = 25T, the two 8bit ld are 16T, one byte longer)

[artrag]

Moreover it relies on the stack, and the side effect of changing a word in ram could be unwanted by the coder
There are cases (extreme cases, I admit) where SP is used for fast access to ram arrays (some time critical ASM loops).
A push/pop implementation for LD IY,DE would lead to unexpected issues hard to track.