* IS_ENABLED() and non-available symbols @ 2011-08-14 10:56 Rabin Vincent 2011-08-14 16:55 ` Arnaud Lacombe 0 siblings, 1 reply; 6+ messages in thread From: Rabin Vincent @ 2011-08-14 10:56 UTC (permalink / raw) To: Michal Marek; +Cc: linux-kbuild, LKML I'm trying to use the new IS_ENABLED() stuff, but it appears that it only works if the symbol being tested has its dependencies met, and results in compiler errors if that is not the case. For example, I've added code using IS_ENABLED(CONFIG_THUMB2_KERNEL) as in the below patch. My config is arch/arm/configs/omap2plus_defconfig which doesn't satisfy the dependencies for CONFIG_THUMB2_KERNEL (defined in arch/arm/Kconfig). Compiling results in the following errors (and indeed the macro hasn't been generated in include/generated/autoconfg.h). If I modify my config to satisfy the dependencies for CONFIG_THUMB2_KERNEL so that it is selectable, the __enabled_CONFIG_THUMB2_KERNEL macro gets generated. arch/arm/kernel/setup.c: In function 'c_show': arch/arm/kernel/setup.c:1056: error: '__enabled_CONFIG_THUMB2_KERNEL' undeclared (first use in this function) arch/arm/kernel/setup.c:1056: error: (Each undeclared identifier is reported only once arch/arm/kernel/setup.c:1056: error: for each function it appears in.) arch/arm/kernel/setup.c:1056: error: '__enabled_CONFIG_THUMB2_KERNEL_MODULE' undeclared (first use in this function) Kernel is latest linus (v3.1-rc1-272-g73e0881). diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index 70bca64..59aa480 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -1053,6 +1053,9 @@ static int c_show(struct seq_file *m, void *v) seq_printf(m, "Serial\t\t: %08x%08x\n", system_serial_high, system_serial_low); + if (IS_ENABLED(CONFIG_THUMB2_KERNEL)) + printk("Thumb-2\n"); + return 0; } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: IS_ENABLED() and non-available symbols 2011-08-14 10:56 IS_ENABLED() and non-available symbols Rabin Vincent @ 2011-08-14 16:55 ` Arnaud Lacombe 2011-08-14 17:02 ` Arnaud Lacombe 0 siblings, 1 reply; 6+ messages in thread From: Arnaud Lacombe @ 2011-08-14 16:55 UTC (permalink / raw) To: Rabin Vincent; +Cc: Michal Marek, linux-kbuild, LKML Hi, On Sun, Aug 14, 2011 at 6:56 AM, Rabin Vincent <rabin@rab.in> wrote: > I'm trying to use the new IS_ENABLED() stuff, but it appears that it > only works if the symbol being tested has its dependencies met, and > results in compiler errors if that is not the case. > > For example, I've added code using IS_ENABLED(CONFIG_THUMB2_KERNEL) as > in the below patch. My config is arch/arm/configs/omap2plus_defconfig > which doesn't satisfy the dependencies for CONFIG_THUMB2_KERNEL (defined > in arch/arm/Kconfig). > > Compiling results in the following errors (and indeed the macro hasn't > been generated in include/generated/autoconfg.h). If I modify my config > to satisfy the dependencies for CONFIG_THUMB2_KERNEL so that it is > selectable, the __enabled_CONFIG_THUMB2_KERNEL macro gets generated. > > arch/arm/kernel/setup.c: In function 'c_show': > arch/arm/kernel/setup.c:1056: error: '__enabled_CONFIG_THUMB2_KERNEL' > undeclared (first use in this function) > arch/arm/kernel/setup.c:1056: error: (Each undeclared identifier is > reported only once > arch/arm/kernel/setup.c:1056: error: for each function it appears in.) > arch/arm/kernel/setup.c:1056: error: > '__enabled_CONFIG_THUMB2_KERNEL_MODULE' undeclared (first use in this > function) > > Kernel is latest linus (v3.1-rc1-272-g73e0881). > Ahhh! That is why we have been stuck with #ifdef all these years! What you point out is quite interesting. I guess that we did not carefully understood the context in which this was printed. That is, we will only ends up printing something if the symbol is visible, or has been selected, or has a default. That said __enabled_CONFIG_THUMB2_KERNEL will get generated whenever it appears in the menuconfig (ie. it's dependency are met). The solution is trivial, but I am not sure we want to go that way: we need to generated a __enabled_ entry for symbols for _all_ symbols in the configuration, even internal one, as I am sure someone will at some point do: if (IS_ENABLED(CONFIG_WIRELESS_EXT)) { .... } but this will continue to be broken until we have a global symbol namespace for all architecture, ie: if (IS_ENABLED(CONFIG_ARM)) { .... } will not be expected to work for an ARCH=mips build. - Arnaud > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c > index 70bca64..59aa480 100644 > --- a/arch/arm/kernel/setup.c > +++ b/arch/arm/kernel/setup.c > @@ -1053,6 +1053,9 @@ static int c_show(struct seq_file *m, void *v) > seq_printf(m, "Serial\t\t: %08x%08x\n", > system_serial_high, system_serial_low); > > + if (IS_ENABLED(CONFIG_THUMB2_KERNEL)) > + printk("Thumb-2\n"); > + > return 0; > } > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: IS_ENABLED() and non-available symbols 2011-08-14 16:55 ` Arnaud Lacombe @ 2011-08-14 17:02 ` Arnaud Lacombe 2011-08-15 10:55 ` Michal Marek 0 siblings, 1 reply; 6+ messages in thread From: Arnaud Lacombe @ 2011-08-14 17:02 UTC (permalink / raw) To: Rabin Vincent; +Cc: Michal Marek, linux-kbuild, linux-kernel, Arnaud Lacombe Hi, On Sun, Aug 14, 2011 at 12:55 PM, Arnaud Lacombe <lacombar@gmail.com> wrote: > The solution is trivial, but I am not sure we want to go that way: we > need to generated a __enabled_ entry for symbols for _all_ symbols in > the configuration, even internal one > That should do the job... -= NOT FOR MERGE =- Not-Signed-off-by: Arnaud Lacombe <lacombar@gmail.com> --- scripts/kconfig/confdata.c | 42 +++++++++++++++++++++++++++++++----------- 1 files changed, 31 insertions(+), 11 deletions(-) diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c index 59b667c..e976ccb 100644 --- a/scripts/kconfig/confdata.c +++ b/scripts/kconfig/confdata.c @@ -503,17 +503,6 @@ header_print_symbol(FILE *fp, struct symbol *sym, const char *value, void *arg) fprintf(fp, "#define %s%s%s 1\n", CONFIG_, sym->name, suffix); } - /* - * Generate the __enabled_CONFIG_* and - * __enabled_CONFIG_*_MODULE macros for use by the - * IS_{ENABLED,BUILTIN,MODULE} macros. The _MODULE variant is - * generated even for booleans so that the IS_ENABLED() macro - * works. - */ - fprintf(fp, "#define __enabled_" CONFIG_ "%s %d\n", - sym->name, (*value == 'y')); - fprintf(fp, "#define __enabled_" CONFIG_ "%s_MODULE %d\n", - sym->name, (*value == 'm')); break; } case S_HEX: { @@ -565,6 +554,35 @@ static struct conf_printer header_printer_cb = }; /* + * Generate the __enabled_CONFIG_* and __enabled_CONFIG_*_MODULE macros for + * use by the IS_{ENABLED,BUILTIN,MODULE} macros. The _MODULE variant is + * generated even for booleans so that the IS_ENABLED() macro works. + */ +static void +header_print__enabled_symbol(FILE *fp, struct symbol *sym, const char *value, void *arg) +{ + + switch (sym->type) { + case S_BOOLEAN: + case S_TRISTATE: { + fprintf(fp, "#define __enabled_" CONFIG_ "%s %d\n", + sym->name, (*value == 'y')); + fprintf(fp, "#define __enabled_" CONFIG_ "%s_MODULE %d\n", + sym->name, (*value == 'm')); + break; + } + default: + break; + } +} + +static struct conf_printer header__enabled_printer_cb = +{ + .print_symbol = header_print__enabled_symbol, + .print_comment = header_print_comment, +}; + +/* * Tristate printer * * This printer is used when generating the `include/config/tristate.conf' file. @@ -945,6 +963,8 @@ int conf_write_autoconf(void) conf_write_heading(out_h, &header_printer_cb, NULL); for_all_symbols(i, sym) { + conf_write_symbol(out_h, sym, &header__enabled_printer_cb, NULL); + sym_calc_value(sym); if (!(sym->flags & SYMBOL_WRITE) || !sym->name) continue; -- 1.7.6.153.g78432 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: IS_ENABLED() and non-available symbols 2011-08-14 17:02 ` Arnaud Lacombe @ 2011-08-15 10:55 ` Michal Marek 2011-08-15 14:52 ` Arnaud Lacombe 0 siblings, 1 reply; 6+ messages in thread From: Michal Marek @ 2011-08-15 10:55 UTC (permalink / raw) To: Arnaud Lacombe; +Cc: Rabin Vincent, linux-kbuild, linux-kernel On 14.8.2011 19:02, Arnaud Lacombe wrote: > Hi, > > On Sun, Aug 14, 2011 at 12:55 PM, Arnaud Lacombe <lacombar@gmail.com> wrote: >> The solution is trivial, but I am not sure we want to go that way: we >> need to generated a __enabled_ entry for symbols for _all_ symbols in >> the configuration, even internal one >> > That should do the job... > > -= NOT FOR MERGE =- > > Not-Signed-off-by: Arnaud Lacombe <lacombar@gmail.com> > > --- > scripts/kconfig/confdata.c | 42 +++++++++++++++++++++++++++++++----------- > 1 files changed, 31 insertions(+), 11 deletions(-) > > for_all_symbols(i, sym) { > + conf_write_symbol(out_h, sym, &header__enabled_printer_cb, NULL); > + > sym_calc_value(sym); > if (!(sym->flags & SYMBOL_WRITE) || !sym->name) > continue; I like the approach, it will work even with not visible symbols and still catch typos (which #ifdef does not protect against). But with the patch as-is, the generated autoconf.h starts with lots of #define __enabled_CONFIG_(null) 1 #define __enabled_CONFIG_(null)_MODULE 0 #define __enabled_CONFIG_(null) 1 #define __enabled_CONFIG_(null)_MODULE 0 #define __enabled_CONFIG_(null) 0 Michal ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: IS_ENABLED() and non-available symbols 2011-08-15 10:55 ` Michal Marek @ 2011-08-15 14:52 ` Arnaud Lacombe 2011-08-15 19:04 ` Arnaud Lacombe 0 siblings, 1 reply; 6+ messages in thread From: Arnaud Lacombe @ 2011-08-15 14:52 UTC (permalink / raw) To: Michal Marek; +Cc: Rabin Vincent, linux-kbuild, linux-kernel Hi, 2011/8/15 Michal Marek <mmarek@suse.cz>: > On 14.8.2011 19:02, Arnaud Lacombe wrote: >> Hi, >> >> On Sun, Aug 14, 2011 at 12:55 PM, Arnaud Lacombe <lacombar@gmail.com> wrote: >>> The solution is trivial, but I am not sure we want to go that way: we >>> need to generated a __enabled_ entry for symbols for _all_ symbols in >>> the configuration, even internal one >>> >> That should do the job... >> >> -= NOT FOR MERGE =- >> >> Not-Signed-off-by: Arnaud Lacombe <lacombar@gmail.com> >> >> --- >> scripts/kconfig/confdata.c | 42 +++++++++++++++++++++++++++++++----------- >> 1 files changed, 31 insertions(+), 11 deletions(-) >> >> for_all_symbols(i, sym) { >> + conf_write_symbol(out_h, sym, &header__enabled_printer_cb, NULL); >> + >> sym_calc_value(sym); >> if (!(sym->flags & SYMBOL_WRITE) || !sym->name) >> continue; > > I like the approach, it will work even with not visible symbols and > still catch typos (which #ifdef does not protect against). > actually, the print should be after `sym_calc_value(sym)' to get the value right.. > But with the > patch as-is, the generated autoconf.h starts with lots of > > #define __enabled_CONFIG_(null) 1 > #define __enabled_CONFIG_(null)_MODULE 0 > #define __enabled_CONFIG_(null) 1 > #define __enabled_CONFIG_(null)_MODULE 0 > #define __enabled_CONFIG_(null) 0 > that should be trivial to fix. Sunday morning patches... I'd be interested to also know the cost of all those new symbols on build time... - Arnaud ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: IS_ENABLED() and non-available symbols 2011-08-15 14:52 ` Arnaud Lacombe @ 2011-08-15 19:04 ` Arnaud Lacombe 0 siblings, 0 replies; 6+ messages in thread From: Arnaud Lacombe @ 2011-08-15 19:04 UTC (permalink / raw) To: Michal Marek; +Cc: Rabin Vincent, linux-kbuild, linux-kernel Hi, On Mon, Aug 15, 2011 at 10:52 AM, Arnaud Lacombe <lacombar@gmail.com> wrote: > [...] > I'd be interested to also know the cost of all those new symbols on > build time... > here it is; on the parsing itself: After 1000 iteration of: % /usr/libexec/gcc/x86_64-redhat-linux/4.5.1/cc1 -E -quiet -v -include include/generated/autoconf.h /dev/null -o /dev/null -mtune=generic -march=x86-64 2>/dev/null x include.before + include.after +--------------------------------------------------------------------------+ | x x x x x x + + + + + + + + +| ||MA_| |_A_| | +--------------------------------------------------------------------------+ N Min Max Median Avg Stddev x 1000 0.016 0.021 0.016 0.01637 0.00077310911 + 1000 0.034 0.043 0.035 0.035227 0.00076161083 Difference at 95.0% confidence 0.018857 +/- 6.7264e-05 115.192% +/- 0.410898% (Student's t, pooled s = 0.000767382) After 1000 successive build of `init/main.o' (directly calling gcc): x include.before + include.after +--------------------------------------------------------------------------+ | xxxxx**++++++ + + | | xxxxx***+++++ x ++ x + + | | xxxxx***++*+++ +x+ x x x++ x + x+ ++* x x * | | xxxxx****+*++**+**+*+*x x*xx*++*x++*+**x+ *+* +x *++x* x*++ + +| ||____|_A___M_A|_____| | +--------------------------------------------------------------------------+ N Min Max Median Avg Stddev x 1000 0.628 0.835 0.631 0.636217 0.024724466 + 1000 0.648 0.873 0.651 0.657341 0.026185882 Difference at 95.0% confidence 0.021124 +/- 0.00223216 3.32025% +/- 0.350849% (Student's t, pooled s = 0.0254657) Now, an `allyesconfig' take about 5h30 hours to complete on my Core2 T5670 (capped at 1.2GHz), so this patch would cost an extra 10min. Still significant, but not too much :-) - Arnaud ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-08-15 19:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-08-14 10:56 IS_ENABLED() and non-available symbols Rabin Vincent 2011-08-14 16:55 ` Arnaud Lacombe 2011-08-14 17:02 ` Arnaud Lacombe 2011-08-15 10:55 ` Michal Marek 2011-08-15 14:52 ` Arnaud Lacombe 2011-08-15 19:04 ` Arnaud Lacombe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.