On Fri, Feb 28, 2020 at 08:42:21AM +0000, Rasmus Villemoes wrote:
On 28/02/2020 00.40, Simon Glass wrote:
Hi Rasmus,
On Thu, 27 Feb 2020 at 00:18, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
Some init functions, e.g. print_resetinfo(), are conditionally defined depending on some config options, and are correspondingly conditionally included in the init_sequence_f[] array.
Others are unconditionally defined and included in init_sequence_f[], but have their entire body, sans a mandatory "return 0", conditionally compiled.
For the simple cases, switch to the former model, making it a bit more consistent. This also makes the U-Boot image very slightly smaller and avoids a few useless calls to no-op functions during board_init_f.
Can you check if it definitely does change the size?
It does, I did build to see how much. On ppc, it's 8 bytes per no-op function (one "put 0 in r3", one blr instruction), plus 4 bytes for the array entry, plus 4 bytes for a .fixup entry - in my case ending up with 7*16=112, because all but the #ifndef CONFIG_OF_EMBED applied.
The reason I ask
is that it inlines those function calls in the normal case, at least from my inspection.
The compiler can't inline and thus eliminate these as they are not called directly, but their addresses are used to populate the init_sequence_f[] array and called through that.
Using if() is preferable to #if if there is no cost.
Completely agree, and I also prefer to have the linker eliminate unused functions rather than cluttering the C code with #ifdefs. But that can't be used in this case.
Anyway, this wasn't primarily to save 112 bytes or whatnot from the U-Boot image, just to use one style a little more consistently.
Perhaps we could come up with a little more macro-magic? In psuedo-code: #define RESERVE_INIT_SEQ_F_ENTRY(fn) \ #if CONFIG_IS_ENABLED(toupper(fn)) reserve_##fn #endif #endif
And then a little harder thinking about negative-case (OF_EMBED) ? Or just have those be the few ifndef's?