public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] config.mk: use memoization in cc-option macro to speed up compilation
@ 2011-11-02 18:46 Daniel Schwierzeck
  2011-11-02 20:39 ` Simon Glass
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Daniel Schwierzeck @ 2011-11-02 18:46 UTC (permalink / raw)
  To: u-boot

Apply memoization to cc-option macro by caching the results of the
gcc calls. This macro is called very often so using cached results
leads to faster compilation times.

This feature can be enabled by setting CACHE_CC_OPTIONS=y in the
environment.

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
---

Some MAKEALL runs for ARM and MIPS (on Core 2 Duo E6600 at 2.4GHz):

time CROSS_COMPILE=/opt/codesourcery/arm-2011.03/bin/arm-none-linux-gnueabi-
MAKEALL_LOGDIR=../logs/ BUILD_DIR=../test/ ./MAKEALL -s omap3 -v ti

real	3m0.380s
user	9m30.570s
sys	1m34.550s


time CACHE_CC_OPTIONS=y CROSS_COMPILE=/opt/codesourcery/arm-2011.03/bin/arm-none-linux-gnueabi-
MAKEALL_LOGDIR=../logs/ BUILD_DIR=../test/ ./MAKEALL -s omap3 -v ti

real	1m15.661s
user	4m21.510s
sys	0m33.190s


time CROSS_COMPILE=/opt/codesourcery/mips-2011.03/bin/mips-linux-gnu-
MAKEALL_LOGDIR=../logs/ BUILD_DIR=../test/ ./MAKEALL mips4kc

real	2m49.883s
user	6m25.840s
sys	0m58.200s


time CACHE_CC_OPTIONS=y CROSS_COMPILE=/opt/codesourcery/mips-2011.03/bin/mips-linux-gnu-
MAKEALL_LOGDIR=../logs/ BUILD_DIR=../test/ ./MAKEALL mips4kc

real	2m18.205s
user	4m59.740s
sys	0m39.530s

 config.mk |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/config.mk b/config.mk
index 11b67e5..27c366f 100644
--- a/config.mk
+++ b/config.mk
@@ -107,8 +107,22 @@ HOSTCFLAGS	+= -pedantic
 # Option checker (courtesy linux kernel) to ensure
 # only supported compiler options are used
 #
+ifeq ($(CACHE_CC_OPTIONS),y)
+sinclude $(OBJTREE)/include/cc-options.mk
+
+cc-option-cached = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
+		> /dev/null 2>&1; then \
+		echo 'CC_OPTIONS += $(strip $1)' \
+			>> $(OBJTREE)/include/cc-options.mk; \
+		echo "$(1)"; else echo "$(2)"; fi ;)
+
+cc-option = $(if $(filter $1,$(CC_OPTIONS)),\
+		$(filter $1,$(CC_OPTIONS)),\
+		$(call cc-option-cached,$1,$2))
+else
 cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
 		> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+endif
 
 #
 # Include the make variables (CC, etc...)
-- 
1.7.7.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-02 18:46 [U-Boot] [PATCH] config.mk: use memoization in cc-option macro to speed up compilation Daniel Schwierzeck
@ 2011-11-02 20:39 ` Simon Glass
  2011-11-02 21:36   ` Daniel Schwierzeck
  2011-11-04 12:53 ` [U-Boot] [PATCH v2] " Daniel Schwierzeck
       [not found] ` <1320410756-30391-1-git-send-email-daniel.schwierzeck@googlemail.com>
  2 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2011-11-02 20:39 UTC (permalink / raw)
  To: u-boot

Hi Daniel,

On Wed, Nov 2, 2011 at 11:46 AM, Daniel Schwierzeck
<daniel.schwierzeck@googlemail.com> wrote:
> Apply memoization to cc-option macro by caching the results of the
> gcc calls. This macro is called very often so using cached results
> leads to faster compilation times.
>
> This feature can be enabled by setting CACHE_CC_OPTIONS=y in the
> environment.
>
> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
> ---
>
> Some MAKEALL runs for ARM and MIPS (on Core 2 Duo E6600 at 2.4GHz):
>
> time CROSS_COMPILE=/opt/codesourcery/arm-2011.03/bin/arm-none-linux-gnueabi-
> MAKEALL_LOGDIR=../logs/ BUILD_DIR=../test/ ./MAKEALL -s omap3 -v ti
>
> real ? ?3m0.380s
> user ? ?9m30.570s
> sys ? ? 1m34.550s
>
>
> time CACHE_CC_OPTIONS=y CROSS_COMPILE=/opt/codesourcery/arm-2011.03/bin/arm-none-linux-gnueabi-
> MAKEALL_LOGDIR=../logs/ BUILD_DIR=../test/ ./MAKEALL -s omap3 -v ti
>
> real ? ?1m15.661s
> user ? ?4m21.510s
> sys ? ? 0m33.190s
>
>
> time CROSS_COMPILE=/opt/codesourcery/mips-2011.03/bin/mips-linux-gnu-
> MAKEALL_LOGDIR=../logs/ BUILD_DIR=../test/ ./MAKEALL mips4kc
>
> real ? ?2m49.883s
> user ? ?6m25.840s
> sys ? ? 0m58.200s
>
>
> time CACHE_CC_OPTIONS=y CROSS_COMPILE=/opt/codesourcery/mips-2011.03/bin/mips-linux-gnu-
> MAKEALL_LOGDIR=../logs/ BUILD_DIR=../test/ ./MAKEALL mips4kc
>
> real ? ?2m18.205s
> user ? ?4m59.740s
> sys ? ? 0m39.530s
>
> ?config.mk | ? 14 ++++++++++++++
> ?1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/config.mk b/config.mk
> index 11b67e5..27c366f 100644
> --- a/config.mk
> +++ b/config.mk
> @@ -107,8 +107,22 @@ HOSTCFLAGS += -pedantic
> ?# Option checker (courtesy linux kernel) to ensure
> ?# only supported compiler options are used
> ?#
> +ifeq ($(CACHE_CC_OPTIONS),y)
> +sinclude $(OBJTREE)/include/cc-options.mk

It would be better to put this into include/generated - it will also
make git ignore it and it will be also be removed on clobber which I
think you need.

> +
> +cc-option-cached = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
> + ? ? ? ? ? ? ? > /dev/null 2>&1; then \
> + ? ? ? ? ? ? ? echo 'CC_OPTIONS += $(strip $1)' \
> + ? ? ? ? ? ? ? ? ? ? ? >> $(OBJTREE)/include/cc-options.mk; \
> + ? ? ? ? ? ? ? echo "$(1)"; else echo "$(2)"; fi ;)
> +
> +cc-option = $(if $(filter $1,$(CC_OPTIONS)),\
> + ? ? ? ? ? ? ? $(filter $1,$(CC_OPTIONS)),\
> + ? ? ? ? ? ? ? $(call cc-option-cached,$1,$2))
> +else
> ?cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
> ? ? ? ? ? ? ? ?> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
> +endif
>

Tested-by: Simon Glass <sjg@chromium.org>

With this patch and Wolfgang's I get 1.122s for an incremental build
now. Now I just need faster SPI flash and a faster car to get to work
sooner.

Is there any reason not to enable this option by default?

Regards,
Simon


> ?#
> ?# Include the make variables (CC, etc...)
> --
> 1.7.7.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-02 20:39 ` Simon Glass
@ 2011-11-02 21:36   ` Daniel Schwierzeck
  2011-11-03 19:47     ` Wolfgang Denk
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Schwierzeck @ 2011-11-02 21:36 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 02.11.2011 21:39, Simon Glass wrote:
...
>>
>> diff --git a/config.mk b/config.mk
>> index 11b67e5..27c366f 100644
>> --- a/config.mk
>> +++ b/config.mk
>> @@ -107,8 +107,22 @@ HOSTCFLAGS += -pedantic
>>   # Option checker (courtesy linux kernel) to ensure
>>   # only supported compiler options are used
>>   #
>> +ifeq ($(CACHE_CC_OPTIONS),y)
>> +sinclude $(OBJTREE)/include/cc-options.mk
>
> It would be better to put this into include/generated - it will also
> make git ignore it and it will be also be removed on clobber which I
> think you need.

The patch is still experimental. But I will move that file to 
include/generated in the final version.

>
>> +
>> +cc-option-cached = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
>> +>  /dev/null 2>&1; then \
>> +               echo 'CC_OPTIONS += $(strip $1)' \
>> +>>  $(OBJTREE)/include/cc-options.mk; \
>> +               echo "$(1)"; else echo "$(2)"; fi ;)
>> +
>> +cc-option = $(if $(filter $1,$(CC_OPTIONS)),\
>> +               $(filter $1,$(CC_OPTIONS)),\
>> +               $(call cc-option-cached,$1,$2))
>> +else
>>   cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
>>                 >  /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
>> +endif
>>
>
> Tested-by: Simon Glass<sjg@chromium.org>
>
> With this patch and Wolfgang's I get 1.122s for an incremental build
> now. Now I just need faster SPI flash and a faster car to get to work
> sooner.
>
> Is there any reason not to enable this option by default?

To see the difference between compilation times with and without this 
optimization and to do not break existing code. In the final version I 
would remove the config switch and enable the optimization by default ;)

Best regards,
Daniel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-02 21:36   ` Daniel Schwierzeck
@ 2011-11-03 19:47     ` Wolfgang Denk
  0 siblings, 0 replies; 16+ messages in thread
From: Wolfgang Denk @ 2011-11-03 19:47 UTC (permalink / raw)
  To: u-boot

Dear Daniel Schwierzeck,

In message <4EB1B7DD.40704@googlemail.com> you wrote:
> 
> The patch is still experimental. But I will move that file to 
> include/generated in the final version.

Indeed, please do.

> To see the difference between compilation times with and without this 
> optimization and to do not break existing code. In the final version I 
> would remove the config switch and enable the optimization by default ;)

Looking forward to seeing this final version soon :-)

Thanks!!

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are two kinds of people, those who do the work  and  those  who
take  the  credit.  Try  to  be  in  the  first  group; there is less
competition there.                                    - Indira Gandhi

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-02 18:46 [U-Boot] [PATCH] config.mk: use memoization in cc-option macro to speed up compilation Daniel Schwierzeck
  2011-11-02 20:39 ` Simon Glass
@ 2011-11-04 12:53 ` Daniel Schwierzeck
  2011-11-04 16:32   ` Simon Glass
                     ` (2 more replies)
       [not found] ` <1320410756-30391-1-git-send-email-daniel.schwierzeck@googlemail.com>
  2 siblings, 3 replies; 16+ messages in thread
From: Daniel Schwierzeck @ 2011-11-04 12:53 UTC (permalink / raw)
  To: u-boot

Apply memoization to cc-option macro by caching the results of the
gcc calls. This macro is called very often so using cached results
leads to faster compilation times.

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
---
Changes for v2:
 - move cache file to $(obj)/include/generated
 - reworked completely
 - cache also non-working gcc options
 - remove CACHE_CC_OPTIONS config switch and enable this optimization
   by default

 config.mk |   23 +++++++++++++++++++++--
 1 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/config.mk b/config.mk
index 918cffe..0da961a 100644
--- a/config.mk
+++ b/config.mk
@@ -107,8 +107,27 @@ HOSTCFLAGS	+= -pedantic
 # Option checker (courtesy linux kernel) to ensure
 # only supported compiler options are used
 #
-cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
-		> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+CC_OPTIONS_CACHE_FILE := $(OBJTREE)/include/generated/cc_options.mk
+
+$(if $(wildcard $(CC_OPTIONS_CACHE_FILE)),,\
+	$(shell mkdir -p $(dir $(CC_OPTIONS_CACHE_FILE))))
+
+sinclude $(CC_OPTIONS_CACHE_FILE)
+
+_ccopt_sys = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
+		> /dev/null 2>&1; then \
+		echo 'CC_OPTIONS += $(strip $1)' >> $(CC_OPTIONS_CACHE_FILE); \
+		echo "$(1)"; else \
+		[ "x$(strip $2)" != "x" ] && \
+			echo 'CC_OPTIONS_NOP += $(strip $2)' >> $(CC_OPTIONS_CACHE_FILE); \
+		echo "$(2)"; fi)
+
+_ccopt_cached = $(if $(filter $1,$(CC_OPTIONS)),$1,)
+_ccopt_nop_cached = $(if $(filter $1,$(CC_OPTIONS_NOP)),$1,)
+
+cc-option = $(if $(call _ccopt_cached,$1),$1,\
+		$(if $(call _ccopt_nop_cached,$2),$2,\
+		$(call _ccopt_sys,$1,$2)))
 
 #
 # Include the make variables (CC, etc...)
-- 
1.7.7.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-04 12:53 ` [U-Boot] [PATCH v2] " Daniel Schwierzeck
@ 2011-11-04 16:32   ` Simon Glass
  2011-11-04 17:15     ` Daniel Schwierzeck
  2011-11-04 17:31   ` Daniel Schwierzeck
  2011-11-07 15:26   ` [U-Boot] [PATCH v3] " Daniel Schwierzeck
  2 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2011-11-04 16:32 UTC (permalink / raw)
  To: u-boot

On Fri, Nov 4, 2011 at 5:53 AM, Daniel Schwierzeck
<daniel.schwierzeck@googlemail.com> wrote:
> Apply memoization to cc-option macro by caching the results of the
> gcc calls. This macro is called very often so using cached results
> leads to faster compilation times.
>
> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>

Tested-by: Simon Glass <sjg@chromium.org>

I see a big speed-up with this:

full build 7.05s -> 4.1s
incremental 2.25s -> 1.05s

> ---
> Changes for v2:
> ?- move cache file to $(obj)/include/generated
> ?- reworked completely
> ?- cache also non-working gcc options
> ?- remove CACHE_CC_OPTIONS config switch and enable this optimization
> ? by default
>
> ?config.mk | ? 23 +++++++++++++++++++++--
> ?1 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/config.mk b/config.mk
> index 918cffe..0da961a 100644
> --- a/config.mk
> +++ b/config.mk
> @@ -107,8 +107,27 @@ HOSTCFLAGS += -pedantic
> ?# Option checker (courtesy linux kernel) to ensure
> ?# only supported compiler options are used
> ?#
> -cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
> - ? ? ? ? ? ? ? > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
> +CC_OPTIONS_CACHE_FILE := $(OBJTREE)/include/generated/cc_options.mk
> +
> +$(if $(wildcard $(CC_OPTIONS_CACHE_FILE)),,\
> + ? ? ? $(shell mkdir -p $(dir $(CC_OPTIONS_CACHE_FILE))))
> +
> +sinclude $(CC_OPTIONS_CACHE_FILE)
> +
> +_ccopt_sys = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
> + ? ? ? ? ? ? ? > /dev/null 2>&1; then \
> + ? ? ? ? ? ? ? echo 'CC_OPTIONS += $(strip $1)' >> $(CC_OPTIONS_CACHE_FILE); \
> + ? ? ? ? ? ? ? echo "$(1)"; else \
> + ? ? ? ? ? ? ? [ "x$(strip $2)" != "x" ] && \

Do shell still need that x bit?

