[PATCH] D48247: lld: add experimental support for SHT_RELR sections.

[Rui Ueyama via llvm-commits]

I'll land this patch for you.

On Fri, Jul 6, 2018 at 12:35 PM Rahul Chaudhry <rahulchaudhry at chromium.org>
wrote:

> On Fri, Jul 6, 2018 at 11:22 AM Rui Ueyama <ruiu at google.com> wrote:
>
>> On Fri, Jul 6, 2018 at 10:45 AM Rahul Chaudhry via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>>> rahulchaudhry added inline comments.
>>>
>>>
>>> ================
>>> Comment at: ELF/Relocations.cpp:717-723
>>> +                             RelType Type) {
>>> +  // Add a relative relocation. If RelrDyn section is enabled, and the
>>> +  // relocation offset is guaranteed to be even, add the relocation to
>>> +  // the RelrDyn section, otherwise add it to the RelaDyn section.
>>> +  // RelrDyn sections don't support odd offsets. Also, RelrDyn sections
>>> +  // don't store the addend values, so we must write it to the relocated
>>> +  // address.
>>> ----------------
>>> ruiu wrote:
>>> > Well, this comment explains what you are doing in this function, but
>>> that's what I can find by reading this function, so I don't find this very
>>> helpful. I'd explain the intention instead. For example, this comment
>>> should answer to the question why the offset must be aligned to 2, what is
>>> RelrDyn in the first place, etc.
>>> >
>>> > Now that I understood the entire patch, I'll rewrite this patch after
>>> you commit this change so that we can proceed, though.
>>> I cannot commit this change as I do not have write access.
>>> I won't ask you to commit it either, since it is clear that you are not
>>> happy with this change.
>>> You've indicated a couple of times that you would like to rewrite this
>>> patch.
>>> I would suggest that you re-implement the functionality to your liking,
>>> and simply commit that instead.
>>> Unfortunately, I will not be able to spend any more time on this.
>>> Thanks for your time,
>>> Rahul
>>>
>>
>> My apologies. I meant I would update this "comment" and not the "patch".
>> Rewriting the entire patch doesn't make sense at all, that's just a silly
>> typo, and my intention is to save your time. My typo unfortunately made my
>> comment arrogant. I'm sorry, I didn't mean it.
>>
>
> Okay.. that's reasonable.
> Let's move past the misunderstanding and land it then?
>
> I would like to add here that it was a pleasure working on this patch for
> LLD. I did not expect the ELF linker code to be remotely as approachable as
> it was.
> And the speed is amazing. I saw it link chromeos-chrome in 13 seconds!
> (and that was a debug build of LLD linking a debug build of chrome!!)
> All this clearly demonstrates great design, as well as good taste when it
> comes to implementation details.
> Thanks, and keep up the good work,
>

Thank you for your kind words! This patch should improve lld even more,
especially for reduction in output file size, and I expect resulting
executables should load faster. Thank you for the improvement!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180706/3239ac46/attachment.html>