[Mesa-dev] [PATCH] mesa: Enable LTO compilation

[Aaron Watry]

On Mon, May 30, 2016 at 3:31 PM, Emil Velikov <emil.l.velikov at gmail.com>
wrote:

> On 30 May 2016 at 21:10, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>> On Mon, May 30, 2016 at 12:27 PM, Emil Velikov <emil.l.velikov at gmail.com>
>> wrote:
>>
>>> Hi there,
>>> On 30 May 2016 at 18:28, ⚛ <0xe2.0x9a.0x9b at gmail.com> wrote:
>>> > This patch enables compilation with -flto.
>>> >
>>> > The performance benefits of LTO (GCC 4.9 & 6.1) are about 1% for
>>> glxgears.
>>> > Performance changes in OpenGL games haven't been measured yet, my
>>> feeling is
>>> > that they are negligible.
>>> >
>>> Please try to send patches via git send-email for the future.
>>>
>>> While not a hard requirement, it would be nice if you setup your email
>>> client/git to show your name/nickname.
>>> It's always better to address someone directly, as opposed to "hey you".
>>>
>>> About the patch itself:
>>>  - Please add a summary message for the change. Peter's has a nice
>>> article on the topic [1].
>>>  - We normally try to include performance metrics in the commit
>>> message. It makes it easier to justify said change.
>>>  - Have you tried debugging the resulting binary ? As Matt mentioned
>>> there used to be issues with LTO, so if those are still unresolved we
>>> want to _avoid_ forcing that for everyone.
>>>
>>
>> Regarding the above two comments:  The patch he attached does not enable
>> LTO, it just disables it for mapi so that *if* you put -flto in your
>> CFLAGS, it doesn't blow up.  If this patch were enabling LTO by default,
>> questions about performance and debugability would be reasonable.  However,
>> given that the patch is purely protective, the only real question required
>> to justify the patch is "is LTO actually a problem for mapi?"
>>
>>
> Seems like I'm the next person to fall trap of the commit message...
> that's embarrassing.
>
> Wrt the approach used... perhaps one should be checking for the same flag
> like they use ? It should still work though - just good practise.
>
>
>>  - Are you use the mapi hunk is needed ? last time I've tried (some
>>> months ago, copying the tweaks from the Clearlinux spec file [2])
>>> things built without issues.
>>>
>>
>> You are claiming it isn't.
>>
>> That said, I tried to build with -flto recently and it blew up.  I don't
>> remember exactly why.
>>
> Do you (others) recall any details about how you enabled it (tweak *FLAGS,
> CC and/or other), if there was anything special in the configure options
> and/or which the GCC version ?
>
> I'll give it another try in a minute, just in case I butchered something.
>

My Build (for one machine):

CXXFLAGS="-flto=8 -ffat-lto-objects -flto-odr-type-merging" CFLAGS="-flto=8
-ffat-lto-objects -flto-odr-type-merging" LD=ld LDFLAGS=" -flto=8" CC=gcc
CXX=g++ /home/awatry/src/mesa/autogen.sh  --with-dri-drivers=i965
--with-dri-driverdir=/usr/local/lib/dri/ --enable-debug --enable-glx-tls
--enable-gles1 --enable-gles2 --enable-texture-float --with-gallium-drivers=

With the mapi/LTO patch, build succeeds. Without it, I get the following:

  CCLD     glapi/libglapi.la
ar: `u' modifier ignored since `D' is the default (see `U')
  CCLD     shared-glapi/libglapi.la
/tmp/ccLynnFb.ltrans7.ltrans.o: In function `entry_get_public':
/home/awatry/src/mesa/src/mapi/entry_x86-64_tls.h:63: undefined reference
to `x86_64_entry_start'
/usr/bin/ld: /tmp/ccLynnFb.ltrans7.ltrans.o: relocation R_X86_64_PC32
against undefined symbol `x86_64_entry_start' can not be used when making a
shared object; recompile with -fPIC
/usr/bin/ld: final link failed: Bad value


Given the header that it's failing in, I removed the --enable-glx-tls flag,
and then things built successfully.

--Aaron


>
> -Emil
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160531/a2ea7282/attachment.html>