[PATCH] D11612: [lld][ELF2] Apply relocations.

[Michael Spencer via llvm-commits]

On Wed, Aug 26, 2015 at 2:38 PM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> Rebased to trunk with all tests passing.
>
> Main issues I see so far:
>
> * There seems to be more code than needed for the test.

I'm sure we can play code golf and shrink this, but this is a pretty
simple (148 lines changed) patch. I'm not seeing any dead code added
here. I'd like to avoid spending a bunch of time "simplifying" code
that I'm going to immediately change back with the next patch.

> * Treating local symbols like regular symbols is incredibly wasteful.
> They don't need the replacement logic.

I agree, however we don't yet have a generic relocation processing
API. The plan was to do this for now, and design a proper API when we
know what we need and can share with the jit. This could be avoided
now by checking in the relocation processing code if it's a local
symbol and handling it differently, but that unnecessarily complicates
code that's in the middle of development.

- Michael Spencer

>
> I will try to further reduce the patch tomorrow.
>
>
> On 26 August 2015 at 16:49, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>> Hacked to pass all tests.
>>
>> Trying to merge now.
>>
>> On 26 August 2015 at 16:35, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>> elf2/basic64be.s is still failing, looking.
>>>
>>> On 26 August 2015 at 16:26, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>>> The attached patch fixes the build and runs git-clang-format.
>>>>
>>>> On 26 August 2015 at 16:23, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>>>> This doesn't even compile:
>>>>>
>>>>> /home/espindola/llvm/llvm/tools/lld/ELF/Symbols.h:135:12: error: use
>>>>> of undeclared identifier 'Sym'
>>>>>     return Sym.st_value;
>>>>>
>>>>> On 26 August 2015 at 02:09, Rui Ueyama <ruiu at google.com> wrote:
>>>>>> ruiu added a comment.
>>>>>>
>>>>>> Rafael should be the person who signs off.
>>>>>>
>>>>>>
>>>>>> http://reviews.llvm.org/D11612
>>>>>>
>>>>>>
>>>>>>