[PATCH] Fix for PR15086

[Bill Wendling]

Okay. Go ahead and commit it to ToT and I can pull it in.

-bw

On Nov 19, 2013, at 3:09 PM, Nadav Rotem <nrotem at apple.com> wrote:

> This patch was reviewed by people who know this part of the code, it solves an important bug, and it is a local fix.  Let’s push it to 3.4. 
> 
> 
> On Nov 19, 2013, at 2:23 PM, Dimitry Andric <dimitry at andric.com> wrote:
> 
>> On 19 Nov 2013, at 23:16, Nick Lewycky <nlewycky at google.com> wrote:
>>> 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).
>> 
>> We just branched, it is not set in stone yet, right? :-)  This is simply a bug fix that could be added during Phase I testing.  If Nadav can have a quick look at it, that would be great.  (Attached the fix rebased against r195016, with one of Rafael's additions to test/CodeGen/X86/tailcallpic2.ll)
>> 
>> -Dimitry
>> <fix-pr15086-3.diff>
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

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