revert clang r244207 - Mark calls in thunk functions as tail-call optimization candidates? { Re: [PATCH] D21948: [DSE] fix - missing store to runtime stack in thunk with tail call bvval arg }

[Gerolf Hoflehner via llvm-commits]

> On Jul 7, 2016, at 8:36 PM, Gerolf Hoflehner <ghoflehner at apple.com> wrote:
> 
> Ahmed pointed out the clang commit causing the issue is r244207 - Mark calls in thunk functions as tail-call optimization candidates
> 
> The purpose of the fix was to prevent thunks from showing up on the call stack.
> 
> It seems that this patch overlooked the implications of tail calls with byval parameters and should be backed out. Any objections?
> 
> Thanks
> Gerolf
> 
> 
>> On Jul 5, 2016, at 4:50 PM, Gerolf Hoflehner via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>> 
>> Sounds reasonable. I’m curious what others think. For a thunk a tail call makes sense, but a byval in tail call needs at least a clarification in the IR specification. Similar, when a byval is not allowed in a tail call. That should be checked by the IR verifier then, too.
>> 
>> Thanks
>> Gerolf
>> 
>>> On Jul 2, 2016, at 7:16 PM, Eli Friedman <eli.friedman at gmail.com <mailto:eli.friedman at gmail.com>> wrote:
>>> 
>>> On Sat, Jul 2, 2016 at 6:35 PM, Gerolf Hoflehner <ghoflehner at apple.com <mailto:ghoflehner at apple.com>> wrote:
>>> 
>>> 
>>> Sent from my iPhone
>>> 
>>> > On Jul 2, 2016, at 5:15 PM, Eli Friedman <eli.friedman at gmail.com <mailto:eli.friedman at gmail.com>> wrote:
>>> >
>>> > eli.friedman added a subscriber: eli.friedman.
>>> > eli.friedman added a comment.
>>> >
>>> > Special-casing this in DSE makes no sense... alias analysis should always return the correct result.
>>> >
>>> > In this case, alias analysis is correct; the definition of "tail" doesn't allow marking this call.
>>> 
>>> Why?
>>> 
>>> 
>>> It could work in theory, but in practice it isn't documented in LangRef and isn't actually supported by the current implementation; tailcallelim doesn't mark calls with byval arguments as "tail", optimization passes aren't expecting it, and targets aren't expecting it.  I mean, it could be changed, but it's not just a simple bugfix.
>>> 
>>> -Eli
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 

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