Backout clang 244207 - 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]

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> 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
> 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/6c012076/attachment.html>