[PATCH] D21948: [DSE] fix - missing store to runtime stack in thunk with tail call bvval arg

Gerolf Hoflehner via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 5 16:50:11 PDT 2016


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> 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

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


More information about the llvm-commits mailing list