alexforencich / xboot

XBoot Extensible Bootloader
125 stars 69 forks source link

always_inline ??????? #3

Closed rotdrop closed 12 years ago

rotdrop commented 12 years ago

Hi there,

thanks for the great work. One remark: all over the code I see attributes "always_inline". This causes (very recent ...) gcc's to fail, and the compiler is right: it rightly complains that it cannot inline function without the function body around, i.e. it does not make sense to label a function "always_inline" and then just provide the proto-type. What was the reason for all this "always_inline" tags?

Best regards,

Claus Heine

alexforencich commented 12 years ago

The original idea was to eliminate any calling overhead with methods that are only called once. As there is only 4K of program space on the smaller xmega chips, throwing out unnecessary code seemed like a good idea. However, now that I look at those methods in the assembly dump, it looks like it didn't work. Also, it seems the calling overhead for some of those functions is rather minimal, so it should be possible to remove the always_inline attributes. I will investigate further and see what the best solution will be in terms of code size.

rotdrop commented 12 years ago

Hi,

thanks for the response. Of course, I have no objections against inlining. My point is rather simple: the "always_inline" attribute will have no effect unless yourgive the compiler a chance to do the actual inlining, and for this, the compiler needs the function body, not just the proto-type. Example: uart_init(), let's do a simple grep:

$ grep uart_init .c .h uart.c:void attribute ((always_inline)) uart_init(void) xboot.c: uart_init(); uart.h:extern void attribute ((always_inline)) uart_init(void);

This means: you place the functions body in the .c-file uart.c, the proto-type in the header file uart.h, and then the call to uart_init() into the c-file xboot.c. How could possibly gcc inline the function uart_init() when it compiles xboot.c? That is impossible. You would have to place the function definition (not just the proto-type) of uart_init() into uart.h. "inline" is in principle a matter of copy and paste, but for this the compiler needs to know what it shall paste into "xboot.c".

Actually, I just stumbled over it because in my attempt to use the latest and greatest gcc, I was using gcc-4.7 (pre-release) and that one emits an error, not just a warning, stating (rightly) that is is impossible to do the inlining without knowledge about what should actually be inlined.

Ways out:

Best,

Claus

On 14.03.2012 19:14, Alex Forencich wrote:

The original idea was to eliminate any calling overhead with methods that are only called once. As there is only 4K of program space on the smaller xmega chips, throwing out unnecessary code seemed like a good idea. However, now that I look at those methods in the assembly dump, it looks like it didn't work. Also, it seems the calling overhead for some of those functions is rather minimal, so it should be possible to remove the always_inline attributes. I will investigate further and see what the best solution will be in terms of code size.

--- Reply to this email directly or view it on GitHub: https://github.com/alexforencich/xboot/issues/3#issuecomment-4504282

Claus-Justus Heine himself@claus-justus-heine.de

http://http://www.claus-justus-heine.de/

alexforencich commented 12 years ago

I went ahead and removed all the always_inline attributes for the time being. I suppose I could move most of the code into the header files and put them back in. For the time being, though, it's not a big deal as I don't believe the calling overhead is particularly significant.