> + ? ? ? ? ? ? ? ? ? ? ? echo 'CC_OPTIONS_NOP += $(strip $2)' >> $(CC_OPTIONS_CACHE_FILE); \
> + ? ? ? ? ? ? ? echo "$(2)"; fi)
> +
> +_ccopt_cached = $(if $(filter $1,$(CC_OPTIONS)),$1,)

Do you need the $(if - doesn't filter give you what you want by itself?

> +_ccopt_nop_cached = $(if $(filter $1,$(CC_OPTIONS_NOP)),$1,)
> +
> +cc-option = $(if $(call _ccopt_cached,$1),$1,\
> + ? ? ? ? ? ? ? $(if $(call _ccopt_nop_cached,$2),$2,\
> + ? ? ? ? ? ? ? $(call _ccopt_sys,$1,$2)))
>
> ?#
> ?# Include the make variables (CC, etc...)
> --
> 1.7.7.1
>
>

Thanks for doing this!

Regards,
Simon

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-04 16:32   ` Simon Glass
@ 2011-11-04 17:15     ` Daniel Schwierzeck
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Schwierzeck @ 2011-11-04 17:15 UTC (permalink / raw)
  To: u-boot

Hi SImon,

On Fri, Nov 4, 2011 at 5:32 PM, Simon Glass <sjg@chromium.org> wrote:
> On Fri, Nov 4, 2011 at 5:53 AM, Daniel Schwierzeck
> <daniel.schwierzeck@googlemail.com> wrote:
>> Apply memoization to cc-option macro by caching the results of the
>> gcc calls. This macro is called very often so using cached results
>> leads to faster compilation times.
>>
>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
>
> Tested-by: Simon Glass <sjg@chromium.org>
>
> I see a big speed-up with this:
>
> full build 7.05s -> 4.1s
> incremental 2.25s -> 1.05s
>
>> ---
>> Changes for v2:
>> ?- move cache file to $(obj)/include/generated
>> ?- reworked completely
>> ?- cache also non-working gcc options
>> ?- remove CACHE_CC_OPTIONS config switch and enable this optimization
>> ? by default
>>
>> ?config.mk | ? 23 +++++++++++++++++++++--
>> ?1 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/config.mk b/config.mk
>> index 918cffe..0da961a 100644
>> --- a/config.mk
>> +++ b/config.mk
>> @@ -107,8 +107,27 @@ HOSTCFLAGS += -pedantic
>> ?# Option checker (courtesy linux kernel) to ensure
>> ?# only supported compiler options are used
>> ?#
>> -cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
>> - ? ? ? ? ? ? ? > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
>> +CC_OPTIONS_CACHE_FILE := $(OBJTREE)/include/generated/cc_options.mk
>> +
>> +$(if $(wildcard $(CC_OPTIONS_CACHE_FILE)),,\
>> + ? ? ? $(shell mkdir -p $(dir $(CC_OPTIONS_CACHE_FILE))))
>> +
>> +sinclude $(CC_OPTIONS_CACHE_FILE)
>> +
>> +_ccopt_sys = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
>> + ? ? ? ? ? ? ? > /dev/null 2>&1; then \
>> + ? ? ? ? ? ? ? echo 'CC_OPTIONS += $(strip $1)' >> $(CC_OPTIONS_CACHE_FILE); \
>> + ? ? ? ? ? ? ? echo "$(1)"; else \
>> + ? ? ? ? ? ? ? [ "x$(strip $2)" != "x" ] && \
>
> Do shell still need that x bit?

Probably not but it is safer and does not really harm ;)

