diff mbox

package/kmsxx: add patch to fix LTO support

Message ID 20160826213442.19246-1-arnout@mind.be
State Superseded
Headers show

Commit Message

Arnout Vandecappelle Aug. 26, 2016, 9:34 p.m. UTC
The LTO support in the kmsxx package uses the host gcc-ar and gcc-ranlib
instead of the ones from the cross-toolchain. Add a patch that tries to
find the right one based on CMAKE_C_COMPILER.

Fixes:
http://autobuild.buildroot.net/results/16a/16a38a4277dd1152a5955d62cb92f85447791ef3

Possibly also fixes:
http://autobuild.buildroot.net/results/f3c/f3c48da3a9706cd366c0e0a96c3cd0ff959f2a78
(it fails later, possibly because an incompatible host ar)

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Thomas P. proposed to instead add a variable for LTO_AR and LTO_RANLIB
that we would override. However, this version works and is upstreamable
and currently we don't even "know" if cross-gcc-ar exists or not.

CMake actually has a target property INTERPROCEDURAL_OPTIMIZATION but
it seems that this is currently only implemented for Intel ICC. Cfr.
https://cmake.org/Bug/view.php?id=15939
---
 ...001-Fix-LTO-support-for-cross-compilation.patch | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 package/kmsxx/0001-Fix-LTO-support-for-cross-compilation.patch

Comments

Thomas Petazzoni Aug. 27, 2016, 7:22 a.m. UTC | #1
Hello,

On Fri, 26 Aug 2016 23:34:42 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:
> The LTO support in the kmsxx package uses the host gcc-ar and gcc-ranlib
> instead of the ones from the cross-toolchain. Add a patch that tries to
> find the right one based on CMAKE_C_COMPILER.

Thanks for this patch! One quick question though.

> +     if (HAS_LTO_FLAG)
> +-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto")
> +-        set(CMAKE_AR gcc-ar)
> +-        set(CMAKE_RANLIB gcc-ranlib)
> ++        find_program(LTO_AR NAMES "${CMAKE_C_COMPILER}-ar" gcc-ar)
> ++        find_program(LTO_RANLIB NAMES "${CMAKE_C_COMPILER}-ranlib" /usr/bin/gcc-ranlib)

Why is the last attempted value for AR "gcc-ar", and for RANLIB it's
"/usr/bin/gcc-ranlib" ? I.e while just the program name in one case,
and a full path in the other case.

Thanks,

Thomas
Samuel Martin Aug. 27, 2016, 9:03 a.m. UTC | #2
Hi all,

On August 26, 2016 11:34:42 PM GMT+02:00, "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be> wrote:
>The LTO support in the kmsxx package uses the host gcc-ar and
>gcc-ranlib
>instead of the ones from the cross-toolchain. Add a patch that tries to
>find the right one based on CMAKE_C_COMPILER.
>
>Fixes:
>http://autobuild.buildroot.net/results/16a/16a38a4277dd1152a5955d62cb92f85447791ef3
>
>Possibly also fixes:
>http://autobuild.buildroot.net/results/f3c/f3c48da3a9706cd366c0e0a96c3cd0ff959f2a78
>(it fails later, possibly because an incompatible host ar)
>
>Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>Cc: Samuel Martin <s.martin49@gmail.com>
>Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>---
>Thomas P. proposed to instead add a variable for LTO_AR and LTO_RANLIB
>that we would override. However, this version works and is upstreamable
>and currently we don't even "know" if cross-gcc-ar exists or not.
>
>CMake actually has a target property INTERPROCEDURAL_OPTIMIZATION but
>it seems that this is currently only implemented for Intel ICC. Cfr.
>https://cmake.org/Bug/view.php?id=15939
>---
>...001-Fix-LTO-support-for-cross-compilation.patch | 45
>++++++++++++++++++++++
> 1 file changed, 45 insertions(+)
>create mode 100644
>package/kmsxx/0001-Fix-LTO-support-for-cross-compilation.patch
>
>diff --git
>a/package/kmsxx/0001-Fix-LTO-support-for-cross-compilation.patch
>b/package/kmsxx/0001-Fix-LTO-support-for-cross-compilation.patch
>new file mode 100644
>index 0000000..9b4a582
>--- /dev/null
>+++ b/package/kmsxx/0001-Fix-LTO-support-for-cross-compilation.patch
>@@ -0,0 +1,45 @@
>+From 5da1f631bc753655ac94b08a6233eecd0d451327 Mon Sep 17 00:00:00 2001
>+From: "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be>
>+Date: Fri, 26 Aug 2016 21:55:06 +0200
>+Subject: [PATCH] Fix LTO support for cross-compilation.
>+
>+When cross-compiling, the ar and ranlib to be used for LTO are
>prefixed
>+by the cross-tuple. gcc-ar and gcc-ranlib may not exist. Cfr.
>+http://autobuild.buildroot.net/results/f3c/f3c48da3a9706cd366c0e0a96c3cd0ff959f2a78/
>+
>+Therefore, search for an appropriate lto-ar and lto-ranlib before
>+enabling LTO.
>+
>+Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>+---
>+Submitted upstream: https://github.com/tomba/kmsxx/pull/14
>+---
>+ CMakeLists.txt | 12 +++++++++---
>+ 1 file changed, 9 insertions(+), 3 deletions(-)
>+
>+diff --git a/CMakeLists.txt b/CMakeLists.txt
>+index e5b5ea5..c61c81d 100644
>+--- a/CMakeLists.txt
>++++ b/CMakeLists.txt
>+@@ -39,9 +39,15 @@ if (NOT ${U_CMAKE_BUILD_TYPE} MATCHES DEBUG)
>+     CHECK_CXX_COMPILER_FLAG("-flto" HAS_LTO_FLAG)
>+ 
>+     if (HAS_LTO_FLAG)
>+-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto")
>+-        set(CMAKE_AR gcc-ar)
>+-        set(CMAKE_RANLIB gcc-ranlib)
>++        find_program(LTO_AR NAMES "${CMAKE_C_COMPILER}-ar" gcc-ar)
>++        find_program(LTO_RANLIB NAMES "${CMAKE_C_COMPILER}-ranlib"
>/usr/bin/gcc-ranlib)
>++        if (LTO_AR AND LTO_RANLIB)
>++            set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto")
>++            set(CMAKE_AR "${LTO_AR}")
>++            set(CMAKE_RANLIB "${LTO_RANLIB}")

These definitions belong to the toolchainfile.cmake file IMHO, but it can be moved there in a follow-up patch.

Regards,

