#3173 [z80] optimizer removes stack frame allocation

[sverx]

As the title said, the optimizer removes three instructions that are needed to preserve local variables on the stack - so we get:

;main.c:5: unsigned char some_func (unsigned char a, unsigned char b, unsigned char c, unsigned char d) {
;   genLabel
;   genFunction
;   ---------------------------------
; Function some_func
; ---------------------------------
;   Register assignment might be sub-optimal.
; Stack space usage: 7 bytes.
_some_func::
    push    ix
    ld  ix,#0
    add ix,sp
; common peephole 1 removed dead load from #-7 into hl.
; common peephole 2a removed add hl,sp due to unused result.
; common peephole 1 removed dead load from hl into sp.
;main.c:9: if (d>=b) {

with sdcc -c -mz80 --fverbose-asm main.c
instead of:

;main.c:5: unsigned char some_func (unsigned char a, unsigned char b, unsigned char c, unsigned char d) {
;   ---------------------------------
; Function some_func
; ---------------------------------
_some_func::
    push    ix
    ld  ix,#0
    add ix,sp
    ld  hl, #-7
    add hl, sp
    ld  sp, hl
;main.c:9: if (d>=b) {

with sdcc -c -mz80 --no-peep main.c

example code attached.

(this is probably causing bug #3171 and #3166 too... ?)

[sverx]

sorry I forgot to mention I'm running latest snapshot:

$ ./sdcc -v
SDCC : mcs51/z80/z180/r2k/r2ka/r3ka/gbz80/tlcs90/ez80_z80/z80n/ds390/pic16/pic14/TININative/ds400/hc08/s08/stm8/pdk13/pdk14/pdk15 4.0.7 #12016 (Linux)
published under GNU General Public License (GPL)

[sverx]

also I think loading anything into SP can't ever be considered "dead load"

; common peephole 1 removed dead load from hl into sp.

[Philipp Klaus Krause]

I'm not sure about that.
On one hand, the value of sp might be read anytime in an interrupt handler.
On the other hand, code that changes the value of sp and then jumps to other code that also changes the value of sp without reading it could be safely optimized by removing the first change to sp.
Maybe the safe thing to do here is to consider any indirect access to memory, i.e. d(ix), (hl), etc a read from sp.

[Philipp Klaus Krause]

Considering every indirect memory read to be a read access of sp is the fix I implemented in [r12035].

[sverx]

Does this protect the local variables from being destroyed by an interrupt being fired while the function is running?

Because that was the case I stumbled upon... the local variables were trashed if an interrupt happened while this function was executed (the PC was pushed onto the stack, over some of the local variables, and the ISR pushed a couple of register pairs too, to preserve them, again over some of the other function local variables...)

[Philipp Klaus Krause]

Yes, it should: The read or write of the local variable would be considered a read of sp.

[Philipp Klaus Krause]

On my Debian GNU/Linux testing system, I had to add --mnax-allocs-per-node 100 to the command line to reproduce the issue: Otherwise the register allocator allocates all variables to registers.

[sverx]

This is weird, I'm not explicitly setting that value to 100 - or anything else.
What's the default? Is there other ways I could have set it to 100 by mistake (environment variables or such)?

[Philipp Klaus Krause]

Lower values for --max-allocs-per-node tend to result in less optimization, especially regarding register allocation.
However, other aspects can affect register allocation, too, such as the C++ library, and the use of the treedec library vs. the Thorup fallback used when the treedec library is not available.