>
>> + ? ? ? ? ? ? ? ? ? ? ? echo 'CC_OPTIONS_NOP += $(strip $2)' >> $(CC_OPTIONS_CACHE_FILE); \
>> + ? ? ? ? ? ? ? echo "$(2)"; fi)
>> +
>> +_ccopt_cached = $(if $(filter $1,$(CC_OPTIONS)),$1,)
>
> Do you need the $(if - doesn't filter give you what you want by itself?

You are right, the if is redundant. I will optimize this.

>
>> +_ccopt_nop_cached = $(if $(filter $1,$(CC_OPTIONS_NOP)),$1,)
>> +
>> +cc-option = $(if $(call _ccopt_cached,$1),$1,\
>> + ? ? ? ? ? ? ? $(if $(call _ccopt_nop_cached,$2),$2,\
>> + ? ? ? ? ? ? ? $(call _ccopt_sys,$1,$2)))
>>
>> ?#
>> ?# Include the make variables (CC, etc...)
>> --
>> 1.7.7.1
>>
>>
>
> Thanks for doing this!
>
> Regards,
> Simon
>

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-04 12:53 ` [U-Boot] [PATCH v2] " Daniel Schwierzeck
  2011-11-04 16:32   ` Simon Glass
@ 2011-11-04 17:31   ` Daniel Schwierzeck
  2011-11-04 17:56     ` Wolfgang Denk
  2011-11-07 15:26   ` [U-Boot] [PATCH v3] " Daniel Schwierzeck
  2 siblings, 1 reply; 16+ messages in thread
From: Daniel Schwierzeck @ 2011-11-04 17:31 UTC (permalink / raw)
  To: u-boot

Hi Albert, Wolfgang,

following code from arch/arm/config does not work correctly with my optimization

PF_CPPFLAGS_ABI := $(call cc-option,\
			-mabi=aapcs-linux -mno-thumb-interwork,\
			$(call cc-option,\
				-mapcs-32,\
				$(call cc-option,\
					-mabi=apcs-gnu,\
				)\
			) $(call cc-option,-mno-thumb-interwork,)\
		)
PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARM) $(PF_CPPFLAGS_ABI)

Compiling with seabord_config and latest CodeSourcery toolchain the
generated cache file
has following content:

CC_OPTIONS += -marm
CC_OPTIONS += -mabi=apcs-gnu
CC_OPTIONS_NOP += -mabi=apcs-gnu
CC_OPTIONS += -mno-thumb-interwork
CC_OPTIONS += -mabi=aapcs-linux -mno-thumb-interwork
CC_OPTIONS += -fno-stack-protector
CC_OPTIONS += -Wno-format-nonliteral
CC_OPTIONS += -Wno-format-security
CC_OPTIONS += -fno-toplevel-reorder

If I rewrite that code to

PF_CPPFLAGS_ABI := $(call cc-option,-mabi=aapcs-linux,)
ifeq ($(PF_CPPFLAGS_ABI),)
PF_CPPFLAGS_ABI := $(call cc-option,-mapcs-32,-mabi=apcs-gnu)
endif
PF_CPPFLAGS_THUMB := $(call cc-option,-mno-thumb-interwork,)
PLATFORM_CPPFLAGS += $(PF_CPPFLAGS_ARM) $(PF_CPPFLAGS_ABI) $(PF_CPPFLAGS_THUMB)

the cache file looks better:

CC_OPTIONS += -marm
CC_OPTIONS += -mabi=aapcs-linux
CC_OPTIONS += -mno-thumb-interwork
CC_OPTIONS += -fno-stack-protector
CC_OPTIONS += -Wno-format-nonliteral
CC_OPTIONS += -Wno-format-security
CC_OPTIONS += -fno-toplevel-reorder

Should we change it? is the semantic still the same?

Best regards,
Daniel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-04 17:31   ` Daniel Schwierzeck
@ 2011-11-04 17:56     ` Wolfgang Denk
  2011-11-05  9:16       ` Albert ARIBAUD
  0 siblings, 1 reply; 16+ messages in thread
From: Wolfgang Denk @ 2011-11-04 17:56 UTC (permalink / raw)
  To: u-boot

Dear Daniel Schwierzeck,

In message <CACUy__XY1873+dT-s=-GyPeBq5hnYsmYwWt-gxEXiwY97EcGdg@mail.gmail.com> you wrote:
> 
> Should we change it? is the semantic still the same?

I'm not sure. At first reading it doesn't look really the same to me.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Never ascribe to malice that which can  adequately  be  explained  by
stupidity.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] config.mk: use memoization in cc-option macro to speed up compilation
       [not found] ` <1320410756-30391-1-git-send-email-daniel.schwierzeck@googlemail.com>
@ 2011-11-05  7:17   ` Aneesh V
  0 siblings, 0 replies; 16+ messages in thread
From: Aneesh V @ 2011-11-05  7:17 UTC (permalink / raw)
  To: u-boot

On Friday 04 November 2011 06:15 PM, Daniel Schwierzeck wrote:
> Apply memoization to cc-option macro by caching the results of the
> gcc calls. This macro is called very often so using cached results
> leads to faster compilation times.

This one gives even better results compared to your
previous version.

Tested-by: Aneesh V <aneesh@ti.com>

br,
Aneesh


