[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.
> 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.
> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits