All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] binutils, gcc: add support for GCC flag -flto
@ 2014-12-20 15:01 Peter Kümmel
  2015-02-03 10:48 ` Thomas Petazzoni
  0 siblings, 1 reply; 2+ messages in thread
From: Peter Kümmel @ 2014-12-20 15:01 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Peter K?mmel <syntheticpp@gmx.net>
---
 package/binutils/Config.in                         | 5 +++++
 package/binutils/binutils.mk                       | 8 ++++++++
 package/gcc/Config.in.host                         | 6 ++++++
 package/gcc/gcc.mk                                 | 4 ++++
 toolchain/toolchain-external/toolchain-external.mk | 3 +++
 5 files changed, 26 insertions(+)

diff --git a/package/binutils/Config.in b/package/binutils/Config.in
index 64d0a09..a5d2cc9 100644
--- a/package/binutils/Config.in
+++ b/package/binutils/Config.in
@@ -19,6 +19,11 @@ config BR2_PACKAGE_BINUTILS_TARGET
 
 	  http://www.gnu.org/software/binutils/
 
+config BR2_PACKAGE_BINUTILS_ENABLE_LTO
+	bool "Enable support for link-time-optimization"
+	help
+	  Enable lto support. Allows passing a lto plugin to ar and ranlib.
+
 endif
 
 comment "binutils needs a toolchain w/ wchar"
diff --git a/package/binutils/binutils.mk b/package/binutils/binutils.mk
index 9a9bb94..3916cad 100644
--- a/package/binutils/binutils.mk
+++ b/package/binutils/binutils.mk
@@ -96,5 +96,13 @@ BINUTILS_PRE_PATCH_HOOKS += BINUTILS_XTENSA_PRE_PATCH
 HOST_BINUTILS_PRE_PATCH_HOOKS += BINUTILS_XTENSA_PRE_PATCH
 endif
 
+ifeq ($(BR2_PACKAGE_BINUTILS_ENABLE_LTO),y)
+BINUTILS_CONF_OPTS += --enable-plugins --enable-lto
+endif
+
+ifeq ($(BR2_GCC_ENABLE_LTO),y)
+HOST_BINUTILS_CONF_OPTS += --enable-plugins --enable-lto
+endif
+
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))
diff --git a/package/gcc/Config.in.host b/package/gcc/Config.in.host
index 0750807..4e0c2ae 100644
--- a/package/gcc/Config.in.host
+++ b/package/gcc/Config.in.host
@@ -128,6 +128,12 @@ config BR2_GCC_ENABLE_TLS
 	  Enable the compiler to generate code for accessing
 	  thread local storage variables
 
+config BR2_GCC_ENABLE_LTO
+	bool "Enable compiler link-time-optimization support"
+	help
+	  Since version 4.5 GCC supports lto. Build GCC with lto support enabled.
+	  Needed when -flto should be used.
+
 config BR2_GCC_ENABLE_OPENMP
 	bool "Enable compiler OpenMP support"
 	depends on !BR2_PTHREADS_NONE && !BR2_avr32 && !BR2_arc && !BR2_microblaze
diff --git a/package/gcc/gcc.mk b/package/gcc/gcc.mk
index cdd71aa..8eaa91d 100644
--- a/package/gcc/gcc.mk
+++ b/package/gcc/gcc.mk
@@ -127,6 +127,10 @@ else
 HOST_GCC_COMMON_CONF_OPTS += --disable-tls
 endif
 
+ifeq ($(BR2_GCC_ENABLE_LTO),y)
+HOST_GCC_COMMON_CONF_OPTS += --enable-plugins --enable-lto
+endif
+
 ifeq ($(BR2_GCC_ENABLE_LIBMUDFLAP),y)
 HOST_GCC_COMMON_CONF_OPTS += --enable-libmudflap
 else
diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
index b07b16c..8baf561 100644
--- a/toolchain/toolchain-external/toolchain-external.mk
+++ b/toolchain/toolchain-external/toolchain-external.mk
@@ -643,6 +643,9 @@ define TOOLCHAIN_EXTERNAL_INSTALL_WRAPPER
 	for i in $(TOOLCHAIN_EXTERNAL_CROSS)*; do \
 		base=$${i##*/}; \
 		case "$$base" in \
+		*-ar|*-ranlib|*-nm) \
+			ln -sf $$(echo $$i | sed 's%^$(HOST_DIR)%../..%') .; \
+			;; \
 		*cc|*cc-*|*++|*++-*|*cpp) \
 			ln -sf ext-toolchain-wrapper $$base; \
 			;; \
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread
* [Buildroot] [PATCH 1/1] binutils, gcc: add support for GCC flag -flto
  2014-12-20 15:01 [Buildroot] [PATCH 1/1] binutils, gcc: add support for GCC flag -flto Peter Kümmel
@ 2015-02-03 10:48 ` Thomas Petazzoni
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Petazzoni @ 2015-02-03 10:48 UTC (permalink / raw)
  To: buildroot

