Fix X86-64 ABIT issue when splitting an i128 into two i64 during function call

[serge-sans-paille]

Summary

X86-64 ABI mandates that wrt. argument passing, i128 is considered as a struct {i64, i64}
and thus that it cannot be split across register and stack.

GCC already implements this behavior, while clang doesn't, which can cause
subtle runtime issues when mixing both object files.

As a side effect, share some code between 32 and 64 bit implementation.

[serge-sans-paille]

This diff relies on the ArgumentNeedsConsecutiveRegisters property which is unused for X86. I'm new to codegen and will happily follow another track.

[craig.topper]

Tests?

[serge-sans-paille]

Add test case

[pengfei]

llvm/lib/Target/X86/X86CallingConv.td

545

This is not correct. See the generated code in large-argument-count-i128.ll:12. We want the i128 (consecutive 2 i64) memory is 16 byte aligned instead of 2 i64 that aligned 16 bytes.

llvm/test/CodeGen/X86/large-argument-count-i128.ll

12

This should be 8?

llvm/lib/Target/X86/X86CallingConv.cpp

for (unsigned I = 0; I < RequiredGprsUponSplit; I++) {
     // Assign the value to the allocated register
     State.addLoc(CCValAssign::getCustomReg(ValNo, ValVT, Reg, LocVT, LocInfo));
   }
 
   // Successful in allocating registers - stop scanning next rules.
   return true;
 }
 
+static bool NotEnoughRemaingRegisters(ISD::ArgFlagsTy &ArgFlags,
+                                      CCState &State) {
+  static const MCPhysReg RegList[] = {X86::RDI, X86::RSI, X86::RDX,
+                                      X86::RCX, X86::R8,  X86::R9};
+  static const unsigned NumRegs = sizeof(RegList) / sizeof(RegList[0]);
+  unsigned FirstFree = State.getFirstUnallocated(RegList);
+  return FirstFree + 1 >= NumRegs;
+}
+
 static ArrayRef<MCPhysReg> CC_X86_VectorCallGetSSEs(const MVT &ValVT) {
   if (ValVT.is512BitVector()) {
     static const MCPhysReg RegListZMM[] = {X86::ZMM0, X86::ZMM1, X86::ZMM2,
                                            X86::ZMM3, X86::ZMM4, X86::ZMM5};
     return makeArrayRef(std::begin(RegListZMM), std::end(RegListZMM));
   }
 
   if (ValVT.is256BitVector()) {

llvm/lib/Target/X86/X86CallingConv.td

def CC_X86_64_C : CallingConv<[
   CCIfCC<"CallingConv::SwiftTail",
     CCIfSRet<CCIfType<[i64], CCAssignToReg<[RAX]>>>>,
 
   // Pointers are always passed in full 64-bit registers.
   CCIfPtr<CCCustom<"CC_X86_64_Pointer">>,
 
   // The first 6 integer arguments are passed in integer registers.
   CCIfType<[i32], CCAssignToReg<[EDI, ESI, EDX, ECX, R8D, R9D]>>,
+  CCIfConsecutiveRegs<CCIf<"NotEnoughRemaingRegisters(ArgFlags, State)", CCAssignToStack<8, 16>>>,
   CCIfType<[i64], CCAssignToReg<[RDI, RSI, RDX, RCX, R8 , R9 ]>>,
 
   // The first 8 MMX vector arguments are passed in XMM registers on Darwin.
   CCIfType<[x86mmx],
             CCIfSubtarget<"isTargetDarwin()",
             CCIfSubtarget<"hasSSE2()",
             CCPromoteToType<v2i64>>>>,
 

llvm/lib/Target/X86/X86ISelLowering.h

public:
     unsigned getVectorTypeBreakdownForCallingConv(
         LLVMContext &Context, CallingConv::ID CC, EVT VT, EVT &IntermediateVT,
         unsigned &NumIntermediates, MVT &RegisterVT) const override;
 
     bool isIntDivCheap(EVT VT, AttributeList Attr) const override;
 
     bool supportSwiftError() const override;
 
+    bool functionArgumentNeedsConsecutiveRegisters(
+        Type *Ty, CallingConv::ID CallConv, bool isVarArg,
+        const DataLayout &DL) const override;
+
     bool hasStackProbeSymbol(MachineFunction &MF) const override;
     bool hasInlineStackProbe(MachineFunction &MF) const override;
     StringRef getStackProbeSymbolName(MachineFunction &MF) const override;
 
     unsigned getStackProbeSize(MachineFunction &MF) const;
 
     bool hasVectorBlend() const override { return true; }
 

llvm/lib/Target/X86/X86ISelLowering.cpp

for (auto *Exit : Exits)
           .addReg(NewVR);
   }
 }
 
 bool X86TargetLowering::supportSwiftError() const {
   return Subtarget.is64Bit();
 }
 
+bool X86TargetLowering::functionArgumentNeedsConsecutiveRegisters(
+    Type *Ty, CallingConv::ID CallConv, bool isVarArg,
+    const DataLayout &DL) const {
+  return Ty->isIntegerTy(128);
+}
+
 /// Returns true if stack probing through a function call is requested.
 bool X86TargetLowering::hasStackProbeSymbol(MachineFunction &MF) const {
   return !getStackProbeSymbolName(MF).empty();
 }
 
 /// Returns true if stack probing through inline assembly is requested.
 bool X86TargetLowering::hasInlineStackProbe(MachineFunction &MF) const {
 

llvm/test/CodeGen/X86/large-argument-count-i128.ll

+; RUN: llc < %s -mtriple=x86_64-- | FileCheck %s
+
+; The important part of this test is that r9d is ot used to store the
+; lower part of the i128. Instead r9d is unused and the i128 is passed
+; on the stack.
+
+; CHECK-LABEL: bar:
+; CHECK:       # %bb.0:
+; CHECK-NEXT: subq  $40, %rsp
+; CHECK-NEXT: .cfi_def_cfa_offset 48
+; CHECK-NEXT: movl  %edi, 36(%rsp)
+; CHECK-NEXT: movq  $0, 16(%rsp)
+; CHECK-NEXT: movq  $1, (%rsp)
+; CHECK-NEXT: leaq  36(%rsp), %rdx
+; CHECK-NEXT: xorl  %edi, %edi
+; CHECK-NEXT: movl  $1, %esi
+; CHECK-NEXT: movl  $3, %ecx
+; CHECK-NEXT: movl  $4, %r8d
+; CHECK-NEXT: callq foo@PLT
+; CHECK-NEXT: addq  $40, %rsp
+; CHECK-NEXT: .cfi_def_cfa_offset 8
+; CHECK-NEXT: retq
+
+
+define void @bar(i32 %a) {
+  %b = alloca i32, align 4
+  store i32 %a, i32* %b
+  call void @foo(i32 0, i32 1, i32* nonnull %b, i32 3, i32 4, i128 1)
+  ret void
+}
+
+declare void @foo(i32, i32, i32*, i32, i32, i128)