Blinkinlabs / ch554_sdcc

CH554 software development kit for SDCC
297 stars 70 forks source link

make more debug functions inline #29

Closed nerdralph closed 4 years ago

nerdralph commented 4 years ago

For issue #17

nerdralph commented 4 years ago

Any issues with the pull request?

cibomahto commented 4 years ago

Thanks- The only issue I can see is that it might be preferable to define the functions as macros, to avoid having function definitions in header files.

nerdralph commented 4 years ago

Thanks- The only issue I can see is that it might be preferable to define the functions as macros, to avoid having function definitions in header files.

Why do you prefer macros? You loose type-safety and introduce the possibility of macro expansion side-effects introducing bugs. SDCC is C99 compliant, and therefore never will emit an external function definition when it is declared inline. https://gustedt.wordpress.com/2010/11/29/myth-and-reality-about-inline-in-c99/

I also made sure the inlined functions were small enough that code space is usually saved even when they are called multiple times. For example CH554UART0SendByte when inlined compiles to just 3 instructions (mov, jnb, & clr) taking 7 bytes. As a standalone function it takes a minimum of 9 bytes if SDCC gets the optimization perfect. Calling the function usually takes 5 bytes; 2 to move the argument to DPL, and 3 for lcall. And if the callee was using any of R0-R7, they need to be pushed/popped since SDCC uses a callee-saves ABI. The speed benefit is also significant; the inline version is typically 2x faster.

cibomahto commented 4 years ago

The optimizations sound great. I was getting hung up on putting function definitions in a header file, but it looks like you're correct about this being the best way to do it, so it makes sense now. Thanks for you explanation!