<div dir="ltr">Hi Tim,<div><br></div><div>Sorry, it's holiday today in UK...</div><div><br></div><div style>So, I think the patch makes sense, but I have some comments.</div><div style><br></div><div style>First, you're re-defining the place-holder many times. I take it it's not used on all cases, but often enough to maybe be commoned up and hope the compiler does a good job?</div>
<div style><br></div><div style>Also, you're using the PRIVATE-0, which has no use AFAICT, but also no comment on it about the expected behaviour on ELF.h. I'm not sure what's the policy on enum reserved entries, but one way is to rename it to something meaningful, another is to add a one-line comment on ELF.h about its usage in the EE.</div>
<div style><br></div><div style>Apart from that, I think it looks good. But I'm no expert in the EE.</div><div style><br></div><div style>cheers,</div><div style>--renato</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On 27 May 2013 08:50, Tim Northover <span dir="ltr"><<a href="mailto:t.p.northover@gmail.com" target="_blank">t.p.northover@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Ping?<br>
<div class="HOEnZb"><div class="h5"><br>
On Sat, May 25, 2013 at 9:16 AM, Tim Northover <<a href="mailto:t.p.northover@gmail.com">t.p.northover@gmail.com</a>> wrote:<br>
> Hi,<br>
><br>
> This patch is designed to fix PR16013, which notes that ARM's MCJIT<br>
> relocation processing can't be reliably applied twice since each<br>
> iteration changes the state used by the next. It should allow remote<br>
> MCJIT to work reliably on ARM ELF.<br>
><br>
> The bulk of the patch brings ARM in line with other targets (like<br>
> i386) in taking the .rel addends from the object-file as emitted<br>
> rather than the one being modified.<br>
><br>
> However, there is an extra complication which I think deserves review.<br>
> The stubs created by the JIT need relocation, but don't actually have<br>
> any addend bits to take from the emitted object-file. This means that<br>
> none of the generic handling can work for them.<br>
><br>
> There are two potential solutions to this:<br>
> 1. Use the R_ARM_PRIVATE_0 special relocation completely internally<br>
> within RuntimeDyldELF (see patch).<br>
> 2. Teach generic RuntimeDyld about the difference between .rel and<br>
> .rela relocations, so that it can handle object files emitted using<br>
> both styles. Then the stubs would become a .rela style R_ARM_ABS32<br>
> with addend 0.<br>
><br>
> I decided that the second option was rather heavyweight since I can't<br>
> see LLVM ever actually emitting both .rel and .rela sections.<br>
><br>
> OK to commit?<br>
><br>
> Tim.<br>
</div></div><br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>