[X86] Better support for the MCU psABI

[mkuper]

Summary

This adds support for the MCU psABI in a way different from r251223 and r251224, basically reverting most of these two patches.
The problem with the approach taken in r251223 is that it only handled libcalls that originated from the backend. However, the midend also inserts quite a few libcalls and - with good reason - assumes these use the platform's default calling convention.

The previous patch tried to insert inregs when necessary both in the FE and, somewhat hackily, in the CG. This patch (and its clang companion patch) goes in a different direction. It defines a new default calling convention for the MCU, which doesn't use inreg marking at all, similarly to what, say, x86-64 does.

[mkuper]

The clang part of this patch is D15055 .

lib/Target/X86/X86CallingConv.h

46

I am not a big fan of this, but I couldn't think of a better to do it (and the idea here is stolen from similar code in the ARM codegen, although the set of constraints there is a bit different).
Other suggestions are welcome.

lib/Target/X86/X86FastISel.cpp

1105

This is actually not an MCU-related issue - this code should fire for x86 regardless of bitness and windows-ness. The DAG-based selection does the right thing, but FastISel didn't. Of course, since the MCU is non-windows 32-bit, it also hits this.

[mkuper]

ping

[mkuper]

ping?

I'd really appreciate a sanity check for this (and D15055 ).

[mkuper]

Fixed a TODO in the previous version on the patch.
It now properly supports fp128 (which is an optional type in the MCU psABI).

[rnk]

Seems reasonable.

lib/Target/X86/X86CallingConv.h

46

Given that it's what ARM does, let's go with it.

lib/Target/X86/X86CallingConv.td

603–604

ECX is an allocatable argument register, so this will not implement 'nest' properly. Either ignoring 'nest' completely or giving a fatal error seems like better behavior than having code that looks like it's trying to implement something.

lib/Target/X86/X86FastISel.cpp

1105

Aha, I changed SDag to do the right thing here, but not fastisel. Oops.

test/CodeGen/X86/mcu-abi.ll

64–66

Idle thought: We should teach LLVM that memcpy returns its first argument, so that we don't have to spill in this kind of situation. :)

[mkuper]

Thanks, Reid!

lib/Target/X86/X86CallingConv.td

603–604

Right, thanks.

test/CodeGen/X86/mcu-abi.ll

64–66

Yeah. Interesting. :-)

include/llvm/Target/TargetLowering.h

struct CallLoweringInfo {
     }
 
     ArgListTy &getArgs() {
       return Args;
     }
 
   };
 
-  // Mark inreg arguments for lib-calls. For normal calls this is done by
-  // the frontend ABI code.
-  virtual void markInRegArguments(SelectionDAG &DAG, 
-                 TargetLowering::ArgListTy &Args) const {
-    return;
-  }
-
   /// This function lowers an abstract call to a function into an actual call.
   /// This returns a pair of operands.  The first element is the return value
   /// for the function (if RetTy is not VoidTy).  The second element is the
   /// outgoing token chain. It calls LowerCall to do the actual lowering.
   std::pair<SDValue, SDValue> LowerCallTo(CallLoweringInfo &CLI) const;
 
   /// This hook must be implemented to lower calls into the specified
   /// DAG. The outgoing arguments to the call are described by the Outs array,

lib/CodeGen/SelectionDAG/TargetLowering.cpp

TargetLowering::makeLibCall(SelectionDAG &DAG,
   for (SDValue Op : Ops) {
     Entry.Node = Op;
     Entry.Ty = Entry.Node.getValueType().getTypeForEVT(*DAG.getContext());
     Entry.isSExt = shouldSignExtendTypeInLibCall(Op.getValueType(), isSigned);
     Entry.isZExt = !shouldSignExtendTypeInLibCall(Op.getValueType(), isSigned);
     Args.push_back(Entry);
   }
 
-  markInRegArguments(DAG, Args);
-
   if (LC == RTLIB::UNKNOWN_LIBCALL)
     report_fatal_error("Unsupported library call operation!");
   SDValue Callee = DAG.getExternalSymbol(getLibcallName(LC),
                                          getPointerTy(DAG.getDataLayout()));
 
   Type *RetTy = RetVT.getTypeForEVT(*DAG.getContext());
   TargetLowering::CallLoweringInfo CLI(DAG);
   bool signExtend = shouldSignExtendTypeInLibCall(RetVT, isSigned);

lib/Target/X86/X86CallingConv.h

 // This file contains the custom routines for the X86 Calling Convention that
 // aren't done by tablegen.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LIB_TARGET_X86_X86CALLINGCONV_H
 #define LLVM_LIB_TARGET_X86_X86CALLINGCONV_H
 
+#include "MCTargetDesc/X86MCTargetDesc.h"
 #include "llvm/CodeGen/CallingConvLower.h"
 #include "llvm/IR/CallingConv.h"
 
 namespace llvm {
 
 inline bool CC_X86_32_VectorCallIndirect(unsigned &ValNo, MVT &ValVT,
                                          MVT &LocVT,
                                          CCValAssign::LocInfo &LocInfo,
inline bool CC_X86_AnyReg_Error(unsigned &, MVT &, MVT &,
                                 CCValAssign::LocInfo &, ISD::ArgFlagsTy &,
                                 CCState &) {
   llvm_unreachable("The AnyReg calling convention is only supported by the " \
                    "stackmap and patchpoint intrinsics.");
   // gracefully fallback to X86 C calling convention on Release builds.
   return false;
 }
 
+inline bool CC_X86_32_MCUInReg(unsigned &ValNo, MVT &ValVT,
+                                         MVT &LocVT,
+                                         CCValAssign::LocInfo &LocInfo,
+                                         ISD::ArgFlagsTy &ArgFlags,
+                                         CCState &State) {
+  // This is similar to CCAssignToReg<[EAX, EDX, ECX]>, but makes sure
+  // not to split i64 and double between a register and stack
+  static const MCPhysReg RegList[] = {X86::EAX, X86::EDX, X86::ECX};
+  static const unsigned NumRegs = 3;
+  
+  SmallVectorImpl<CCValAssign> &PendingMembers = State.getPendingLocs();
+
+  // TODO: Do we need to generalize this to larger cases? That is, can
+  // we end up with something that's larger than 2 parts here?
+  // I think not, but need to check - unfortunately, it doesn't seem
+  // like there's enough information at this point to perform the check...
+
+  // If this is the first part of an i64/double, add it to the pending list
+  if (ArgFlags.isSplit()) {
+    PendingMembers.push_back(
+        CCValAssign::getPending(ValNo, ValVT, LocVT, LocInfo));
+    return true;
+  }
+
+  // If there are no pending members, this is not the second part of
+  // an i64/double, so do the usual inreg stuff
+  if (PendingMembers.empty()) {
+    if (unsigned Reg = State.AllocateReg(RegList)) {
+      State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo));
+      return true;
+    }
+    return false;
+  }
+
+  // Ok, so, now what we do depends on how many free registers we have
+  // We need to have at least 2, because we need both parts to either
+  // be on the stack or in memory.
+  unsigned FirstFree = State.getFirstUnallocated(RegList);
+  bool UseRegs = FirstFree <= (NumRegs - 2);
+
+  PendingMembers.push_back(CCValAssign::getPending(ValNo, ValVT, LocVT,
+    LocInfo));
+  for (auto &It : PendingMembers) {
+    if (UseRegs)
+      It.convertToReg(State.AllocateReg(RegList[FirstFree++]));
+    else
+      It.convertToMem(State.AllocateStack(4, 4));
+    State.addLoc(It);
+  }
+
+  PendingMembers.clear();
+
+  return true;
+}
+
 } // End llvm namespace
 
 #endif
 

lib/Target/X86/X86CallingConv.td

def CC_X86_32_C : CallingConv<[
   // The first 3 integer arguments, if marked 'inreg' and if the call is not
   // a vararg call, are passed in integer registers.
   CCIfNotVarArg<CCIfInReg<CCIfType<[i32], CCAssignToReg<[EAX, EDX, ECX]>>>>,
 
   // Otherwise, same as everything else.
   CCDelegateTo<CC_X86_32_Common>
 ]>;
 
+def CC_X86_32_MCU : CallingConv<[
+  // Handles byval parameters.  Note that, like FastCC, we can't rely on
+  // the delegation to CC_X86_32_Common because that happens after code that
+  // puts arguments in registers.
+  CCIfByVal<CCPassByVal<4, 4>>,
+
+  // Promote i1/i8/i16 arguments to i32.
+  CCIfType<[i1, i8, i16], CCPromoteToType<i32>>,
+
+  // The 'nest' parameter, if any, is passed in ECX.
+  CCIfNest<CCAssignToReg<[ECX]>>,
+
+  // If the call is not a vararg call, some arguments may be passed
+  // in integer registers.
+  CCIfNotVarArg<CCIfType<[i32], CCCustom<"CC_X86_32_MCUInReg">>>,
+
+  // Otherwise, same as everything else.
+  CCDelegateTo<CC_X86_32_Common>
+]>;
+
 def CC_X86_32_FastCall : CallingConv<[
   // Promote i1/i8/i16 arguments to i32.
   CCIfType<[i1, i8, i16], CCPromoteToType<i32>>,
 
   // The 'nest' parameter, if any, is passed in EAX.
   CCIfNest<CCAssignToReg<[EAX]>>,
 
   // The first 2 integer arguments are passed in ECX/EDX

 ]>;
 
 //===----------------------------------------------------------------------===//
 // X86 Root Argument Calling Conventions
 //===----------------------------------------------------------------------===//
 
 // This is the root argument convention for the X86-32 backend.
 def CC_X86_32 : CallingConv<[
+  CCIfSubtarget<"isTargetMCU()", CCDelegateTo<CC_X86_32_MCU>>,
   CCIfCC<"CallingConv::X86_FastCall", CCDelegateTo<CC_X86_32_FastCall>>,
   CCIfCC<"CallingConv::X86_VectorCall", CCDelegateTo<CC_X86_32_VectorCall>>,
   CCIfCC<"CallingConv::X86_ThisCall", CCDelegateTo<CC_X86_32_ThisCall>>,
   CCIfCC<"CallingConv::Fast", CCDelegateTo<CC_X86_32_FastCC>>,
   CCIfCC<"CallingConv::GHC", CCDelegateTo<CC_X86_32_GHC>>,
   CCIfCC<"CallingConv::HiPE", CCDelegateTo<CC_X86_32_HiPE>>,
 
   // Otherwise, drop to normal X86-32 CC

lib/Target/X86/X86FastISel.cpp

if (Ret->getNumOperands() > 0) {
     // Add register to return instruction.
     RetRegs.push_back(VA.getLocReg());
   }
 
   // The x86-64 ABI for returning structs by value requires that we copy
   // the sret argument into %rax for the return. We saved the argument into
   // a virtual register in the entry block, so now we copy the value out
   // and into %rax. We also do the same with %eax for Win32.
-  if (F.hasStructRetAttr() &&
-      (Subtarget->is64Bit() || Subtarget->isTargetKnownWindowsMSVC())) {
+  if (F.hasStructRetAttr()) {
     unsigned Reg = X86MFInfo->getSRetReturnReg();
     assert(Reg &&
            "SRetReturnReg should have been set in LowerFormalArguments()!");
     unsigned RetReg = Subtarget->is64Bit() ? X86::RAX : X86::EAX;
     BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
             TII.get(TargetOpcode::COPY), RetReg).addReg(Reg);
     RetRegs.push_back(RetReg);
   }
static unsigned computeBytesPoppedByCallee(const X86Subtarget *Subtarget,
   if (Subtarget->getTargetTriple().isOSMSVCRT())
     return 0;
   if (CC == CallingConv::Fast || CC == CallingConv::GHC ||
       CC == CallingConv::HiPE)
     return 0;
 
   if (CS)
     if (CS->arg_empty() || !CS->paramHasAttr(1, Attribute::StructRet) ||
-        CS->paramHasAttr(1, Attribute::InReg))
+        CS->paramHasAttr(1, Attribute::InReg) || Subtarget->isTargetMCU())
       return 0;
 
   return 4;
 }
 
 bool X86FastISel::fastLowerCall(CallLoweringInfo &CLI) {
   auto &OutVals       = CLI.OutVals;
   auto &OutFlags      = CLI.OutFlags;

lib/Target/X86/X86ISelLowering.h

public:
     bool isNoopAddrSpaceCast(unsigned SrcAS, unsigned DestAS) const override;
 
     bool useLoadStackGuardNode() const override;
     /// \brief Customize the preferred legalization strategy for certain types.
     LegalizeTypeAction getPreferredVectorAction(EVT VT) const override;
 
     bool isIntDivCheap(EVT VT, AttributeSet Attr) const override;
 
-    void markInRegArguments(SelectionDAG &DAG, TargetLowering::ArgListTy& Args)
-      const override;
-
   protected:
     std::pair<const TargetRegisterClass *, uint8_t>
     findRepresentativeClass(const TargetRegisterInfo *TRI,
                             MVT VT) const override;
 
   private:
     /// Keep a pointer to the X86Subtarget around so that we can
     /// make the right decision when generating code for different targets.

lib/Target/X86/X86ISelLowering.cpp

 /// CallIsStructReturn - Determines whether a call uses struct return
 /// semantics.
 enum StructReturnType {
   NotStructReturn,
   RegStructReturn,
   StackStructReturn
 };
 static StructReturnType
-callIsStructReturn(const SmallVectorImpl<ISD::OutputArg> &Outs) {
+callIsStructReturn(const SmallVectorImpl<ISD::OutputArg> &Outs, bool IsMCU) {
   if (Outs.empty())
     return NotStructReturn;
 
   const ISD::ArgFlagsTy &Flags = Outs[0].Flags;
   if (!Flags.isSRet())
     return NotStructReturn;
-  if (Flags.isInReg())
+  if (Flags.isInReg() || IsMCU)
     return RegStructReturn;
   return StackStructReturn;
 }
 
 /// Determines whether a function uses struct return semantics.
 static StructReturnType
-argsAreStructReturn(const SmallVectorImpl<ISD::InputArg> &Ins) {
+argsAreStructReturn(const SmallVectorImpl<ISD::InputArg> &Ins, bool IsMCU) {
   if (Ins.empty())
     return NotStructReturn;
 
   const ISD::ArgFlagsTy &Flags = Ins[0].Flags;
   if (!Flags.isSRet())
     return NotStructReturn;
-  if (Flags.isInReg())
+  if (Flags.isInReg() || IsMCU)
     return RegStructReturn;
   return StackStructReturn;
 }
 
 /// Make a copy of an aggregate at address specified by "Src" to address
 /// "Dst" with size and alignment information specified by the specific
 /// parameter attribute. The copy will be passed as a byval function parameter.
 static SDValue
SDValue X86TargetLowering::LowerFormalArguments(
   if (X86::isCalleePop(CallConv, Is64Bit, isVarArg,
                        MF.getTarget().Options.GuaranteedTailCallOpt)) {
     FuncInfo->setBytesToPopOnReturn(StackSize); // Callee pops everything.
   } else {
     FuncInfo->setBytesToPopOnReturn(0); // Callee pops nothing.
     // If this is an sret function, the return should pop the hidden pointer.
     if (!Is64Bit && !canGuaranteeTCO(CallConv) &&
         !Subtarget->getTargetTriple().isOSMSVCRT() &&
-        argsAreStructReturn(Ins) == StackStructReturn)
+        argsAreStructReturn(Ins, Subtarget->isTargetMCU()) == StackStructReturn)
       FuncInfo->setBytesToPopOnReturn(4);
   }
 
   if (!Is64Bit) {
     // RegSaveFrameIndex is X86-64 only.
     FuncInfo->setRegSaveFrameIndex(0xAAAAAAA);
     if (CallConv == CallingConv::X86_FastCall ||
         CallConv == CallingConv::X86_ThisCall)
X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
   SDValue Callee                        = CLI.Callee;
   CallingConv::ID CallConv              = CLI.CallConv;
   bool &isTailCall                      = CLI.IsTailCall;
   bool isVarArg                         = CLI.IsVarArg;
 
   MachineFunction &MF = DAG.getMachineFunction();
   bool Is64Bit        = Subtarget->is64Bit();
   bool IsWin64        = Subtarget->isCallingConvWin64(CallConv);
-  StructReturnType SR = callIsStructReturn(Outs);
+  StructReturnType SR = callIsStructReturn(Outs, Subtarget->isTargetMCU());
   bool IsSibcall      = false;
   X86MachineFunctionInfo *X86Info = MF.getInfo<X86MachineFunctionInfo>();
   auto Attr = MF.getFunction()->getFnAttribute("disable-tail-calls");
 
   if (Attr.getValueAsString() == "true")
     isTailCall = false;
 
   if (Subtarget->isPICStyleGOT() &&
bool X86TargetLowering::isIntDivCheap(EVT VT, AttributeSet Attr) const {
   // The exception to this is vector division. Since x86 doesn't have vector
   // integer division, leaving the division as-is is a loss even in terms of
   // size, because it will have to be scalarized, while the alternative code
   // sequence can be performed in vector form.
   bool OptSize = Attr.hasAttribute(AttributeSet::FunctionIndex,
                                    Attribute::MinSize);
   return OptSize && !VT.isVector();
 }
-
-void X86TargetLowering::markInRegArguments(SelectionDAG &DAG,
-       TargetLowering::ArgListTy& Args) const {
-  // The MCU psABI requires some arguments to be passed in-register.
-  // For regular calls, the inreg arguments are marked by the front-end.
-  // However, for compiler generated library calls, we have to patch this
-  // up here.
-  if (!Subtarget->isTargetMCU() || !Args.size())
-    return;
-
-  unsigned FreeRegs = 3;
-  for (auto &Arg : Args) {
-    // For library functions, we do not expect any fancy types.
-    unsigned Size = DAG.getDataLayout().getTypeSizeInBits(Arg.Ty);
-    unsigned SizeInRegs = (Size + 31) / 32;
-    if (SizeInRegs > 2 || SizeInRegs > FreeRegs)
-      continue;
-
-    Arg.isInReg = true;
-    FreeRegs -= SizeInRegs;
-    if (!FreeRegs)
-      break;
-  }
-}

test/CodeGen/X86/mcu-abi.ll

 ; RUN: llc < %s -mtriple=i686-pc-elfiamcu | FileCheck %s
 
+%struct.st12_t = type { i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32 }
+
+; CHECK-LABEL: test_ints:
+; CHECK: addl    %edx, %eax
+; CHECK-NEXT: imull   %ecx, %eax
+; CHECK-NEXT: addl    4(%esp), %eax
+; CHECK-NEXT: retl
+define i32 @test_ints(i32 %a, i32 %b, i32 %c, i32 %d) #0 {
+entry:
+  %r1 = add i32 %b, %a
+  %r2 = mul i32 %c, %r1
+  %r3 = add i32 %d, %r2
+  ret i32 %r3
+}
+
+; CHECK-LABEL: test_floats:
+; CHECK: addl    %edx, %eax
+; CHECK-NEXT: imull   %ecx, %eax
+; CHECK-NEXT: addl    4(%esp), %eax
+; CHECK-NEXT: retl
+define i32 @test_floats(i32 %a, i32 %b, float %c, float %d) #0 {
+entry:
+  %ci = bitcast float %c to i32
+  %di = bitcast float %d to i32
+  %r1 = add i32 %b, %a
+  %r2 = mul i32 %ci, %r1
+  %r3 = add i32 %di, %r2
+  ret i32 %r3
+}
+
+; CHECK-LABEL: test_doubles:
+; CHECK: addl    4(%esp), %eax
+; CHECK-NEXT: adcl    8(%esp), %edx
+; CHECK-NEXT: retl
+define double @test_doubles(double %d1, double %d2) #0 {
+entry:
+    %d1i = bitcast double %d1 to i64
+    %d2i = bitcast double %d2 to i64
+    %r = add i64 %d1i, %d2i
+    %rd = bitcast i64 %r to double
+    ret double %rd
+}
+
+; CHECK-LABEL: test_mixed_doubles:
+; CHECK: addl    %ecx, %eax
+; CHECK-NEXT: adcl    $0, %edx
+; CHECK-NEXT: retl
+define double @test_mixed_doubles(double %d2, i32 %i) #0 {
+entry:
+    %iext = zext i32 %i to i64
+    %d2i = bitcast double %d2 to i64
+    %r = add i64 %iext, %d2i
+    %rd = bitcast i64 %r to double
+    ret double %rd
+}
+
+; CHECK-LABEL: ret_large_struct:
+; CHECK: pushl   %esi
+; CHECK-NEXT: movl    %eax, %esi
+; CHECK-NEXT: leal    8(%esp), %edx
+; CHECK-NEXT: movl    $48, %ecx
+; CHECK-NEXT: calll   memcpy
+; CHECK-NEXT: movl    %esi, %eax
+; CHECK-NEXT: popl    %esi
+; CHECK-NOT:  retl $4
+; CHECK-NEXT: retl
+define void @ret_large_struct(%struct.st12_t* noalias nocapture sret %agg.result, %struct.st12_t* byval nocapture readonly align 4 %r) #0 {
+entry:
+  %0 = bitcast %struct.st12_t* %agg.result to i8*
+  %1 = bitcast %struct.st12_t* %r to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %0, i8* %1, i32 48, i32 1, i1 false)
+  ret void
+}
+
+; CHECK-LABEL: var_args:
+; CHECK: movl    4(%esp), %eax
+; CHECK-NEXT: retl
+define i32 @var_args(i32 %i1, ...) #0 {
+entry:
+  ret i32 %i1
+}
+
 ; CHECK-LABEL: test_lib_args:
 ; CHECK: movl %edx, %eax
 ; CHECK: calll __fixsfsi
-define i32 @test_lib_args(float inreg %a, float inreg %b) #0 {
+define i32 @test_lib_args(float %a, float %b) #0 {
   %ret = fptosi float %b to i32
   ret i32 %ret
 }
 
+declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture readonly, i32, i32, i1) #1
+
 attributes #0 = { nounwind "use-soft-float"="true"}
+attributes #1 = { nounwind argmemonly }