error when musttail cannot tail

Author: folkertdev

llvm/lib/Target/X86/X86ISelLoweringCall.cpp

@@ -2053,14 +2053,6 @@ X86TargetLowering::ByValCopyKind X86TargetLowering::ByValNeedsCopyForTailCall(
   if (!FixedSrc || (FixedSrc && SrcOffset < 0))
     return CopyOnce;
 
-  // In the case of byval arguments split between registers and the stack,
-  // computeAddrForCallArg returns a FrameIndex which corresponds only to the
-  // stack portion, but the Src SDValue will refer to the full value, including
-  // the local stack memory that the register portion gets stored into. We only
-  // need to compare them for equality, so normalise on the full value version.
-  uint64_t RegSize = Flags.getByValSize() - MFI.getObjectSize(DstFI);
-  DstOffset -= RegSize;
-
   // If the value is already in the correct location, then no copying is
   // needed. If not, then we need to copy via a temporary.
   if (SrcOffset == DstOffset)
@@ -2154,19 +2146,15 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
     IsSibcall = IsEligibleForTailCallOptimization(CLI, CCInfo, ArgLocs,
                                                   IsCalleePopSRet);
 
-    if (!IsMustTail) {
-      isTailCall = IsSibcall;
-
-      // Sibcalls are automatically detected tailcalls which do not require
-      // ABI changes.
-      IsSibcall = IsSibcall && !IsGuaranteeTCO;
-    }
+    if (isTailCall && !getTargetMachine().Options.GuaranteedTailCallOpt &&
+        CallConv != CallingConv::Tail && CallConv != CallingConv::SwiftTail)
+      IsSibcall = true;
 
     if (isTailCall)
       ++NumTailCalls;
   }
 
-  if (IsMustTail && !isTailCall)
+  if (!isTailCall && CLI.CB && CLI.CB->isMustTailCall())
     report_fatal_error("failed to perform tail call elimination on a call "
                        "site marked musttail");
 
@@ -2231,7 +2219,7 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
 
       // Destination: where this byval should live in the callee’s frame
       // after the tail call.
-      int32_t Offset = VA.getLocMemOffset() + FPDiff + RetAddrSize;
+      int32_t Offset = VA.getLocMemOffset() + FPDiff;
       int Size = VA.getLocVT().getFixedSizeInBits() / 8;
       int FI = MF.getFrameInfo().CreateFixedObject(Size, Offset, true);
       SDValue Dst = DAG.getFrameIndex(FI, PtrVT);
@@ -2265,6 +2253,7 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
         ByValCopyChains.push_back(CopyChain);
       }
     }
+
     if (!ByValCopyChains.empty())
       ByValTempChain =
           DAG.getNode(ISD::TokenFactor, dl, MVT::Other, ByValCopyChains);
@@ -2484,7 +2473,7 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
   // For tail calls lower the arguments to the 'real' stack slots.  Sibcalls
   // don't need this because the eligibility check rejects calls that require
   // shuffling arguments passed in memory.
-  if (!IsSibcall && isTailCall) {
+  if (isTailCall) {
     // Force all the incoming stack arguments to be loaded from the stack
     // before any new outgoing arguments or the return address are stored to the
     // stack, because the outgoing stack slots may alias the incoming argument
@@ -2543,14 +2532,15 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
 
         // FIXME: contrary to the arm backend, with the current logic we always
         // seem to need a stack copy.
-        (void)NeedsStackCopy;
+        if (NeedsStackCopy) {
 
-        auto PtrVT = getPointerTy(DAG.getDataLayout());
-        SDValue DstAddr = DAG.getFrameIndex(FI, PtrVT);
+          auto PtrVT = getPointerTy(DAG.getDataLayout());
+          SDValue DstAddr = DAG.getFrameIndex(FI, PtrVT);
 
-        // Copy the struct contents from ByValSrc to DstAddr.
-        MemOpChains2.push_back(CreateCopyOfByValArgument(
-            ByValSrc, DstAddr, Chain, Flags, DAG, dl));
+          // Copy the struct contents from ByValSrc to DstAddr.
+          MemOpChains2.push_back(CreateCopyOfByValArgument(
+              ByValSrc, DstAddr, Chain, Flags, DAG, dl));
+        }
       } else {
         // Store relative to framepointer.
         MemOpChains2.push_back(DAG.getStore(
@@ -2951,9 +2941,10 @@ mayBeSRetTailCallCompatible(const TargetLowering::CallLoweringInfo &CLI,
 
 /// Check whether the call is eligible for tail call optimization. Targets
 /// that want to do tail call optimization should implement this function.
-/// Note that the x86 backend does not check musttail calls for eligibility! The
-/// rest of x86 tail call lowering must be prepared to forward arguments of any
-/// type.
+///
+/// Note that this function also processes musttail calls, so when this
+/// function returns false on a valid musttail call, a fatal backend error
+/// occurs.
 bool X86TargetLowering::IsEligibleForTailCallOptimization(
     TargetLowering::CallLoweringInfo &CLI, CCState &CCInfo,
     SmallVectorImpl<CCValAssign> &ArgLocs, bool IsCalleePopSRet) const {
@@ -3080,26 +3071,6 @@ bool X86TargetLowering::IsEligibleForTailCallOptimization(
   // If the callee takes no arguments then go on to check the results of the
   // call.
   if (!Outs.empty()) {
-    if (StackArgsSize > 0) {
-      // Check if the arguments are already laid out in the right way as
-      // the caller's fixed stack objects.
-      MachineFrameInfo &MFI = MF.getFrameInfo();
-      const MachineRegisterInfo *MRI = &MF.getRegInfo();
-      const X86InstrInfo *TII = Subtarget.getInstrInfo();
-      for (unsigned I = 0, E = ArgLocs.size(); I != E; ++I) {
-        const CCValAssign &VA = ArgLocs[I];
-        SDValue Arg = OutVals[I];
-        ISD::ArgFlagsTy Flags = Outs[I].Flags;
-        if (VA.getLocInfo() == CCValAssign::Indirect)
-          return false;
-        if (!VA.isRegLoc()) {
-          if (!MatchingStackOffset(Arg, VA.getLocMemOffset(), Flags, MFI, MRI,
-                                   TII, VA))
-            return false;
-        }
-      }
-    }
-
     bool PositionIndependent = isPositionIndependent();
     // If the tailcall address may be in a register, then make sure it's
     // possible to register allocate for it. In 32-bit, the call address can
@@ -3137,6 +3108,11 @@ bool X86TargetLowering::IsEligibleForTailCallOptimization(
       X86::isCalleePop(CalleeCC, Subtarget.is64Bit(), isVarArg,
                        MF.getTarget().Options.GuaranteedTailCallOpt);
 
+  // If the stack arguments for this call do not fit into our own save area then
+  // the call cannot be made tail.
+  if (CCInfo.getStackSize() > FuncInfo->getArgumentStackSize())
+    return false;
+
   if (unsigned BytesToPop = FuncInfo->getBytesToPopOnReturn()) {
     // If we have bytes to pop, the callee must pop them.
     bool CalleePopMatches = CalleeWillPop && BytesToPop == StackArgsSize;

llvm/test/CodeGen/X86/musttail-struct.ll

@@ -42,6 +42,8 @@ define dso_local i32 @test5(ptr byval(%struct.5xi32) %0) {
 define dso_local i32 @testManyArgs(i32 %0, i32 %1, i32 %2, i32 %3, i32 %4, i32 %5, i8 %6, ptr byval(%struct.5xi32) %7) {
 ; CHECK-LABEL: testManyArgs:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    movzx eax, byte ptr [rsp + 8]
+; CHECK-NEXT:    mov byte ptr [rsp + 8], al
 ; CHECK-NEXT:    jmp FuncManyArgs # TAILCALL
   %r = musttail call i32 @FuncManyArgs(i32 %0, i32 %1, i32 %2, i32 %3, i32 %4, i32 %5, i8 %6, ptr byval(%struct.5xi32) %7)
   ret i32 %r
@@ -50,6 +52,8 @@ define dso_local i32 @testManyArgs(i32 %0, i32 %1, i32 %2, i32 %3, i32 %4, i32 %
 define dso_local i32 @testRecursion(i32 %0, i32 %1, i32 %2, i32 %3, i32 %4, i32 %5, i8 %6, ptr byval(%struct.5xi32) %7) {
 ; CHECK-LABEL: testRecursion:
 ; CHECK:       # %bb.0:
+; CHECK-NEXT:    movzx eax, byte ptr [rsp + 8]
+; CHECK-NEXT:    mov byte ptr [rsp + 8], al
 ; CHECK-NEXT:    jmp testRecursion # TAILCALL
   %r = musttail call i32 @testRecursion(i32 %0, i32 %1, i32 %2, i32 %3, i32 %4, i32 %5, i8 %6, ptr byval(%struct.5xi32) %7)
   ret i32 %r
@@ -75,10 +79,8 @@ define dso_local i32 @swapByValArguments(ptr byval(%struct.1xi32) %0, ptr byval(
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    mov eax, dword ptr [rsp + 8]
 ; CHECK-NEXT:    mov dword ptr [rsp - 16], eax
-; CHECK-NEXT:    mov ecx, dword ptr [rsp + 16]
-; CHECK-NEXT:    mov dword ptr [rsp - 8], ecx
-; CHECK-NEXT:    mov dword ptr [rsp + 8], ecx
-; CHECK-NEXT:    mov dword ptr [rsp + 16], eax
+; CHECK-NEXT:    mov eax, dword ptr [rsp + 16]
+; CHECK-NEXT:    mov dword ptr [rsp - 8], eax
 ; CHECK-NEXT:    jmp swap # TAILCALL
 
 
@@ -143,20 +145,12 @@ define void @swap_byvals(%twenty_bytes* byval(%twenty_bytes) align 4 %a, %twenty
 ; CHECK-NEXT:    mov dword ptr [rsp - 8], eax
 ; CHECK-NEXT:    movaps xmm0, xmmword ptr [rsp + 8]
 ; CHECK-NEXT:    movaps xmmword ptr [rsp - 24], xmm0
-; CHECK-NEXT:    mov ecx, dword ptr [rsp + 48]
-; CHECK-NEXT:    mov dword ptr [rsp - 32], ecx
-; CHECK-NEXT:    mov rdx, qword ptr [rsp + 32]
-; CHECK-NEXT:    mov rsi, qword ptr [rsp + 40]
-; CHECK-NEXT:    mov qword ptr [rsp - 40], rsi
-; CHECK-NEXT:    mov qword ptr [rsp - 48], rdx
-; CHECK-NEXT:    mov qword ptr [rsp + 8], rdx
-; CHECK-NEXT:    mov qword ptr [rsp + 16], rsi
-; CHECK-NEXT:    mov dword ptr [rsp + 24], ecx
-; CHECK-NEXT:    mov rcx, qword ptr [rsp + 8]
-; CHECK-NEXT:    mov rdx, qword ptr [rsp + 16]
-; CHECK-NEXT:    mov qword ptr [rsp + 32], rcx
-; CHECK-NEXT:    mov qword ptr [rsp + 40], rdx
-; CHECK-NEXT:    mov dword ptr [rsp + 48], eax
+; CHECK-NEXT:    mov eax, dword ptr [rsp + 48]
+; CHECK-NEXT:    mov dword ptr [rsp - 32], eax
+; CHECK-NEXT:    mov rax, qword ptr [rsp + 32]
+; CHECK-NEXT:    mov rcx, qword ptr [rsp + 40]
+; CHECK-NEXT:    mov qword ptr [rsp - 40], rcx
+; CHECK-NEXT:    mov qword ptr [rsp - 48], rax
 ; CHECK-NEXT:    jmp two_byvals_callee@PLT # TAILCALL
 entry:
   musttail call void @two_byvals_callee(%twenty_bytes* byval(%twenty_bytes) align 4 %b, %twenty_bytes* byval(%twenty_bytes) align 4 %a)