Menu

#182 jp (hl) required to return from naked function

None
closed
None
5
2023-01-25
2022-11-21
Under4Mhz
No

I'm trying to rewrite a function in assembler. I've found I need to add "pop hl/pop af/jp (hl)" at the end instead of "ret" to get it run.

Is that expected? I haven't needed it on any other assembler functions I've rewritten. Is there a pattern I should be looking out for when to add it?

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

#define __IO_VDP_DATA_OUT 0xbe
static volatile __sfr __at __IO_VDP_DATA_OUT VDPDataPortOut;

#define vdu_next_set_fast(value) VDPDataPortOut = value

#if 0
void vdu_memcpy_offset( const uint8_t *data, uint8_t offset, uint8_t size ) {

    do {

        vdu_next_set_fast( *(data++) + offset );

    } while( --size );
}
#else
void vdu_memcpy_offset( const uint8_t *data, uint8_t offset, uint8_t size ) __naked {

__asm

    pop af
    pop bc
    push bc
    push af

    ld a,b
    or a
    ret z

loop_memcpy_offset:

    ld a, (hl)
    add a, c
    out (__IO_VDP_DATA_OUT), a
    inc hl
    djnz loop_memcpy_offset

#if 0
    pop    hl
    pop    af
    jp    (hl)
#endif
    ret

__endasm;
}
#endif

int main() {

    char *data = "Hello world";

    printf("Start\n");
    for(int i = 0; i < 1000; i++ ) {
        vdu_memcpy_offset( data, 2, 11 );
        printf("%d\n", i);
    }
    printf("End\n");

    return 0;
}

I get the following output indefinitely:

uCsim 0.7.6, Copyright (C) 1997 Daniel Drotos.
0> Loading from jphl.ihx
3188 words read from jphl.ihx
r
Simulation started, PC=0x000000
Start
2818
End
Start
2818
End
...
sdcc -v
SDCC : mcs51/z80/z180/r2k/r2ka/r3ka/sm83/tlcs90/ez80_z80/z80n/ds390/pic16/pic14/TININative/ds400/hc08/s08/stm8/pdk13/pdk14/pdk15/mos6502 4.2.9 #13746 (Linux)
1 Attachments

Discussion

  • Philipp Klaus Krause

    You didn't specify which target you use, from the assembler code I guess it is z80.
    See section 4.3.3, "Z80, Z180 and Z80N calling conventions" in the manual, in particular the parts on caller vs. callee-cleanup of stack parameters.

     
    • Under4Mhz

      Under4Mhz - 2022-12-07

      Yes, I missed that. It's z80.

      sdcc -mz80 --fverbose-asm ./jphl.c  -o jphl.ihx && ucsim_z80 -I if=outputs[0xff] jphl.ihx
      

      So the manual states:
      after the call, the stack parameters are cleaned up by the caller

      I read that to mean that main (the caller?) will deal with the stack, and the function vdu_memcpy_offset (the callee?) doesn't need to do anything and just return. Have I understood that correctly?

       
      • Philipp Klaus Krause

        You mean this part: "If __z88dk_callee is not used, after the call, the stack parameters are cleaned up by the caller, with the following exceptions: functions that do not have variable arguments and return void or a type of at most 16 bits, or have both a first parameter of type float and a return value of type float."?

         
        • Under4Mhz

          Under4Mhz - 2022-12-07

          Yes that's the bit. I'm not using __ z88dk_callee and don't have variable arguments, so I interpret that as the stack should be cleaned up by main, not by vdu_memcpy_offset.

          I just re-read that twice, so I'm "returning void" in this case? So I should clean it up? I don't tend to type "return void;" so I missed that. I'd call that "returns nothing".

          That's a complicated sentence. It may be correct but it's quite difficult to read. There's too much in it. It reads like a contract. I'd suggest giving it it's own paragraph and expanding on it. I'm experienced and technically capable and I didn't understand it.

           
          • Maarten Brock

            Maarten Brock - 2022-12-07

            The "contract" is in the function declaration/definition/prototype and the return type is what is in front of the function name. Functions that return void have "void" in front.

             
            • Under4Mhz

              Under4Mhz - 2022-12-09

              I gave an honest effort to read the manual before posting, and I wasn't able to understand what this sentence was attempting to explain.

              If the purpose of the manual is for users to be able to read and understand the stack cleanup issue, then, at least for me, the documentation was unsuccessful.

              I would perhaps suggest expanding this sentence into a few paragraphs, possibly with examples.

              Thanks for your help.

               
  • Philipp Klaus Krause

    Ticket moved from /p/sdcc/bugs/3521/

    Can't be converted:

    • _category: other
     
  • Philipp Klaus Krause

    In [r13815], I've added a flowchart for this in the manual.

     

    Related

    Commit: [r13815]

  • Philipp Klaus Krause

    • status: open --> closed
    • assigned_to: Philipp Klaus Krause
    • Group: -->
     

Log in to post a comment.

Auth0 Logo