From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Fri, 28 Feb 2020 10:46:20 -0500 Subject: [PATCH] common/board_f.c: use #ifdefs a little more consistently In-Reply-To: <76d3bc74-0406-cd11-8793-b2192ffdb7ee@prevas.dk> References: <20200227081825.11039-1-rasmus.villemoes@prevas.dk> <76d3bc74-0406-cd11-8793-b2192ffdb7ee@prevas.dk> Message-ID: <20200228154620.GJ18302@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > 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: