[X86] pr51000 in-register struct return tailcalling

[urnathan]

Summary

This addresses pr51000 -- a failure to tailcall an object factory function. The underlying issue is tailcalling to or from functions that return a structure.

The x86 backend -- in common with many others -- disables tailcalling when either the caller or callee return a struct (regardless of whether the struct is in registers or via additional out parameter). I suspect this is from when the middle end was less smart, and didn't handle the return object lifetime well. However, AFAICT, it is now smarter, and its tailcallable optimizations do account for that object's lifetime -- just like any other local object or pointer parameter. Plus gone are the days of ABIs that returned structs in globals! (at least I hope so). However, I'm not sure whether there are x86 ABIs that return structs via a pointer in an abn
ormal register that the middle end doesn't know about? I'm not sure how such an implementation would be representable without actually modeling at least a proxy for that register in the middle end, as otherwise it wouldn't know about liveness or escaping of the object to which that register ends up pointing?

The x86 backend only tries to tailcall fns the middle end has marked as tail calls, and then it checks for additional constraints like not being able to tail call through the PLT, or variadic fns in all cases.

So, just removing the struct restriction appears to be sufficient. Obviously this affects the existing sibcall.ll test, and I added a new C++ testcase to check that we generate the expected code in the 51000 cases, and some closely related cases (and do not tail call when the callee returns a large struct into the caller's stack frame).

I bootstrapped an x86_64-linux release compiler and observed that the stage 2 and stage 3 compilers have exactly the same size text, data and bss. Both compilers validate, of course.

pr51000 hypothesizes the issue is arch independent -- that is incorrect. Each code generator that manifests this issue has the same restriction. I've only looked at x86 for the moment.

[mizvekov]

Thank you so much for this!

Quick question, when the backend disables tail-calling, how does that integrate with the new [[clang::musttail]] attribute?
Do we correctly reject the program in that case, or are we silently dropping the tail call?

[urnathan]

Quick question, when the backend disables tail-calling, how does that integrate with the new [[clang::musttail]] attribute?
Do we correctly reject the program in that case, or are we silently dropping the tail call?

that logic looks, at best, suspicious -- but I've not altered it.
if (isTailCall && !IsMustTail) { ... check arch-specific constraints maybe clear isTailCall ... }
if (IsMustTail && !isTailCall) { .. error ..}
As you can see, when 'musttail' is in play we never check that ISA can actually do it.
I don't know the semantics of musttail though -- is it 'I, the programmer, assert this is ok to tailcall', or 'I, the programmer, expect a correct tail call here'.
If it is the former, the logic is sound, if it is the latter, it is not. But I think that's an orthogonal issue.

[mizvekov]

In D105807#2871228, @urnathan wrote:

that logic looks, at best, suspicious -- but I've not altered it.
if (isTailCall && !IsMustTail) { ... check arch-specific constraints maybe clear isTailCall ... }
if (IsMustTail && !isTailCall) { .. error ..}
As you can see, when 'musttail' is in play we never check that ISA can actually do it.
I don't know the semantics of musttail though -- is it 'I, the programmer, assert this is ok to tailcall', or 'I, the programmer, expect a correct tail call here'.
If it is the former, the logic is sound, if it is the latter, it is not. But I think that's an orthogonal issue.

It definitely means there *must* be a tail call here, otherwise we will use unbounded amount of stack :)
Ping @haberman, who worked on the DR which implemented this: https://reviews.llvm.org/D99517

I know it's orthogonal, but when I saw your patch it immediately clicked me that there is something off here.

Apologies if this is more of a LLVM musttail issue rather than clang's, and if this was already known limitation. But I feel that it is dangerous to have this attribute but then not be able to enforce it...

Thanks!

[haberman]

When I implemented [[clang::musttail]], I wasn't aware that there were cases like this, where musttail in the IR would pass IR verification, but then a tail call would fail to generate in the backend. The IR documentation for musttail doesn't warn about this case.

Since implementing the attribute, I've become aware of other examples of this problem. For example, PowerPC fails to generate tail calls in several cases, throwing an error like:

fatal error: error in backend: failed to perform tail call elimination on a call site marked musttail

It definitely means there *must* be a tail call here, otherwise we will use unbounded amount of stack :)

I agree with this, [[clang::musttail]] is supposed to provide a guarantee.

I'm not sure the right way to proceed here. Clang should issue a diagnostic and fail if a tail call is not possible. But only the backend knows authoritatively if a tail call is possible or not.

Do we need a way to query the backend? eg. expose IsEligibleForTailCallOptimization() or similar to front-ends like Clang?

[urnathan]

Do we need a way to query the backend? eg. expose IsEligibleForTailCallOptimization() or similar to front-ends like Clang?

In general that's not possible until we've done code generation. (a) the middle end is responsible for determing a bunch of 'can tail-call' pieces (like local variable liveness at the point of the call). The backend is responsible for determining other items (like 'this is a cross-SO call'. Heck, code generation might not be being done in the same process as the compiler invocation -- LTO is a thing :).

I think there are two types of tail-call failure.
a) middle-end says no -- eg, a local stack-resident variable is live across the call.
b) the back-end says no -- eg, we have a cross-SO call here (and the got-pointer is callee-save).

ETA: darn, forgot to mention, the sibcall optimization was the first optimization I ever implemented in a compiler!

[efriedma]

re: [[clang::musttail]]: if the C++-level types match, musttail should always succeed. If it doesn't, that's generally a backend bug. Note that the [[clang::musttail]] rules imply the sret markings match. (The old PowerPC ABI has complicated issues with tail calls, though...)

llvm/test/CodeGen/X86/sibcall.ll

986

Are you sure it's legal to transform t21_sret_to_non_sret?

From the x86_64 ABI document: "If the type has class MEMORY [...] On return %rax will contain the address that has been passed in by the caller in %rdi." t21_f_non_sret won't do that.

[haberman]

a) middle-end says no -- eg, a local stack-resident variable is live across the call.

What is an example of this? The musttail attribute requires that it is immediately followed by a ret, so I can't think of any way to trigger this case that would pass IR verification.

The Clang code for musttail already detects and rejects the case where a variable with a non-trivial destructor is in scope at the call site: https://github.com/llvm/llvm-project/blob/072669521456a369409cf9db30739a3fac740173/clang/test/SemaCXX/attr-musttail.cpp#L58-L62

b) the back-end says no -- eg, we have a cross-SO call here (and the got-pointer is callee-save).

This is the case I am worried about. I don't know how we detect this in the front-end, since it's arch-dependent whether cross-SO calls will work.

[efriedma]

llvm/test/CodeGen/X86/sibcall.ll

979

Oh, now I remember what happened the last time someone tried to fix this... t21_sret_to_sret_structs_mismatch() is also illegal to transform, for similar reasons to t21_sret_to_non_sret().

[urnathan]

llvm/test/CodeGen/X86/sibcall.ll

979

Ah, I think I understand now -- I was thinking this would be UB, but I can see how it might be legit. Let me walk though my understanding.

define fastcc void @t21_sret_to_sret_structs_mismatch(%struct.foo* noalias sret(%struct.foo) %agg.result, %struct.foo* noalias %a) nounwind {
we receive an sret as %agg.result and a explicit pointer in %a (to uninitialized memory? or is that too C++-specific?).

%b = call fastcc %struct.foo* @ret_struct()
this gives us a pointer to a foo in %b [aside, this seems superfluous to the xform being tested?]

tail call fastcc void @t21_f_sret2(%struct.foo* noalias sret(%struct.foo) %a, %struct.foo* noalias %b) nounwind
We call t21_f_sret2 passing it %a as its incoming sret pointer and %b as an explicit arg. It'll return %a, not our incoming sret.

I thought this would be UB, because we're not initializing our own return value -- in C++ we should be calling a constructor. but (a) we could have inlined that and (b) it could be trivial and do nothing. That was my logical error.

So, I think
a) if we're not an sret function we can tail call[*],
b) else if we're calling a non-sret function we cannot tail call,
c) else if we're passing our sret register as the callee's sret register we can tail call[*],
d) else we cannot.

• provided the remaining checks are ok

[efriedma]

llvm/test/CodeGen/X86/sibcall.ll

979

That set of rules looks right.

For the testcase, a C equivalent would be something like this:

struct A { int x[10]; };
__attribute((noinline)) struct A t21_f_sret2(struct A *__restrict x) {
  return (struct A){0};
}
struct A t21_sret_to_sret_structs_mismatch(struct A a[__restrict static 1]) {
  struct A r = {};
  *a = t21_f_sret2(0);
  return r;
}

If you run this through clang -O2, you get IR almost identical to this testcase. (The result is missing a "tail" marker, but that's just a missed optimization; we're missing a run of tailcallelim in the normal pass pipeline.)

[urnathan]

Here is an updated patch with a simpler goal. It only prevents sibcalling when the caller is an sret pointer function. To enable that to sibcall we need to trace the data flow to the callee's sret parameter, and that doesn't appear easy -- at least as a first foray into llvm's backend. So let's just have this smaller improvement, which does solve the problem the original bug report had.

[efriedma]

llvm/lib/Target/X86/X86ISelLowering.cpp

4848

I feel like I must be blind here, but this is what we used to call isCalleeStructRet, right?
I thought we wanted to check isCallerStructRet?

[urnathan]

llvm/lib/Target/X86/X86ISelLowering.cpp

4848

Not quite. callIsStructReturn's result is tri-valued. Previously we were checking != NotStructReturn. It is sufficient to check == StackStructReturn, as RegStructReturn is tailcallable. I moved the call to callIsStructReturn here, from the caller, because its the only use of that value.

[efriedma]

The test llvm/test/CodeGen/X86/sibcall.ll doesn't pass with the current version of the patch; please update.

[urnathan]

I need a vacation

[urnathan]

Ok, having had a nice long break here's an updated patch. It addresses the case of tail calling a function that returns a struct in registers. These are just like tail calling functions that return scalars.

Is it acceptable to have the FIXME comment pointing at pr51000 about the more complicated case when return is via an sret pointer parameter?

[rnk]

There's a long discussion involving musttail above that I'm going to ignore, since it doesn't seem to have anything to do with PR51000.

In D105807#2952398, @urnathan wrote:

Ok, having had a nice long break here's an updated patch. It addresses the case of tail calling a function that returns a struct in registers. These are just like tail calling functions that return scalars.

I agree, I think the code change is functionally correct: we want to power-down TCO whenever the calling function has to return something in RAX. For now, you're conservatively assuming the callee won't do that, and that's fine.

Is it acceptable to have the FIXME comment pointing at pr51000 about the more complicated case when return is via an sret pointer parameter?

I can't say these are normative conventions, but my experience is that LLVM generally avoids referencing bug numbers from comments. There are plenty of standard references and links to things like docs and object file standards, but otherwise, the idea is that the code comments should have everything you need to reason about the behavior, and the reader shouldn't have to refer to a bug to understand why the code is a certain way.

llvm/lib/Target/X86/X86ISelLowering.cpp

4848

I think the new way of writing this is clearer: the constraint we're really looking for is, do we need to return the sret parameter at the end of the function? If yes, block TCO. If not, do it.

4853

My sense is that the link isn't actually that helpful. I think the comments you wrote are reasonable straightforward. At least, I got it, but maybe I'm special. :)

[urnathan]

this removes the pr51000 comment and fixes a test failure on windows -- need to accept @PLT suffixes, hopefully got the regex right for making that optional.

Thanks for your feedback.

[rnk]

Looks like there's a correctness issue on 32-bit x86.

clang/test/CodeGenCXX/pr51000.cpp

2

This is an integration test between Clang and LLVM, which are generally discouraged. Your code change is to the x86 backend, so the LLVM CodeGen/X86 test should be sufficient. I was unable to find any documentation to point to support this discouragement. >_>

llvm/test/CodeGen/X86/sibcall.ll

661

I dug into the history, and it seems @xbolva00 added this test in a previous attempt to do more TCO around sret. The test is from rG642ed40e57faa34ee3ae7ba0e32f75ff582d408b and the attempt was D46262.

Honestly, I think you can remove the test. I assumed it had some purpose, but I don't see any motivation for this UB in the review comments.

1026–1027

Hang on, this seems wrong. The stack adjustments aren't balanced.

I think we can't do TCO for 32-bit x86. It pops off the sret pointer. See this IR:

define ccc void @t22_f_sret(i32* sret(i32) %p) {
        store i32 0, i32* %p
        ret void
}
define ccc void @t22_non_sret_to_sret(i32* %agg.result) nounwind  {
        tail call ccc void @t22_f_sret(i32* noalias sret(i32) %agg.result) nounwind
        ret void
}

->
$ llc t.ll -o - -mtriple=i686-linux-gnu

t22_f_sret:                             # @t22_f_sret
        .cfi_startproc
# %bb.0:
        movl    4(%esp), %eax
        movl    $0, (%eax)
        retl    $4
.Lfunc_end0:
...
t22_non_sret_to_sret:                   # @t22_non_sret_to_sret
# %bb.0:
        subl    $12, %esp
        movl    16(%esp), %eax
        movl    %eax, (%esp)
        calll   t22_f_sret@PLT
        addl    $8, %esp
        retl

The retl $4 instruction here is key, it adjusts ESP to pop off extra argument memory.

[urnathan]

• Removed pr51000.cpp testcase
• Prevent tailcall on i686 from non-sret to sret
• Removed UB sibcall.ll test

AFAICT by the time we get to this point of code generation, in-register struct return has been flattened to explicit tuple types. So this is mostly already handled by existing scalar/multiple-value sibcall machinery and testing.

The StructReturnType enum's RegStructReturn enumerator is never checked -- so that bit of API could be cleaned up to just be a 'is-sret-pointer' boolean. I also notice that {callIs,argsAre}StructReturn only look at the first result/parm, whereas formal parameter lowering iterates to the first one that has isSret set, and one of the sibcall tests passes a non-initial sret parameter. That seems inconsistent (more UB?)

[urnathan]

llvm/test/CodeGen/X86/sibcall.ll

1026–1027

oh, I didn't realize sret was callee pop in that case. There is later code in the tailcall checking, concerning caller and callee pop matching, but it doesn't seem prepared to deal with partial popping. Simpler just to explicitly check for i686.

[xbolva00]

llvm/test/CodeGen/X86/sibcall.ll

661

Feel free to delete it

[rnk]

lgtm

Thanks!

clang/test/CodeGenCXX/pr51000.cpp

+// RUN: %clang -S %s -o - -O2 -Xclang -triple=x86_64-linux | FileCheck %s --check-prefix=X86 --check-prefix=X86_64
+// RUN: %clang -S %s -o - -O2 -Xclang -triple=x86_64-linux-gnux32 | FileCheck %s --check-prefix=X86 --check-prefix=X86_32
+// RUN: %clang -S %s -o - -O2 -Xclang -triple=x86_64-win64 | FileCheck %s --check-prefix=X86 --check-prefix=X86_64
+// RUN: %clang -S %s -o - -O2 -Xclang -triple=x86_64-win32 | FileCheck %s --check-prefix=X86_WIN
+
+inline void *operator new(decltype(sizeof(0)), void *p) noexcept {
+  return p;
+}
+inline void operator delete(void *p, decltype(sizeof(0))) noexcept {
+}
+
+namespace One { // pr 51000
+
+struct T {
+  int x;
+  T(int)
+  noexcept;
+  ~T();
+};
+
+T factory(int) noexcept;
+
+alignas(T) char buffer[sizeof(T)];
+
+void placement_new() {
+  // tailcallable ctor
+  ::new ((void *)buffer) T(42);
+}
+// X86-LABEL: _ZN3One13placement_newEv:
+// X86: jmp _ZN3One1TC1Ei    # TAILCALL
+// X86_WIN-LABEL: "?placement_new@One@@YAXXZ":
+// X86_WIN: jmp "??0T@One@@QEAA@H@Z"    # TAILCALL
+
+void placement_call() {
+  // tailcallable factory
+  ::new ((void *)buffer) T(factory(42));
+}
+// X86-LABEL: _ZN3One14placement_callEv:
+// X86: jmp _ZN3One7factoryEi       # TAILCALL
+// X86_WIN-LABEL: "?placement_call@One@@YAXXZ":
+// X86_WIN: jmp "?factory@One@@YA?AUT@1@H@Z"    # TAILCALL
+
+} // namespace One
+
+namespace Two {
+
+struct A { // return in register
+  int m;
+};
+
+A foo();
+
+A baz() {
+  // tailcallable
+  return foo();
+}
+// X86-LABEL: _ZN3Two3bazEv:
+// X86: jmp _ZN3Two3fooEv       # TAILCALL
+// X86_WIN-LABEL: "?baz@Two@@YA?AUA@1@XZ":
+// X86_WIN: "?foo@Two@@YA?AUA@1@XZ"         # TAILCALL
+
+void bar() {
+  // tailcallable
+  foo();
+}
+// X86-LABEL: _ZN3Two3barEv:
+// X86: jmp _ZN3Two3fooEv       # TAILCALL
+// X86_WIN-LABEL: "?bar@Two@@YAXXZ":
+// X86_WIN: "?foo@Two@@YA?AUA@1@XZ"         # TAILCALL
+
+} // namespace Two
+
+namespace Three {
+
+struct A { // return via pointer
+  int m[16];
+};
+
+A foo();
+
+A baz() {
+  // Although theoretically tailcallable we're unable to trace foo's sret
+  // argument value is our incoming sret parameter value, and the ABI requires
+  // Baz to return that pointer value.  IMHO that's mis-optimization in the ABI
+  // as usually the location of the returned object is a stack slot, which is
+  // easily rematerializable.
+  // FIXME: See the bug report for more information/ideas
+  return foo();
+}
+// X86-LABEL: _ZN5Three3bazEv:
+// X86:   pushq %rbx
+// X86:   movq %rdi, %rbx
+// X86:   callq _ZN5Three3fooEv
+// X86_64:  movq %rbx, %rax
+// X86_32:  movl %ebx, %eax
+// X86:   popq %rbx
+// X86:   retq
+// X86_WIN-LABEL: "?baz@Three@@YA?AUA@1@XZ":
+// X86_WIN: pushq %rsi
+// X86_WIN: subq $32, %rsp
+// X86_WIN: movq %rcx, %rsi
+// X86_WIN: callq "?foo@Three@@YA?AUA@1@XZ"
+// X86_WIN: movq %rsi, %rax
+// X86_WIN: addq $32, %rsp
+// X86_WIN: popq %rsi
+// X86_WIN: retq
+
+void bar() {
+  // NOT tailcallable
+  foo();
+}
+// X86-LABEL: _ZN5Three3barEv:
+// X86_64:  subq  ${{[0-9]+}}, %rsp
+// X86_32:  subl  ${{[0-9]+}}, %esp
+// X86:   callq _ZN5Three3fooEv
+// X86_64:  addq  ${{[0-9]+}}, %rsp
+// X86_32:  addl  ${{[0-9]+}}, %esp
+// X86:   retq
+// X86_WIN-LABEL: "?bar@Three@@YAXXZ":
+// X86_WIN: subq  ${{[0-9]+}}, %rsp
+// X86_WIN: callq "?foo@Three@@YA?AUA@1@XZ"
+// X86_WIN: addq  ${{[0-9]+}}, %rsp
+// X86_WIN: retq
+
+} // namespace Three

llvm/lib/Target/X86/X86ISelLowering.h

