Page MenuHomePhabricator

Add 'musttail' marker to call instructions
AbandonedPublic

Authored by rnk on Mar 31 2014, 6:30 PM.

Details

Reviewers
nicholas
nlewycky
Summary

This is similar to the 'tail' marker, except that it guarantees that
tail call optimization will occur. It also comes with convervative IR
verification rules that ensure that tail call optimization is possible.

Diff Detail

Event Timeline

majnemer added inline comments.Mar 31 2014, 8:20 PM
include/llvm/IR/Instructions.h
1293–1294

Wouldn't it make more sense to have setTailCall take an enum TailCallKind instead of having both setTailCall and setTailCallKind ?

For the LangRef, I suggest we remove the text related to when a "tail"
call is guaranteed. With the addition of "musttail" these semantics are
redundant. Instead, "tail" should be advisory only. This would involve
breaking backwards compatibility.

With the new option, should GauranteedTailCallOpt exist? I suggest not.

In code, it would be useful to refer to the old "tail" attribute as "may
tail". This clarifies the semantics in code even if we don't change the
IR name. In particular, I'm referring to CallSite/Instruction here.

In the verifier you need to handle chains of bitcasts. These will
reduce to a single bitcast, but InstCombine may not have run yet. We
don't want each opto pass to need to do this simplification.

Otherwise, LGTM and I support the addition.

Philip

  • {F53037, layout=link}
rnk updated this revision to Unknown Object (????).Apr 15 2014, 3:19 PM
  • Make the musttail property of a call immutable
  • Add harder LangRef guarantees about arg forwarding and stack growth
rnk added a comment.Apr 15 2014, 3:23 PM

I ended up leaving the docs on tail calls in. Removing that support is tricky, because I don't want musttail to change the calling convention of the call. As implemented, -tailcallopt changes the ABI of fastcc, GHC, and HiPE. I checked with GHC and HiPE, and they either never pass args in memory, or always use callee-pop.

But IMO we need to split fastcc into two ccs: one for guaranteed TCO and one for fast codegen. That feels like a big change that I should split out.

rnk updated this revision to Unknown Object (????).Apr 17 2014, 5:50 PM
  • Make musttail mutable again, since the inliner has to tweak it.
rnk updated this revision to Unknown Object (????).Apr 18 2014, 11:30 AM
  • Remove wording about the inliner and variadic arguments, since we don't have Ellipsis support yet
rnk edited reviewers, added: nlewycky; removed: chandlerc.Apr 22 2014, 4:08 PM
rnk abandoned this revision.Aug 15 2014, 1:36 PM

This landed some how over time.