[PATCH] common/board_f.c: use #ifdefs a little more consistently

[Tom Rini <trini@konsulko.com>]

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?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200228/0e4c7bb6/attachment.sig>