>
> Signed-off-by: Daniel Schwierzeck<daniel.schwierzeck@googlemail.com>
> ---
> Changes for v2:
>   - move cache file to $(obj)/include/generated
>   - reworked completely
>   - cache also non-working gcc options
>   - remove CACHE_CC_OPTIONS config switch and enable this optimization
>     by default
>
>   config.mk |   23 +++++++++++++++++++++--
>   1 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/config.mk b/config.mk
> index 918cffe..0da961a 100644
> --- a/config.mk
> +++ b/config.mk
> @@ -107,8 +107,27 @@ HOSTCFLAGS	+= -pedantic
>   # Option checker (courtesy linux kernel) to ensure
>   # only supported compiler options are used
>   #
> -cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
> -		>  /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
> +CC_OPTIONS_CACHE_FILE := $(OBJTREE)/include/generated/cc_options.mk
> +
> +$(if $(wildcard $(CC_OPTIONS_CACHE_FILE)),,\
> +	$(shell mkdir -p $(dir $(CC_OPTIONS_CACHE_FILE))))
> +
> +sinclude $(CC_OPTIONS_CACHE_FILE)
> +
> +_ccopt_sys = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
> +		>  /dev/null 2>&1; then \
> +		echo 'CC_OPTIONS += $(strip $1)'>>  $(CC_OPTIONS_CACHE_FILE); \
> +		echo "$(1)"; else \
> +		[ "x$(strip $2)" != "x" ]&&  \
> +			echo 'CC_OPTIONS_NOP += $(strip $2)'>>  $(CC_OPTIONS_CACHE_FILE); \
> +		echo "$(2)"; fi)
> +
> +_ccopt_cached = $(if $(filter $1,$(CC_OPTIONS)),$1,)
> +_ccopt_nop_cached = $(if $(filter $1,$(CC_OPTIONS_NOP)),$1,)
> +
> +cc-option = $(if $(call _ccopt_cached,$1),$1,\
> +		$(if $(call _ccopt_nop_cached,$2),$2,\
> +		$(call _ccopt_sys,$1,$2)))
>
>   #
>   # Include the make variables (CC, etc...)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-04 17:56     ` Wolfgang Denk
@ 2011-11-05  9:16       ` Albert ARIBAUD
  2011-11-05 13:30         ` Wolfgang Denk
  2011-11-05 13:43         ` Daniel Schwierzeck
  0 siblings, 2 replies; 16+ messages in thread
From: Albert ARIBAUD @ 2011-11-05  9:16 UTC (permalink / raw)
  To: u-boot

Hi all,

Le 04/11/2011 18:56, Wolfgang Denk a ?crit :
> Dear Daniel Schwierzeck,
>
> In message<CACUy__XY1873+dT-s=-GyPeBq5hnYsmYwWt-gxEXiwY97EcGdg@mail.gmail.com>  you wrote:
>>
>> Should we change it? is the semantic still the same?
>
> I'm not sure. At first reading it doesn't look really the same to me.

They are not, at least for ELDK4.2.

The only difference is in -mabi options, where the change would reduce 
"-mabi=apcs-gnu -mabi=aapcs-linux" to "-mabi=aapcs-linux".

apcs-gnu, IIUC, is 'old ABI', while 'aapcs-linux' is 'new ABI', aka 
eabi. Most of the toolchains I see are eabi (ELDK and CS notably). There 
may be 'old ABI' toolchains out there, but I don't think they are old 
ABI either.

Anyway, I've just tried ./MAKEALL edminiv2 with ELD42 and a couple of CS 
toolchains, and nowhere in the log does -mabi=apcs-gnu show up -- the 
gcc invocations only have -mabi=aapcs-linux.

I've also tested making ED Mini V2 with and without the patch but 
without Daniel's proposed change to arch/arm/config.mk, and there is no 
difference in build commands (except that for some reason the patch 
inserts multiple spaces between some gcc invocation options.

Daniel, what do you mean with "does not work correctly"?

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-05  9:16       ` Albert ARIBAUD
@ 2011-11-05 13:30         ` Wolfgang Denk
  2011-11-05 13:43         ` Daniel Schwierzeck
  1 sibling, 0 replies; 16+ messages in thread
From: Wolfgang Denk @ 2011-11-05 13:30 UTC (permalink / raw)
  To: u-boot

Dear Albert ARIBAUD,

In message <4EB4FF09.5070601@aribaud.net> you wrote:
> 
> apcs-gnu, IIUC, is 'old ABI', while 'aapcs-linux' is 'new ABI', aka 
> eabi. Most of the toolchains I see are eabi (ELDK and CS notably). There 
> may be 'old ABI' toolchains out there, but I don't think they are old 
> ABI either.
> 
> Anyway, I've just tried ./MAKEALL edminiv2 with ELD42 and a couple of CS 
> toolchains, and nowhere in the log does -mabi=apcs-gnu show up -- the 
> gcc invocations only have -mabi=aapcs-linux.

Try ELDK 4.1 ...


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If God had a beard, he'd be a UNIX programmer.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-05  9:16       ` Albert ARIBAUD
  2011-11-05 13:30         ` Wolfgang Denk
@ 2011-11-05 13:43         ` Daniel Schwierzeck
  2011-11-05 15:02           ` Albert ARIBAUD
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Schwierzeck @ 2011-11-05 13:43 UTC (permalink / raw)
  To: u-boot

Hi Albert,

On 05.11.2011 10:16, Albert ARIBAUD wrote:
> Hi all,
>
> Le 04/11/2011 18:56, Wolfgang Denk a ?crit :
>> Dear Daniel Schwierzeck,
>>
>> In
>> message<CACUy__XY1873+dT-s=-GyPeBq5hnYsmYwWt-gxEXiwY97EcGdg@mail.gmail.com>
>> you wrote:
>>>
>>> Should we change it? is the semantic still the same?
>>
>> I'm not sure. At first reading it doesn't look really the same to me.
>
> They are not, at least for ELDK4.2.
>
> The only difference is in -mabi options, where the change would reduce
> "-mabi=apcs-gnu -mabi=aapcs-linux" to "-mabi=aapcs-linux".
>
> apcs-gnu, IIUC, is 'old ABI', while 'aapcs-linux' is 'new ABI', aka
> eabi. Most of the toolchains I see are eabi (ELDK and CS notably). There
> may be 'old ABI' toolchains out there, but I don't think they are old
> ABI either.
>
> Anyway, I've just tried ./MAKEALL edminiv2 with ELD42 and a couple of CS
> toolchains, and nowhere in the log does -mabi=apcs-gnu show up -- the
> gcc invocations only have -mabi=aapcs-linux.

Looks like I read it wrong. So you always want "-mabi=apcs-gnu 
-mabi=aapcs-linux -mno-thumb-interwork" in $(PF_CPPFLAGS_ABI) with EABI? 
Sorry but I am not an ARM expert ;)

>
> I've also tested making ED Mini V2 with and without the patch but
> without Daniel's proposed change to arch/arm/config.mk, and there is no
> difference in build commands (except that for some reason the patch
> inserts multiple spaces between some gcc invocation options.

I guess this comes from making the macros more readable. Maybe I can 
optimize this.

>
> Daniel, what do you mean with "does not work correctly"?

that the generated cache file looks not right

CC_OPTIONS += -mabi=apcs-gnu
CC_OPTIONS_NOP += -mabi=apcs-gnu
CC_OPTIONS += -mno-thumb-interwork
CC_OPTIONS += -mabi=aapcs-linux -mno-thumb-interwork

But if you want "-mabi=apcs-gnu -mabi=aapcs-linux -mno-thumb-interwork" 
then it should already work correctly without my change in
arch/arm/config.mk.

Best regards,
Daniel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v2] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-05 13:43         ` Daniel Schwierzeck
@ 2011-11-05 15:02           ` Albert ARIBAUD
  0 siblings, 0 replies; 16+ messages in thread
From: Albert ARIBAUD @ 2011-11-05 15:02 UTC (permalink / raw)
  To: u-boot

Hi Daniel,

Le 05/11/2011 14:43, Daniel Schwierzeck a ?crit :
> Hi Albert,
>
> On 05.11.2011 10:16, Albert ARIBAUD wrote:
>> Hi all,
>>
>> Le 04/11/2011 18:56, Wolfgang Denk a ?crit :
>>> Dear Daniel Schwierzeck,
>>>
>>> In
>>> message<CACUy__XY1873+dT-s=-GyPeBq5hnYsmYwWt-gxEXiwY97EcGdg@mail.gmail.com>
>>>
>>> you wrote:
>>>>
>>>> Should we change it? is the semantic still the same?
>>>
>>> I'm not sure. At first reading it doesn't look really the same to me.
>>
>> They are not, at least for ELDK4.2.
>>
>> The only difference is in -mabi options, where the change would reduce
>> "-mabi=apcs-gnu -mabi=aapcs-linux" to "-mabi=aapcs-linux".
>>
>> apcs-gnu, IIUC, is 'old ABI', while 'aapcs-linux' is 'new ABI', aka
>> eabi. Most of the toolchains I see are eabi (ELDK and CS notably). There
>> may be 'old ABI' toolchains out there, but I don't think they are old
>> ABI either.
>>
>> Anyway, I've just tried ./MAKEALL edminiv2 with ELD42 and a couple of CS
>> toolchains, and nowhere in the log does -mabi=apcs-gnu show up -- the
>> gcc invocations only have -mabi=aapcs-linux.
>
> Looks like I read it wrong. So you always want "-mabi=apcs-gnu
> -mabi=aapcs-linux -mno-thumb-interwork" in $(PF_CPPFLAGS_ABI) with EABI?
> Sorry but I am not an ARM expert ;)

No, I don't want that. :)

The problem I see is having two conflicting -mabi options, 
-mabi=apcs-gnu and -mabi=aapcs-linux, in the same command line. There 
should be only one -- and it should be the same across the whole U-Boot 
building process.

The duplicate -mno-thumb-interwork does not worry me fronm a functional 
standpoint; it's just a waste of space, that's all.

>> I've also tested making ED Mini V2 with and without the patch but
>> without Daniel's proposed change to arch/arm/config.mk, and there is no
>> difference in build commands (except that for some reason the patch
>> inserts multiple spaces between some gcc invocation options.
>
> I guess this comes from making the macros more readable. Maybe I can
> optimize this.
>
>>
>> Daniel, what do you mean with "does not work correctly"?
>
> that the generated cache file looks not right
>
> CC_OPTIONS += -mabi=apcs-gnu
> CC_OPTIONS_NOP += -mabi=apcs-gnu
> CC_OPTIONS += -mno-thumb-interwork
> CC_OPTIONS += -mabi=aapcs-linux -mno-thumb-interwork

I'm not a makefile expert, so "looks not right" is a bit meaningless to 
me. If "not right" means "there are several conflicting -mabi options 
and there are repeated -mno-thumb-interwork options" then I agree with 
your "does not look right" statement.

> But if you want "-mabi=apcs-gnu -mabi=aapcs-linux -mno-thumb-interwork"
> then it should already work correctly without my change in
> arch/arm/config.mk.

As I said, no, I don't want that. I want the right -mabi option only, 
and I'd like a single -mno-thumb-interwork option.

But what I don't understand is the discrepancy which I see between the 
CC_OPTIONS resulting from include/generated/cc_options.mk and the actual 
command line options used to generate e.g. edminiv2.

> Best regards,
> Daniel

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-04 12:53 ` [U-Boot] [PATCH v2] " Daniel Schwierzeck
  2011-11-04 16:32   ` Simon Glass
  2011-11-04 17:31   ` Daniel Schwierzeck
