[cairo] [PATCH] Remove LTO support

[Uli Schlachter]

On 22.07.2014 07:38, Bryce Harrington wrote:
> On Mon, Jul 21, 2014 at 05:10:16PM +0200, Uli Schlachter wrote:
>> This just never worked too well and caused too many issues. I don't think anyone
>> will miss this.
>>
>> As mentioned in the below bug report, proper LTO support also requires using
>> special versions of ranlib, nm and ar which support the LTO object files.
>> Otherwise, calling the normal ranlib on an .a library breaks the list of
>> exported symbols and thus completely breaks the static library.
>>
>> This (partly) reverts the following commits:
>>
>> c3645d97ebd24c6f7ad850785d585aebc706a11c configure.ac: Add a --disable-lto configure option
>> d486ea30f1a58640a1178de74f705a73845b1cda configure: Conditionally include -flto
>> 0870c6fb5b39dcc04fa376123848adde2d06d2ce gcc-4.5 warnings and optimisation flags.
>>
>> (The last commit is the one which brought us -flto in the first place even
>> though it doesn't talk about this. It's also the one which is only reverted
>> partly.)
>>
>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=77060
>> CC: Chris Wilson <chris at chris-wilson.co.uk>
>> Signed-off-by: Uli Schlachter <psychon at znc.in>
> 
> Reviewed-by: Bryce Harrington <b.harrington at samsung.com>

Thanks.

> I've no qualms about seeing this go.  However I do wonder if there is a
> benefit to it, that we'd lose?  I gather it optimizes the build process,
> but I can't tell if there are any user-visible benefits?

The reason why I just decided to revert this is the commit that introduced it
(and this also the reason why I CC'd Chris, although I should have mentioned that):

commit 0870c6fb5b39dcc04fa376123848adde2d06d2ce
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Apr 15 20:31:24 2010 +0100

    gcc-4.5 warnings and optimisation flags.

diff --git a/build/configure.ac.warnings b/build/configure.ac.warnings
index 5b561e1..2f5745f 100644
--- a/build/configure.ac.warnings
+++ b/build/configure.ac.warnings
@@ -17,7 +17,7 @@ MAYBE_WARN="-Wall -Wextra \
 -Wbad-function-cast -Wvolatile-register-var \
 -Wstrict-aliasing=2 -Winit-self -Wunsafe-loop-optimizations \
 -Wno-missing-field-initializers -Wno-unused-parameter \
--Wno-attributes -Wno-long-long -Winline"
+-Wno-attributes -Wno-long-long -Winline -Wlogical-op"

 dnl Sun Studio 12 likes to rag at us for abusing enums like
 dnl having cairo_status_t variables hold cairo_int_status_t
@@ -27,7 +27,7 @@ MAYBE_WARN="$MAYBE_WARN -erroff=E_ENUM_TYPE_MISMATCH_ARG \

 dnl We also abuse the warning-flag facility to enable other compiler
 dnl options.  Namely, the following:
-MAYBE_WARN="$MAYBE_WARN -fno-strict-aliasing -fno-common"
+MAYBE_WARN="$MAYBE_WARN -fno-strict-aliasing -fno-common -flto"

 dnl Also to turn various gcc/glibc-specific preprocessor checks
 MAYBE_WARN="$MAYBE_WARN -Wp,-D_FORTIFY_SOURCE=2"


This doesn't seem all that well-thought to me and doesn't explain any reason for
this. (And the second commit that did the conditional include is just broken.
AC_TRY_LINK does not use $MAYBE_WARN and thus that test didn't test anything at
all.) Also, the current linking issues that we have due to LTO make me wonder if
GCC was actually doing LTO or not. I'm not sure. Oh and apparently no one ever
investigated if this makes a difference.

With such a track record of success, getting rid of this seemed like the easiest
solution. I'd like to give Chris some more time to say "the commit message
doesn't say so, but I had good reasons for adding this and it makes a huge
difference for something" and then this can be committed.

Cheers,
Uli
-- 
- Captain, I think I should tell you I've never
  actually landed a starship before.
- That's all right, Lieutenant, neither have I.