[PATCH] D67855: [X86] Add new calling convention that guarantees tail call optimization

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

I'd just like to note that this code is not well tested. A number of features and platforms have been added to the x86 prologue codepath that have not been tested in combination with guaranteed TCO, such as Win64 support. I've always assumed that users of this feature just like to live life dangerously.

Aside from that, the code looks correct, and I think this is a step towards simplifying things. As a long term goal, I think we should get rid of the `GuaranteedTailCallOpt` option and just ask people to use this convention. These are the conventions that work with this feature today:

- fastcc, replaced by tailcc
- GHC, we can probably make this always be callee cleanup and guarantee TCO for it in the same way
- HiPE, probably the same
- HHVM, unclear, I hope it's the same
- X86_RegCall, I'm not sure if it was intended for us to change the ABI of this convention when users pass `-tailcallopt` to llc. It seems like that could be a bug.



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:3694-3695
   if (Subtarget.isPICStyleGOT() &&
-      !MF.getTarget().Options.GuaranteedTailCallOpt) {
+      !MF.getTarget().Options.GuaranteedTailCallOpt &&
+      CallConv != CallingConv::Tail) {
     // If we are using a GOT, disable tail calls to external symbols with
----------------
There are four cases this handles:
- regular calls
- safe, friendly, good, ABI-preserving, opportunistic tail calls, called "sibcalls" in this code
- musttail calls, which, because of the IR verifier checks, we know will not require moving the return address
- ABI changing, return-address-moving, guaranteed tail calls

We have named booleans for case 2 and 3, but this code would benefit from a bool for case 4. I think you can call `shouldGuaranteeTCO` earlier and set up a bool with a name like `GuaranteeTCO` and check it in the rest of the cases throughout `LowerCall`.


================
Comment at: llvm/test/CodeGen/X86/tailcall-tailcc.ll:1
+; RUN: llc < %s -mtriple=i686-- | grep TAILCALL | count 7
+
----------------
lebedev.ri wrote:
> Please use FileCheck + update_llc_test_checks.py
+1


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67855/new/

https://reviews.llvm.org/D67855