@ 2011-11-07 15:26   ` Daniel Schwierzeck
  2011-11-07 21:07     ` Wolfgang Denk
  2 siblings, 1 reply; 16+ messages in thread
From: Daniel Schwierzeck @ 2011-11-07 15:26 UTC (permalink / raw)
  To: u-boot

Apply memoization to cc-option macro by caching the results of the
gcc calls. This macro is called very often so using cached results
leads to faster compilation times.

The old behaviour can be restored by defining the config option
CONFIG_CC_OPT_CACHE_DISABLE=y.

Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
---
Changes for v3:
 - reworked handling of non-working gcc options
 - add config option CONFIG_CC_OPT_CACHE_DISABLE

Changes for v2:
 - move cache file to $(obj)/include/generated
 - reworked completely
 - cache also non-working gcc options
 - remove CACHE_CC_OPTIONS config switch and enable this optimization
   by default

 config.mk |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/config.mk b/config.mk
index 918cffe..ddaa477 100644
--- a/config.mk
+++ b/config.mk
@@ -107,8 +107,24 @@ HOSTCFLAGS	+= -pedantic
 # Option checker (courtesy linux kernel) to ensure
 # only supported compiler options are used
 #
-cc-option = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
-		> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+CC_OPTIONS_CACHE_FILE := $(OBJTREE)/include/generated/cc_options.mk
+
+$(if $(wildcard $(CC_OPTIONS_CACHE_FILE)),,\
+	$(shell mkdir -p $(dir $(CC_OPTIONS_CACHE_FILE))))
+
+-include $(CC_OPTIONS_CACHE_FILE)
+
+cc-option-sys = $(shell if $(CC) $(CFLAGS) $(1) -S -o /dev/null -xc /dev/null \
+		> /dev/null 2>&1; then \
+		echo 'CC_OPTIONS += $(strip $1)' >> $(CC_OPTIONS_CACHE_FILE); \
+		echo "$(1)"; fi)
+
+ifeq ($(CONFIG_CC_OPT_CACHE_DISABLE),y)
+cc-option = $(strip $(if $(call cc-option-sys,$1),$1,$2))
+else
+cc-option = $(strip $(if $(findstring $1,$(CC_OPTIONS)),$1,\
+		$(if $(call cc-option-sys,$1),$1,$2)))
+endif
 
 #
 # Include the make variables (CC, etc...)
-- 
1.7.7.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [U-Boot] [PATCH v3] config.mk: use memoization in cc-option macro to speed up compilation
  2011-11-07 15:26   ` [U-Boot] [PATCH v3] " Daniel Schwierzeck
@ 2011-11-07 21:07     ` Wolfgang Denk
  0 siblings, 0 replies; 16+ messages in thread
From: Wolfgang Denk @ 2011-11-07 21:07 UTC (permalink / raw)
  To: u-boot

Dear Daniel Schwierzeck,

In message <1320679603-24847-1-git-send-email-daniel.schwierzeck@googlemail.com> you wrote:
> Apply memoization to cc-option macro by caching the results of the
> gcc calls. This macro is called very often so using cached results
> leads to faster compilation times.
> 
> The old behaviour can be restored by defining the config option
> CONFIG_CC_OPT_CACHE_DISABLE=y.
> 
> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck@googlemail.com>
> ---
> Changes for v3:
>  - reworked handling of non-working gcc options
>  - add config option CONFIG_CC_OPT_CACHE_DISABLE
> 
> Changes for v2:
>  - move cache file to $(obj)/include/generated
>  - reworked completely
>  - cache also non-working gcc options
>  - remove CACHE_CC_OPTIONS config switch and enable this optimization
>    by default
> 
>  config.mk |   20 ++++++++++++++++++--
>  1 files changed, 18 insertions(+), 2 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Human beings were created by water to transport it uphill.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2011-11-07 21:07 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-02 18:46 [U-Boot] [PATCH] config.mk: use memoization in cc-option macro to speed up compilation Daniel Schwierzeck
2011-11-02 20:39 ` Simon Glass
2011-11-02 21:36   ` Daniel Schwierzeck
2011-11-03 19:47     ` Wolfgang Denk
2011-11-04 12:53 ` [U-Boot] [PATCH v2] " Daniel Schwierzeck
2011-11-04 16:32   ` Simon Glass
2011-11-04 17:15     ` Daniel Schwierzeck
2011-11-04 17:31   ` Daniel Schwierzeck
2011-11-04 17:56     ` Wolfgang Denk
2011-11-05  9:16       ` Albert ARIBAUD
2011-11-05 13:30         ` Wolfgang Denk
2011-11-05 13:43         ` Daniel Schwierzeck
2011-11-05 15:02           ` Albert ARIBAUD
2011-11-07 15:26   ` [U-Boot] [PATCH v3] " Daniel Schwierzeck
2011-11-07 21:07     ` Wolfgang Denk
     [not found] ` <1320410756-30391-1-git-send-email-daniel.schwierzeck@googlemail.com>
2011-11-05  7:17   ` [U-Boot] [PATCH v2] " Aneesh V

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox