[llvm] r335508 - Re-land r335297 "[X86] Implement more of x86-64 large and medium PIC code models"

[Reid Kleckner via llvm-commits]

I made it so my large+PIC changes are all ELF-specific, and relanded this
as r337740.

We might still want to make LLVM responsible for picking reasonable default
relocation and code models for JITs at some point just to save users the
trouble of working it out for themselves.

On Fri, Jul 20, 2018 at 2:49 PM Lang Hames <lhames at gmail.com> wrote:

> Hi Reid,
>
> Well, before my change, large+pic wouldn't actually generate PIC code, the
>> code would contain hard-coded 64-bit immediate addresses. If we want to
>> store to a global, we'd generate code like:
>>
>
>
>   movabsq $global, %rax
>>   movl $42, (%rax)
>> So, we were really generating large+static code. Large+PIC basically
>> wasn't implemented.
>
>
> For MachO, Large + PIC wasn't documented, so we got to define it however
> we wanted for the JIT. It's "PIC" in the sense that all those references
> had symbolic relocations attached, so we could re-load the object at a new
> address if we wanted to. The aim was to support caching of object files
> produced by the JIT.
>
> When I landed my change the first time, the Mac bots complained that
>> RuntimeDyldMachO didn't implement some relocations. I had to implement
>> relocations in RuntimeDyldELF to make this configuration work, but I didn't
>> do anything for MachO. It still needs to be implemented.
>>
>
> Yep. This seems like a good opportunity to re-think the MachO large code
> model. After I am done with the builder failures from r337608 I will
> re-apply your patch and try to get a sense of how much work would needed.
>
>
>> Why does LLDB want MCJIT to generate PIC code? Does it plan to relocate
>> it? Would it be possible to remove the call to
>> EngineBuilder::setRelocationModel and let LLVM decide on the correct models
>> for JITing based on the object file format?
>
>
> Right now the unwritten advice for setRelocationModel is "don't touch it
> unless you really know what you're doing, it will default to something
> sane". I'll add an explicit comment to that effect, but I'd like to keep
> the API: I plan to implement support for other MachO code models in the JIT
> in the not too distant future.
>
> Cheers,
> Lang.
>
> On Fri, Jul 20, 2018 at 11:36 AM Reid Kleckner <rnk at google.com> wrote:
>
>> On Thu, Jun 28, 2018 at 5:36 PM Lang Hames <lhames at gmail.com> wrote:
>>
>>> Looks like this got re-reverted as it is breaking LLDB too. I would like
>>> to understand it better since we typically rely on Large/PIC for the code
>>> model in the JIT, and it has worked historically.
>>>
>>
>> Well, before my change, large+pic wouldn't actually generate PIC code,
>> the code would contain hard-coded 64-bit immediate addresses. If we want to
>> store to a global, we'd generate code like:
>>   movabsq $global, %rax
>>   movl $42, (%rax)
>>
>> So, we were really generating large+static code. Large+PIC basically
>> wasn't implemented.
>>
>>
>>> I restricted the MCJIT/eh-lg-pic.ll test to Linux, since the large PIC
>>>> code model is not implemented for MachO yet.
>>>
>>>
>>> What is the source for that claim? I thought the behavior of a
>>> relocation model/code-model pair was up to the platform to decide (though
>>> in practice if it's not well defined on MachO yet we may want to match
>>> Linux's behavior).
>>>
>>
>> When I landed my change the first time, the Mac bots complained that
>> RuntimeDyldMachO didn't implement some relocations. I had to implement
>> relocations in RuntimeDyldELF to make this configuration work, but I didn't
>> do anything for MachO. It still needs to be implemented.
>>
>> Why does LLDB want MCJIT to generate PIC code? Does it plan to relocate
>> it? Would it be possible to remove the call to
>> EngineBuilder::setRelocationModel and let LLVM decide on the correct models
>> for JITing based on the object file format?
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180723/09ac39d8/attachment.html>