>++        else()
>++            message(STATUS "gcc-ar or gcc-ranlib not found, disabling
>LTO")
>++        endif()
>+     endif()
>+ endif()
>+ 
>+-- 
>+2.9.3
>+
Arnout Vandecappelle Aug. 27, 2016, 10:21 a.m. UTC | #3
On 27-08-16 09:22, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 26 Aug 2016 23:34:42 +0200, Arnout Vandecappelle
> (Essensium/Mind) wrote:
>> The LTO support in the kmsxx package uses the host gcc-ar and gcc-ranlib
>> instead of the ones from the cross-toolchain. Add a patch that tries to
>> find the right one based on CMAKE_C_COMPILER.
> 
> Thanks for this patch! One quick question though.
> 
>> +     if (HAS_LTO_FLAG)
>> +-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto")
>> +-        set(CMAKE_AR gcc-ar)
>> +-        set(CMAKE_RANLIB gcc-ranlib)
>> ++        find_program(LTO_AR NAMES "${CMAKE_C_COMPILER}-ar" gcc-ar)
>> ++        find_program(LTO_RANLIB NAMES "${CMAKE_C_COMPILER}-ranlib" /usr/bin/gcc-ranlib)
> 
> Why is the last attempted value for AR "gcc-ar", and for RANLIB it's
> "/usr/bin/gcc-ranlib" ? I.e while just the program name in one case,
> and a full path in the other case.

 The /usr/bin/gcc-ranlib is obviously wrong. That was a left-over from a test to
be sure that find_program also finds it if the full path is given.

 I'll send a v2.

 Regards,
 Arnout

> 
> Thanks,
> 
> Thomas
>
Arnout Vandecappelle Aug. 27, 2016, 10:40 a.m. UTC | #4
On 27-08-16 11:03, Samuel Martin wrote:
> Hi all,
> 
> On August 26, 2016 11:34:42 PM GMT+02:00, "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be> wrote:
>> The LTO support in the kmsxx package uses the host gcc-ar and
>> gcc-ranlib
>> instead of the ones from the cross-toolchain. Add a patch that tries to
>> find the right one based on CMAKE_C_COMPILER.
[snip]
>> +     if (HAS_LTO_FLAG)
>> +-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto")
>> +-        set(CMAKE_AR gcc-ar)
>> +-        set(CMAKE_RANLIB gcc-ranlib)
>> ++        find_program(LTO_AR NAMES "${CMAKE_C_COMPILER}-ar" gcc-ar)
>> ++        find_program(LTO_RANLIB NAMES "${CMAKE_C_COMPILER}-ranlib"
>> /usr/bin/gcc-ranlib)
>> ++        if (LTO_AR AND LTO_RANLIB)
>> ++            set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto")
>> ++            set(CMAKE_AR "${LTO_AR}")
>> ++            set(CMAKE_RANLIB "${LTO_RANLIB}")
> 
> These definitions belong to the toolchainfile.cmake file IMHO, but it can be moved there in a follow-up patch.

 Well, even if it is in toolchainfile.cmake, this patch is needed because the
CMakeLists.txt will just blindly override the defaults.

 But at the moment, we don't have global LTO support. We can build an internal
toolchain with LTO support, but it's up to individual packages to enable/use it.
Which makes sense, because LTO is still a bit experimental and possibly breaks
things.

 So we could add a global LTO_AR and LTO_RANLIB to our toolchainfile.cmake, but
since no package would make use of it, it wouldn't make a whole lot of sense.

 Regards,
 Arnout


> 
> Regards,
> 
>> ++        else()
>> ++            message(STATUS "gcc-ar or gcc-ranlib not found, disabling
>> LTO")
>> ++        endif()
>> +     endif()
>> + endif()
>> + 
>> +-- 
>> +2.9.3
>> +
>
diff mbox

Patch

diff --git a/package/kmsxx/0001-Fix-LTO-support-for-cross-compilation.patch b/package/kmsxx/0001-Fix-LTO-support-for-cross-compilation.patch
new file mode 100644
index 0000000..9b4a582
--- /dev/null
+++ b/package/kmsxx/0001-Fix-LTO-support-for-cross-compilation.patch
@@ -0,0 +1,45 @@ 
+From 5da1f631bc753655ac94b08a6233eecd0d451327 Mon Sep 17 00:00:00 2001
+From: "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be>
+Date: Fri, 26 Aug 2016 21:55:06 +0200
+Subject: [PATCH] Fix LTO support for cross-compilation.
+
+When cross-compiling, the ar and ranlib to be used for LTO are prefixed
+by the cross-tuple. gcc-ar and gcc-ranlib may not exist. Cfr.
+http://autobuild.buildroot.net/results/f3c/f3c48da3a9706cd366c0e0a96c3cd0ff959f2a78/
+
+Therefore, search for an appropriate lto-ar and lto-ranlib before
+enabling LTO.
+
+Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
+---
+Submitted upstream: https://github.com/tomba/kmsxx/pull/14
+---
+ CMakeLists.txt | 12 +++++++++---
+ 1 file changed, 9 insertions(+), 3 deletions(-)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index e5b5ea5..c61c81d 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -39,9 +39,15 @@ if (NOT ${U_CMAKE_BUILD_TYPE} MATCHES DEBUG)
+     CHECK_CXX_COMPILER_FLAG("-flto" HAS_LTO_FLAG)
+ 
+     if (HAS_LTO_FLAG)
+-        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto")
+-        set(CMAKE_AR gcc-ar)
+-        set(CMAKE_RANLIB gcc-ranlib)
++        find_program(LTO_AR NAMES "${CMAKE_C_COMPILER}-ar" gcc-ar)
++        find_program(LTO_RANLIB NAMES "${CMAKE_C_COMPILER}-ranlib" /usr/bin/gcc-ranlib)
++        if (LTO_AR AND LTO_RANLIB)
++            set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -flto")
++            set(CMAKE_AR "${LTO_AR}")
++            set(CMAKE_RANLIB "${LTO_RANLIB}")
++        else()
++            message(STATUS "gcc-ar or gcc-ranlib not found, disabling LTO")
++        endif()
+     endif()
+ endif()
+ 
+-- 
+2.9.3
+