private:
 
     // Call lowering helpers.
 
     /// Check whether the call is eligible for tail call optimization. Targets
     /// that want to do tail call optimization should implement this function.
     bool IsEligibleForTailCallOptimization(SDValue Callee,
                                            CallingConv::ID CalleeCC,
                                            bool isVarArg,
-                                           bool isCalleeStructRet,
-                                           bool isCallerStructRet,
                                            Type *RetTy,
                                     const SmallVectorImpl<ISD::OutputArg> &Outs,
                                     const SmallVectorImpl<SDValue> &OutVals,
                                     const SmallVectorImpl<ISD::InputArg> &Ins,
                                            SelectionDAG& DAG) const;
     SDValue EmitTailCallLoadRetAddr(SelectionDAG &DAG, SDValue &OutRetAddr,
                                     SDValue Chain, bool IsTailCall,
                                     bool Is64Bit, int FPDiff,

llvm/lib/Target/X86/X86ISelLowering.cpp

if (Subtarget.isPICStyleGOT() && !IsGuaranteeTCO && !IsMustTail) {
     if (!G || (!G->getGlobal()->hasLocalLinkage() &&
                G->getGlobal()->hasDefaultVisibility()))
       isTailCall = false;
   }
 
 
   if (isTailCall && !IsMustTail) {
     // Check if it's really possible to do a tail call.
-    isTailCall = IsEligibleForTailCallOptimization(Callee, CallConv,
-                    isVarArg, SR != NotStructReturn,
-                    MF.getFunction().hasStructRetAttr(), CLI.RetTy,
-                    Outs, OutVals, Ins, DAG);
+    isTailCall = IsEligibleForTailCallOptimization(
+        Callee, CallConv, isVarArg, CLI.RetTy, Outs, OutVals, Ins, DAG);
 
     // Sibcalls are automatically detected tailcalls which do not require
     // ABI changes.
     if (!IsGuaranteeTCO && isTailCall)
       IsSibcall = true;
 
     if (isTailCall)
       ++NumTailCalls;
bool MatchingStackOffset(SDValue Arg, unsigned Offset, ISD::ArgFlagsTy Flags,
   }
 
   return Bytes == MFI.getObjectSize(FI);
 }
 
 /// Check whether the call is eligible for tail call optimization. Targets
 /// that want to do tail call optimization should implement this function.
 bool X86TargetLowering::IsEligibleForTailCallOptimization(
-    SDValue Callee, CallingConv::ID CalleeCC, bool isVarArg,
-    bool isCalleeStructRet, bool isCallerStructRet, Type *RetTy,
+    SDValue Callee, CallingConv::ID CalleeCC, bool isVarArg, Type *RetTy,
     const SmallVectorImpl<ISD::OutputArg> &Outs,
     const SmallVectorImpl<SDValue> &OutVals,
     const SmallVectorImpl<ISD::InputArg> &Ins, SelectionDAG &DAG) const {
   if (!mayTailCallThisCC(CalleeCC))
     return false;
 
   // If -tailcallopt is specified, make fastcc functions tail-callable.
   MachineFunction &MF = DAG.getMachineFunction();
bool X86TargetLowering::IsEligibleForTailCallOptimization(
   // require ABI changes. This is what gcc calls sibcall.
 
   // Can't do sibcall if stack needs to be dynamically re-aligned. PEI needs to
   // emit a special epilogue.
   const X86RegisterInfo *RegInfo = Subtarget.getRegisterInfo();
   if (RegInfo->hasStackRealignment(MF))
     return false;
 
-  // Also avoid sibcall optimization if either caller or callee uses struct
-  // return semantics.
-  if (isCalleeStructRet || isCallerStructRet)
+  // Also avoid sibcall optimization if we're an sret return fn and the callee
+  // is incompatible. See comment in LowerReturn about why hasStructRetAttr is
+  // insufficient.
+  if (MF.getInfo<X86MachineFunctionInfo>()->getSRetReturnReg()) {
+    // For a compatible tail call the callee must return our sret pointer. So it
+    // needs to be (a) an sret function itself and (b) we pass our sret as its
+    // sret. Condition #b is not easy to determine at this point.
+    // FIXME: See pr51000 for more information.
     return false;
+  }
 
   // Do not sibcall optimize vararg calls unless all arguments are passed via
   // registers.
   LLVMContext &C = *DAG.getContext();
   if (isVarArg && !Outs.empty()) {
     // Optimizing for varargs on Win64 is unlikely to be safe without
     // additional testing.
     if (IsCalleeWin64 || IsCallerWin64)

llvm/test/CodeGen/X86/sibcall.ll

 ; X32-NEXT:    movl %ebx, %eax
 ; X32-NEXT:    popq %rbx
 ; X32-NEXT:    retq
   tail call fastcc void @t21_f_sret(%struct.foo* noalias sret(%struct.foo) %agg.result) nounwind
   ret void
 }
 
 define fastcc void @t21_sret_to_sret_alloca(%struct.foo* noalias sret(%struct.foo) %agg.result) nounwind  {
 ; X86-LABEL: t21_sret_to_sret_alloca:
 ; X86:       # %bb.0:
 ; X86-NEXT:    pushl %esi
 ; X86-NEXT:    subl $24, %esp
 ; X86-NEXT:    movl %ecx, %esi
 ; X86-NEXT:    leal {{[0-9]+}}(%esp), %ecx
 ; X86-NEXT:    calll t21_f_sret
 ; X86-NEXT:    movl %esi, %eax
 ; X86-NEXT:    addl $24, %esp

 ; X32-NEXT:    movq %rbx, %rdi
 ; X32-NEXT:    callq t21_f_sret2
 ; X32-NEXT:    movl %r14d, %eax
 ; X32-NEXT:    addl $8, %esp
 ; X32-NEXT:    popq %rbx
 ; X32-NEXT:    popq %r14
 ; X32-NEXT:    retq
   %b = call fastcc %struct.foo* @ret_struct()
   tail call fastcc void @t21_f_sret2(%struct.foo* noalias sret(%struct.foo) %a, %struct.foo* noalias %b) nounwind
   ret void
 }
 
 declare ccc %struct.foo* @ret_struct() nounwind
 
 
 define fastcc void @t21_sret_to_non_sret(%struct.foo* noalias sret(%struct.foo) %agg.result) nounwind  {
 ; X86-LABEL: t21_sret_to_non_sret:
 ; X86:       # %bb.0:
 ; X86-NEXT:    pushl %esi
 ; X86-NEXT:    subl $8, %esp
 ; X86-NEXT:    movl %ecx, %esi
 ; X86-NEXT:    calll t21_f_non_sret
 ; X86-NEXT:    movl %esi, %eax
 ; X86-NEXT:    addl $8, %esp

 ; X32-NEXT:    callq t21_f_non_sret
 ; X32-NEXT:    movl %ebx, %eax
 ; X32-NEXT:    popq %rbx
 ; X32-NEXT:    retq
   tail call fastcc void @t21_f_non_sret(%struct.foo* %agg.result) nounwind
   ret void
 }
 
-
 define ccc void @t22_non_sret_to_sret(%struct.foo* %agg.result) nounwind  {
 ; X86-LABEL: t22_non_sret_to_sret:
 ; X86:       # %bb.0:
-; X86-NEXT:    subl $12, %esp
-; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X86-NEXT:    movl %eax, (%esp)
-; X86-NEXT:    calll t22_f_sret@PLT
-; X86-NEXT:    addl $8, %esp
-; X86-NEXT:    retl
+; X86-NEXT:    jmp  t22_f_sret@PLT                  # TAILCALL
 ;
 ; X64-LABEL: t22_non_sret_to_sret:
 ; X64:       # %bb.0:
-; X64-NEXT:    pushq %rax
-; X64-NEXT:    callq t22_f_sret@PLT
-; X64-NEXT:    popq %rax
-; X64-NEXT:    retq
+; X64-NEXT:    jmp  t22_f_sret@PLT                  # TAILCALL
 ;
 ; X32-LABEL: t22_non_sret_to_sret:
 ; X32:       # %bb.0:
-; X32-NEXT:    pushq %rax
-; X32-NEXT:    callq t22_f_sret@PLT
-; X32-NEXT:    popq %rax
-; X32-NEXT:    retq
+; X32-NEXT:    jmp  t22_f_sret@PLT                  # TAILCALL
   tail call ccc void @t22_f_sret(%struct.foo* noalias sret(%struct.foo) %agg.result) nounwind
   ret void
 }
 
 declare dso_local fastcc void @t21_f_sret(%struct.foo* noalias sret(%struct.foo)) nounwind
 declare dso_local fastcc void @t21_f_sret2(%struct.foo* noalias sret(%struct.foo), %struct.foo* noalias) nounwind
 declare dso_local fastcc void @t21_f_non_sret(%struct.foo*) nounwind
 
 declare ccc void @t22_f_sret(%struct.foo* noalias sret(%struct.foo)) nounwind
 
 declare ccc void @f_sret(%struct.foo* noalias sret(%struct.foo), i32, i32) nounwind