<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, May 30, 2016 at 3:31 PM, Emil Velikov <span dir="ltr"><<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On 30 May 2016 at 21:10, Jason Ekstrand <span dir="ltr"><<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Mon, May 30, 2016 at 12:27 PM, Emil Velikov <span dir="ltr"><<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi there,<br>
<div><div>On 30 May 2016 at 18:28, ⚛ <<a href="mailto:0xe2.0x9a.0x9b@gmail.com" target="_blank">0xe2.0x9a.0x9b@gmail.com</a>> wrote:<br>
> This patch enables compilation with -flto.<br>
><br>
> The performance benefits of LTO (GCC 4.9 & 6.1) are about 1% for glxgears.<br>
> Performance changes in OpenGL games haven't been measured yet, my feeling is<br>
> that they are negligible.<br>
><br>
</div></div>Please try to send patches via git send-email for the future.<br>
<br>
While not a hard requirement, it would be nice if you setup your email<br>
client/git to show your name/nickname.<br>
It's always better to address someone directly, as opposed to "hey you".<br>
<br>
About the patch itself:<br>
 - Please add a summary message for the change. Peter's has a nice<br>
article on the topic [1].<br>
 - We normally try to include performance metrics in the commit<br>
message. It makes it easier to justify said change.<br>
 - Have you tried debugging the resulting binary ? As Matt mentioned<br>
there used to be issues with LTO, so if those are still unresolved we<br>
want to _avoid_ forcing that for everyone.<br></blockquote><div><br></div></span><div>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?"<br></div><span><div> </div></span></div></div></div></blockquote></span><div>Seems like I'm the next person to fall trap of the commit message... that's embarrassing.</div><div><br></div><div>Wrt the approach used... perhaps one should be checking for the same flag like they use ? It should still work though - just good practise. </div><span class=""><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
 - Are you use the mapi hunk is needed ? last time I've tried (some<br>
months ago, copying the tweaks from the Clearlinux spec file [2])<br>
things built without issues.<br></blockquote></span><div><br>You are claiming it isn't.<br><br></div><div>That said, I tried to build with -flto recently and it blew up.  I don't remember exactly why.<br></div><div></div></div></div></div></blockquote></span><div>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 ?</div><div><br></div><div>I'll give it another try in a minute, just in case I butchered something.</div></div></div></div></blockquote><div><br></div><div>My Build (for one machine):<br><br></div><div>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=<br></div><div><br></div><div>With the mapi/LTO patch, build succeeds. Without it, I get the following:<br><br>  CCLD     glapi/<a href="http://libglapi.la">libglapi.la</a><br>ar: `u' modifier ignored since `D' is the default (see `U')<br>  CCLD     shared-glapi/<a href="http://libglapi.la">libglapi.la</a><br>/tmp/ccLynnFb.ltrans7.ltrans.o: In function `entry_get_public':<br>/home/awatry/src/mesa/src/mapi/entry_x86-64_tls.h:63: undefined reference to `x86_64_entry_start'<br>/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<br>/usr/bin/ld: final link failed: Bad value<br><br></div><div><br></div><div>Given the header that it's failing in, I removed the --enable-glx-tls flag, and then things built successfully.<br><br></div><div>--Aaron<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><font color="#888888"><div><br></div><div>-Emil</div></font></span></div></div></div>
<br>_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
<br></blockquote></div><br></div></div>