<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 9, 2018 at 5:47 PM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> writes:<br>
<br>
> On Tue, Jan 9, 2018 at 4:52 PM, Rafael Avila de Espindola <<br>
> <a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br>
><br>
>> Rui Ueyama via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> writes:<br>
>><br>
>> > ruiu added a comment.<br>
>> ><br>
>> > Isn't it an ABI violation not to write the PLT header for PPC64? If so,<br>
>> we fix that ABI violation rather than "correctly" emitting ABI-violating<br>
>> executables.<br>
>><br>
>> The PPC64 implementation is incomplete. Finishing it would be nice, but<br>
>> independent of this change.<br>
>><br>
>> Without this change a target must set Target->PltHeaderSize to avoid<br>
>> having lld confuse the plt and iplt sections, which is not a good design<br>
>> IMHO.<br>
>><br>
><br>
> It is okay to be incomplete, but I'm not very fond of making a change that<br>
> is not needed for the proper ABI. How about setting a non-zero value to the<br>
> PPC64 PLT header? We don't have to fill the actual instruction sequence.<br>
> Setting a non-zero value should fix the issue, right?<br>
<br>
</span>I think the way to look at this patch is a cleanup, not a fix.<br>
<br>
I find<br>
<br>
  if (IsIplt) {<br>
<br>
A lot easier to read than<br>
<br>
  if (HeaderSize == 0) {<br>
<br>
I does happen to fix a problem with the ppc64 target, but that is almost<br>
incidental. And yes, it could be made a pure cleanup by setting the<br>
ppc64 PltHeaderSize to a nonzero value first.<br></blockquote><div><br></div><div>Sure. But if PLTs for all targets have the PLT header for lazy symbol resolution (I believe that's the case), and IPLT doesn't have one (I believe that's true too), then we can use `bool is Iplt() { return HeaderSize == 0; }` instead of adding a new boolean flag to the ctor.</div></div></div></div>