On Thu, Jan 13, 2022 at 08:56:02AM +0100, Rasmus Villemoes wrote: > On 12/01/2022 22.56, Tom Rini wrote: > > On Wed, Jan 12, 2022 at 02:28:21PM -0700, Simon Glass wrote: > >> Hi Ilias, > >> > >> On Mon, 1 Nov 2021 at 01:05, Ilias Apalodimas > >> <ilias.apalodi...@linaro.org> wrote: > >>> > >>> On Mon, 1 Nov 2021 at 03:19, Simon Glass <s...@chromium.org> wrote: > >>>> > >>>> At present if an optional Kconfig value needs to be used it must be > >>>> bracketed by #ifdef. For example, with this Kconfig setup: > >>>> > >>>> config WIBBLE > >>>> bool "Support wibbles, the world needs more wibbles" > >>>> > >>>> config WIBBLE_ADDR > >>>> hex "Address of the wibble" > >>>> depends on WIBBLE > >>>> > >>>> then the following code must be used: > >>>> > >>>> #ifdef CONFIG_WIBBLE > >>>> static void handle_wibble(void) > >>>> { > >>>> int val = CONFIG_WIBBLE_ADDR; > >>>> > >>>> ... > >>>> } > >>>> #endif > >>>> > >>> > >>> The example here might be a bit off and we might need this for int > >>> related values. Was this function handle_wibble() supposed to return > >>> an int or not? We could shield the linker easier here without adding > >>> macros. Something along the lines of > >>> static void handle_wibble(void) > >>> { > >>> #ifdef CONFIG_WIBBLE > >>> int val = CONFIG_WIBBLE_ADDR; > >>> #endif > >>> } > >>> > >>> In that case you don't an extra ifdef to call handle_wibble(). > >>> Personally I find this easier to read. > >> > >> But how does that help with the problem here? I am trying to avoid > >> using preprocessor macros in this case. > > > > I'm not sure I see a problem here. A number of the finish-converting-X > > that I did recently had a guard symbol first because usage wasn't fully > > converted but really everyone using that area of code needed to set the > > value, or use the default. > > > > There might be some cases where we do still need a guard symbol because > > usage is in common code and maybe shouldn't be, but instead moved to > > other usage-specific files. > > > > I also think I've seen cases where doing: > > if (CONFIG_EVALUATES_TO_ZERO) { > > ... > > } > > > > takes more space in the binary than an #ifdef does. > > Please provide a specific example. If CONFIG_EVALUATES_TO_ZERO is any > integer-constant-expression evaluating at compile-time to 0, gcc throws > away the whole block very early during parsing. If it doesn't, that's a > compiler bug, so let's please not make decisions based on > not-even-anecdotal data.
OK. I believe it was commit 7856cd5a6dd6 ("Convert CONFIG_SYS_PCI_64BIT to Kconfig") a few platforms changed size and as best I can tell, the used / evaluated value for CONFIG_SYS_PCI_64BIT didn't change. > > And finally for the moment, we also have many cases where zero is a > > valid value. That's what leads to potentially harder to read code or > > needing a guard, I think. > > I like Simon's idea, but the replacement/fallback should _not_ be a > literal 0. We want a guarantee that the code has actually been discarded > by the compiler or linker (i.e., that the access is done in code that is > otherwise guarded by the "parent" Kconfig symbol), so instead the > fallback should be a call to (the nowhere defined of course) > > extern long invalid_use_of_IF_ENABLED_INT(void); > > Of course, if people don't build with -O2 and > -ffunction-sections,-fdata-sections and link with --gc-sections, that > may break, but why should we care? LTO also gets this correct I assume and yes, I like that better. -- Tom
signature.asc
Description: PGP signature