diff mbox

IS_ENABLED() and non-available symbols

Message ID 1313341346-15605-1-git-send-email-lacombar@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Arnaud Lacombe Aug. 14, 2011, 5:02 p.m. UTC
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(-)

Comments

Michal Marek Aug. 15, 2011, 10:55 a.m. UTC | #1
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
--
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
Arnaud Lacombe Aug. 15, 2011, 2:52 p.m. UTC | #2
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
--
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
Arnaud Lacombe Aug. 15, 2011, 7:04 p.m. UTC | #3
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
--
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
diff mbox

Patch

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;