[PATCH] Fix for PR15086

Nick Lewycky nlewycky at google.com
Tue Nov 19 14:16:06 PST 2013


On 19 November 2013 12:24, Dimitry Andric <dimitry at andric.com> wrote:

> On 13 Nov 2013, at 16:31, Rafael EspĂ­ndola <rafael.espindola at gmail.com>
> wrote:
> > I *think* this is a fine improvement, but please add
> >
> > * A comment saying why jmp foo at PLT is not valid or when it would be
> valid.
>
> Rafael, did you mean a comment in X86ISelLowering.cpp?  I think a jmp
> foo at PLT is only valid if the stack is setup so the callee can correctly
> pop it, if needed, and the return types are exactly the same?
>
>
> > * A note in tailcallpic2.ll saying that it could be optimized to a
> > direct call (no got, no plt). We don't implement ELF's interposition
> > logic, but then pretend we do during codegen. This patch doesn't need
> > to fix that, but should note that we could do better.
>
> Ok, that is easily done.
>
>
> On 13 Nov 2013, at 23:39, Nick Lewycky <nlewycky at google.com> wrote:
>
> > Sorry, I forgot about this thread entirely. I generally don't approve
> non-trivial changes under lib/Target and lib/CodeGen, I'd rather somebody
> else with more assembly/object file/backend knowledge took a look.
>
> Nick, do you feel this is now reviewed enough?  I think this is a pretty
> low-risk change in itself, it only disables an optimization that might lead
> to trouble, and only on x86/GOT-style platforms.  Improving some edge cases
> (like the direct call Rafael mentioned) can easily be done later, but I
> would really like to get this change in for the 3.4 release. :-)
>

I don't think it needs any more review from me, I'll let Rafael (or someone
else) decide.

Sorry, but the 3.4 release has been cut. If this is really low risk then we
could argue that this is low risk enough to be cherrypicked into 3.4. That
would require a sign off from the x86 backend owner (+cc Nadav).

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131119/fc64f0c4/attachment.html>


More information about the llvm-commits mailing list