<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 21, 2016 at 10:13 AM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><span class="gmail-">On 21 June 2016 at 01:39, James Y Knight <<a href="mailto:jyknight@google.com">jyknight@google.com</a>> wrote:<br>
> I disagree. I don't think this makes sense. Either the linker you're using<br>
> supports this feature, or it doesn't. Having it enabled for llc if your<br>
> linker doesn't support it is not fun.<br>
><br>
> Further note that this also affects basically all other code using llvm<br>
> libraries -- other than Clang, which explicitly sets it back to false by<br>
> default, unless you set the ENABLE_X86_RELAX_RELOCATIONS cmake flag to true.<br>
><br>
> If you want to enable the relax mode across all llvm tools in some<br>
> circumstances, I think it should be via moving the cmake flag from clang<br>
> down into llvm.<br>
<br>
</span>But the correct answer is tool dependent.<br>
<br>
A nice consequence of this patch is that it found two different cases:<br>
<br>
* The gold plugin needs to figure out a way to check the gold version.<br>
For now it just uses the old relocs.<br></blockquote><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
* The LTO support in lld always wants the new relocs since it is part<br>
of lld and knows lld supports them</blockquote><div><br></div><div><br class="gmail-Apple-interchange-newline">Both of these seem like the same case: instead of using the default (compile-time cmake flag), it has additional knowledge -- it knows the EXACT linker that will be used to link the generate objects. So it's safe for it to enable the feature at runtime when it knows for sure the linker supports it, even when it was not enabled by default. That makes fine sense.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><span class="gmail-">
> I'm going to revert this commit, since I both think it intrinsically doesn't<br>
> make sense to do this, and because it's breaking some of our tools.<br>
<br>
</span>OK. I would be ok with a cmake flag for "assume an unknown user<br>
supports these", but I don't think the cmake option should be used in<br>
TargetOptions.cpp. It should be used in tools where it makes sense. In<br>
tree right now that would be just tools/gold and clang.<br></blockquote><div><br></div><div>Why do you think it doesn't make sense to have a global default that all tools use? I'm not arguing that tools can't override it if they have additional knowledge, like LTO plugins would have, but it seems to me that all MC invocations without such knowledge should default to not using the new relocations...and that it should be possible to change that, if you happen to know you're on a system with a new linker.</div><div><br></div></div></div></div>