#519 Z80 & Z180 library contains suboptimal code

[Sergey Belyashov]

trunk/sdcc/device/lib/z180/abs.s:
trunk/sdcc/device/lib/z80/abs.s:
pop hl and push hl are not needed, because hl later just nulled
it is better (faster) to load null to HL using ld hl,0 rather than xor a; ld h,a; ld l,a

trunk/sdcc/device/lib/z180/mulchar.s:
trunk/sdcc/device/lib/z180/mul.s:
Z180 has unsigned mul instructions like MLT HL: HL = H * L; but they are not used.

/trunk/sdcc/device/lib/z180/crt0.s:
/trunk/sdcc/device/lib/z80/crt0.s:
all interrupt handlers should have ei before reti instruction, because reti does not restore interrupt state (bug of processor itself)
there is no NMI handler at address 0x66, which should be just retn

I suggest to do interrupt handlers vector like:
.org 0x8
jp RST8_Handler
.org 0x10
jp RST10_Handler
...
.org 0x38
jp RST38_Handler
.org 0x66
jp NMI_Handler

And implement them using weak functions, which can be reimplemented later by user without changing crt0.s:
.weak NMI_Handler
.type NMI_Handler, @function
NMI_Handler:
retn
.size NMI_Handler, .-NMI_Handler

.weak Dummy_RST_Handler
.type Dummy_RST_Handler, @function
ret
.size Dummy_RST_Handler, .-Dummy_RST_Handler

.weak   RST8_Handler
.set    RST8_Handler,Dummy_RST_Handler
.weak   RST10_Handler
.set    RST10_Handler,Dummy_RST_Handler

...
.weak RST38_Handler
.type RST38_Handler, @function
ei ;RST38 is in most cases interrupt handler in IM 0/1
reti
.size RST38_Handler, .-RST38_Handler

[Philipp Klaus Krause]

Ticket moved from /p/sdcc/bugs/2565/

Can't be converted:

• _category: Z80

[Philipp Klaus Krause]

Ticket moved from /p/schlange/feature-requests/4/

[Philipp Klaus Krause]

Moved to feature requests, since the code might be suboptimal, but its not broken.

Philipp

[Sergey Belyashov]

My misstake about abs.s. POP HL is required in any case, because DE value is not on top of stack...

[Philipp Klaus Krause]

abs(): As you noted, the pop hl is needed to get de. And the xor a,a ld h,a ld l, a is used since it also resets the C flag, while with ld hl, #0 another instruction to reset C would be needed.

Philipp

[Philipp Klaus Krause]

In [r10069], the z180 library got a 16x16->16 multiplication routine that uses mlt.

So out of the 3 items, now only the interrupt stuff remains to be implemented (which is probably the least important, as most users use a custom crt0 anyway).

Philipp

[Philipp Klaus Krause]

For the interrupt handlers, we wouldn't use weak functions in asm. We'd instead handle interrupt handers like in the stm8 or mcs51 backend - require the user to declare them in the same file as main() using SDCC-specific keywords.

Philipp

[Sergey Belyashov]

I do not understand why. In most microcontroller toolchains it is standard way to work with interrupt handlers. If user do not defines them, then used empty handlers, else they will be replaced by user defined.

[Maarten Brock]

The assembler does not have a .weak directive and the linker doesn't understand the concept either.

I've never seen the .weak concept used by anything but GCC / binutils ld. And just like all other microcontroller toolchains SDCC has its own way of defining interrupt handlers.

We allow the user to specify the name of the ISR instead of use a fixed one, because it might handle any kind of peripheral. Further when an interrupt is not used, there is no point in consuming code space for the vector and a dummy implementation.

On a Z80 an RST is not reserved for interrupt handlers. And an interrupt can be serviced by other means as well (IM0/1/2).

[Philipp Klaus Krause]

The ei was added in [r10459].

Philipp