Add support for generating an autoconf.h header file that can be used in the source instead of #ifdef.
For example, instead of:
#ifdef CONFIG_VERSION_VARIABLE setenv("ver", version_string); /* set version variable */ #endif
you can do:
if (autoconf_version_variable()) setenv("ver", version_string); /* set version variable */
The compiler will ensure that the dead code is eliminated, so the result is the same.
Where the value of the CONFIG define is 0, you can use the autoconf_has...() form. For example CONFIG_BOOTDELAY can be -ve, 0 or +ve, but if it is defined at all, it affects behaviour:
#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) s = getenv ("bootdelay"); #endif
So we use:
if (autoconf_has_bootdelay() && autoconf_bootdelay() >= 0) s = getenv ("bootdelay");
This later form should only be used for such 'difficult' defines where a zero value still means that the CONFIG should be considered to be defined.
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v3: - Update config_xxx() to autoconf_xxx() in comments/README/sed - Update config_xxx_enabled() to autoconf_has_xxx() in comments/README/sed - Add comment as to why we use [A-Za-z0-9_][A-Za-z0-9_]* - Rename sed scripts to more useful names
Changes in v2: - Split out changes to main.c into separate patches - Fix up a few errors and comments in the original RFC - Use autoconf_...() instead of config_...() - Use autoconf_has_...() instead of config_..._enabled() - Add a grep to the sed/sort pipe to speed up processing
Makefile | 43 ++++++++++++++++++++- README | 87 ++++++++++++++++++++++++++++++++++++++++-- include/common.h | 3 ++ include/config_drop.h | 17 +++++++++ tools/scripts/define2value.sed | 37 ++++++++++++++++++ tools/scripts/define2zero.sed | 32 ++++++++++++++++ 6 files changed, 215 insertions(+), 4 deletions(-) create mode 100644 include/config_drop.h create mode 100644 tools/scripts/define2value.sed create mode 100644 tools/scripts/define2zero.sed
diff --git a/Makefile b/Makefile index fc18dd4..8ba068d 100644 --- a/Makefile +++ b/Makefile @@ -614,6 +614,7 @@ updater: # parallel sub-makes creating .depend files simultaneously. depend dep: $(TIMESTAMP_FILE) $(VERSION_FILE) \ $(obj)include/autoconf.mk \ + $(obj)include/generated/autoconf.h \ $(obj)include/generated/generic-asm-offsets.h \ $(obj)include/generated/asm-offsets.h for dir in $(SUBDIRS) $(CPUDIR) $(LDSCRIPT_MAKEFILE_DIR) ; do \ @@ -688,6 +689,45 @@ $(obj)include/autoconf.mk: $(obj)include/config.h sed -n -f tools/scripts/define2mk.sed > $@.tmp && \ mv $@.tmp $@
+# Create a C header file where every '#define CONFIG_XXX value' becomes +# '#define autoconf_xxx() value', or '#define autoconf_xxx() 0' where the +# CONFIG is not used by this board configuration. This allows C code to do +# things like 'if (autoconf_xxx())' and have the compiler remove the dead code, +# instead of using '#ifdef CONFIG_XXX...#endif'. Note that in most cases +# if the autoconf_...() returns 0 then the option is not enabled. In some rare +# cases such as CONFIG_BOOTDELAY, the config can be enabled but still have a +# a value of 0. So in addition we a #define autoconf_has_xxx(), setting the +# value to 0 if the option is disabled, 1 if enabled. This last feature will +# hopefully be deprecated soon. +# The file is regenerated when any U-Boot header file changes. +$(obj)include/generated/autoconf.h: $(obj)include/config.h + @$(XECHO) Generating $@ ; \ + set -e ; \ + : Extract the config macros to a C header file ; \ + $(CPP) $(CFLAGS) -DDO_DEPS_ONLY -dM include/common.h | \ + sed -n -f tools/scripts/define2value.sed > $@.tmp; \ + : Regenerate our list of all config macros if neeed ; \ + if [ ! -f $@-all.tmp ] || \ + find $(src) -name '*.h' -type f -newer $@-all.tmp | \ + egrep -qv 'include/(autoconf.h|generated|config.h)'; \ + then \ + : Extract all config macros from all C header files ; \ + : We can grep for CONFIG since the value will be dropped ; \ + ( \ + find ${src} -name "*.h" -type f | xargs \ + cat | grep CONFIG | \ + sed -n -f tools/scripts/define2zero.sed \ + ) | sort | uniq > $@-all.tmp; \ + fi; \ + : Extract the enabled config macros to a C header file ; \ + $(CPP) $(CFLAGS) -DDO_DEPS_ONLY -dM include/common.h | \ + sed -n -f tools/scripts/define2zero.sed | \ + sort > $@-enabled.tmp; \ + set -e ; \ + : Find CONFIGs that are not enabled ; \ + comm -13 $@-enabled.tmp $@-all.tmp >>$@.tmp && \ + mv $@.tmp $@ + $(obj)include/generated/generic-asm-offsets.h: $(obj)include/autoconf.mk.dep \ $(obj)lib/asm-offsets.s @$(XECHO) Generating $@ @@ -770,7 +810,8 @@ include/license.h: tools/bin2header COPYING unconfig: @rm -f $(obj)include/config.h $(obj)include/config.mk \ $(obj)board/*/config.tmp $(obj)board/*/*/config.tmp \ - $(obj)include/autoconf.mk $(obj)include/autoconf.mk.dep + $(obj)include/autoconf.mk $(obj)include/autoconf.mk.dep \ + $(obj)include/generated/autoconf.h
%_config:: unconfig @$(MKCONFIG) -A $(@:_config=) diff --git a/README b/README index d8cb394..e6f3c04 100644 --- a/README +++ b/README @@ -5434,11 +5434,92 @@ Notes: * If you modify existing code, make sure that your new code does not add to the memory footprint of the code ;-) Small is beautiful! When adding new features, these should compile conditionally only - (using #ifdef), and the resulting code with the new feature - disabled must not need more memory than the old code without your - modification. + (avoiding #ifdef where at all possible), and the resulting code with + the new feature disabled must not need more memory than the old code + without your modification.
* Remember that there is a size limit of 100 kB per message on the u-boot mailing list. Bigger patches will be moderated. If they are reasonable and not too big, they will be acknowledged. But patches bigger than the size limit should be avoided. + + +Use of #ifdef: +-------------- +Many parts of the U-Boot code base are sprinkled with #ifdefs. This makes +different boards compile different versions of the source code, meaning +that we must build all boards to check for failures. It is easy to misspell +an #ifdef and there is not as much checking of this by the compiler. For +someone coming new into the code base, #ifdefs are a big turn-off. Multiple +dependent #ifdefs are harder to do than with if..then..else. Variable +declarations must be #idefed as well as the code that uses them, often much +later in the file/function. #ifdef indents don't match code indents and +have their own separate indent feature. Overall, excessive use of #idef +hurts readability and makes the code harder to modify and refactor. + +In an effort to reduce the use of #ifdef in U-Boot, without requiring lots +of special static inlines all over the header files, a single autoconf.h +header file with lower-case function-type macros has been made available. + +This file has either: + +# #define autoconf_xxx() value + +for enabled options, or: + +# #define autoconf_xxx() 0 + +for disabled options. You can therefore generally change code like this: + + #ifdef CONFIG_XXX + do_something + #else + do_something_else + #endif + +to this: + + if (autoconf_xxx()) + do_something; + else + do_something_else; + +The compiler will see that autoconf_xxx() evalutes to a constant and will +eliminate the dead code. The resulting code (and code size) is the same. + +Multiple #ifdefs can be converted also: + + #if defined(CONFIG_XXX) && !defined(CONFIG_YYY) + do_something + #endif + + if (autoconf_xxx() && !autoconf_yyy()) + do_something; + +Where the macro evaluates to a string, it will be non-NULL, so the above +will work whether the macro is a string or a number. + +This takes care of almost all CONFIG macros. Unfortunately there are a few +cases where a value of 0 does not mean the option is disabled. For example +CONFIG_BOOTDELAY can be defined to 0, which means that the bootdelay +code should be used, but with a value of 0. To get around this and other +sticky cases, an addition macro with an 'has_' prefix is provided, where +the value is always either 0 or 1: + + // Will work even if boaard config has '#define CONFIG_BOOTDELAY 0' + if (autoconf_has_bootdelay()) + do_something; + +(Probably such config options should be deprecated and then we can remove +this feature) + +U-Boot already has a Makefile scheme to permit files to be easily included +based on CONFIG. This can be used where the code to be compiled exists in +its own source file. So the following rules apply: + + 1. Use #ifdef to conditionally compile an exported function or variable + 2. Use ordinary C code with autoconf_xxx() everywhere else + 3. Mark your functions and data structures static where possible + 4. Use the autoconf_has_xxx() variants only if essential + 5. When changing existing code, first create a new patch to replace + #ifdefs in the surrounding area diff --git a/include/common.h b/include/common.h index 4ad17ea..491783b 100644 --- a/include/common.h +++ b/include/common.h @@ -35,6 +35,9 @@ typedef volatile unsigned short vu_short; typedef volatile unsigned char vu_char;
#include <config.h> +#ifndef DO_DEPS_ONLY +#include <generated/autoconf.h> +#endif #include <asm-offsets.h> #include <linux/bitops.h> #include <linux/types.h> diff --git a/include/config_drop.h b/include/config_drop.h new file mode 100644 index 0000000..bf68b50 --- /dev/null +++ b/include/config_drop.h @@ -0,0 +1,17 @@ +/* + * Copyright 2013 Google, Inc + * + * This file is licensed under the terms of the GNU General Public + * License Version 2. This file is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#ifndef _CONFIG_DROP_H +#define _CONFIG_DROP_H + +/* Options which don't seem to be defined by any header in U-Boot */ +#define CONFIG_MENUPROMPT "Auto-boot prompt" +#define CONFIG_MENUKEY +#define CONFIG_UPDATE_TFTP + +#endif diff --git a/tools/scripts/define2value.sed b/tools/scripts/define2value.sed new file mode 100644 index 0000000..205f9fe --- /dev/null +++ b/tools/scripts/define2value.sed @@ -0,0 +1,37 @@ +# +# Sed script to parse CPP macros and generate a list of CONFIG macros +# +# This converts: +# #define CONFIG_XXX value +#into: +# #define autoconf_xxx() value +# #define autoconf_has_xxx() 1 + +# Macros with parameters are ignored. +# (Note we avoid + since it doesn't appear to work) +/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*(/ { + d +} + +# Only process values prefixed with #define CONFIG_ +/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*/ { + # Strip the #define prefix + s/#define[ \t]*CONFIG_/autoconf_/; + # Change to form CONFIG_*=VALUE + s/[\t ][\t ]*/=/; + # Handle lines with no value + s/^([^=]*)$/\1=/; + # Drop trailing spaces + s/ *$//; + # Change empty values to '1' + s/=$/=1/; + # Add #define at the start + s/^([^=]*)=/#define \L\1() / + # print the line + p + # Create autoconf_has_...(), value 1 + s/().*/() 1/ + s/(autoconf_)/\1has_/ + # print the line + p +} diff --git a/tools/scripts/define2zero.sed b/tools/scripts/define2zero.sed new file mode 100644 index 0000000..95e6860 --- /dev/null +++ b/tools/scripts/define2zero.sed @@ -0,0 +1,32 @@ +# +# Sed script to parse CPP macros and generate a list of autoconf macros +# +# This converts: +# #define CONFIG_XXX value +#into: +# #define autoconf_xxx() 0 +# #define autoconf_has_xxx() 0 + +# Macros with parameters are ignored. +# (Note we avoid + since it doesn't appear to work) +/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*(/ { + s/.*// +} + +# Only process values prefixed with #define CONFIG_ +/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*/ { + # Strip the #define prefix + s/#define *//; + # Remove the value + s/[ \t].*//; + # Convert to lower case, prepend #define + s/CONFIG_(.*)/#define autoconf_\L\1/ + # Append 0 + s/$/() 0/ + # print the line + p + # Create autoconf_has_...(), value 0 + s/(autoconf_)/\1has_/ + # print the line + p +}