Dear Peter K?mmel,

We reviewed your patch during the Buildroot meeting, here is some
feedback.

For such a non-trivial feature, having an empty commit log is a bit
weird. Can you write down some more details in the commit log?

On Sat, 20 Dec 2014 16:01:09 +0100, Peter K?mmel wrote:

> diff --git a/package/binutils/Config.in b/package/binutils/Config.in
> index 64d0a09..a5d2cc9 100644
> --- a/package/binutils/Config.in
> +++ b/package/binutils/Config.in
> @@ -19,6 +19,11 @@ config BR2_PACKAGE_BINUTILS_TARGET
>  
>  	  http://www.gnu.org/software/binutils/
>  
> +config BR2_PACKAGE_BINUTILS_ENABLE_LTO
> +	bool "Enable support for link-time-optimization"
> +	help
> +	  Enable lto support. Allows passing a lto plugin to ar and ranlib.
> +

We believe this is not needed, as we don't support development on the
target. Even though we have a target binutils package for binutils
libraries (used by oprofile or  other things), or because sometimes
using objdump on the target might be useful, we generally don't support
development on the target, so having the option of enabling LTO in the
target binutils is not very useful.


> +ifeq ($(BR2_PACKAGE_BINUTILS_ENABLE_LTO),y)
> +BINUTILS_CONF_OPTS += --enable-plugins --enable-lto
> +endif

So this chunk would no longer be needed.

> +
> +ifeq ($(BR2_GCC_ENABLE_LTO),y)
> +HOST_BINUTILS_CONF_OPTS += --enable-plugins --enable-lto
> +endif

This looks fine.

> +
>  $(eval $(autotools-package))
>  $(eval $(host-autotools-package))
> diff --git a/package/gcc/Config.in.host b/package/gcc/Config.in.host
> index 0750807..4e0c2ae 100644
> --- a/package/gcc/Config.in.host
> +++ b/package/gcc/Config.in.host
> @@ -128,6 +128,12 @@ config BR2_GCC_ENABLE_TLS
>  	  Enable the compiler to generate code for accessing
>  	  thread local storage variables
>  
> +config BR2_GCC_ENABLE_LTO
> +	bool "Enable compiler link-time-optimization support"
> +	help
> +	  Since version 4.5 GCC supports lto. Build GCC with lto support enabled.
> +	  Needed when -flto should be used.

Since it's only supported since GCC 4.5, then maybe make it 'depends
on !BR2_GCC_VERSION_4_2_2_AVR32_2_1_5'.

> +ifeq ($(BR2_GCC_ENABLE_LTO),y)
> +HOST_GCC_COMMON_CONF_OPTS += --enable-plugins --enable-lto
> +endif

Looks fine.


> +
>  ifeq ($(BR2_GCC_ENABLE_LIBMUDFLAP),y)
>  HOST_GCC_COMMON_CONF_OPTS += --enable-libmudflap
>  else
> diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
> index b07b16c..8baf561 100644
> --- a/toolchain/toolchain-external/toolchain-external.mk
> +++ b/toolchain/toolchain-external/toolchain-external.mk
> @@ -643,6 +643,9 @@ define TOOLCHAIN_EXTERNAL_INSTALL_WRAPPER
>  	for i in $(TOOLCHAIN_EXTERNAL_CROSS)*; do \
>  		base=$${i##*/}; \
>  		case "$$base" in \
> +		*-ar|*-ranlib|*-nm) \
> +			ln -sf $$(echo $$i | sed 's%^$(HOST_DIR)%../..%') .; \
> +			;; \

Please add a comment here in the code to explain why this is happening.
During the review, we had to find a previous version of your patch with
a more detailed commit log to understand what was going on.

I'll mark your patch as 'Changes Requested' in patchwork, so please
resend an updated version otherwise we will forget about it. Thanks a
lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-02-03 10:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-20 15:01 [Buildroot] [PATCH 1/1] binutils, gcc: add support for GCC flag -flto Peter Kümmel
2015-02-03 10:48 ` Thomas Petazzoni

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.