[PATCH] D107965: Fix X86-64 ABIT issue when splitting an i128 into two i64 during function call

[serge via Phabricator via llvm-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: efriedma, rnk, pengfei.
Herald added a subscriber: hiraditya.
serge-sans-paille requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107965

Files:
  llvm/lib/Target/X86/X86CallingConv.cpp
  llvm/lib/Target/X86/X86CallingConv.td
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h


Index: llvm/lib/Target/X86/X86ISelLowering.h
===================================================================
--- llvm/lib/Target/X86/X86ISelLowering.h
+++ llvm/lib/Target/X86/X86ISelLowering.h
@@ -1399,6 +1399,10 @@
 
     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;
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -52401,6 +52401,12 @@
   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();
Index: llvm/lib/Target/X86/X86CallingConv.td
===================================================================
--- llvm/lib/Target/X86/X86CallingConv.td
+++ llvm/lib/Target/X86/X86CallingConv.td
@@ -533,6 +533,7 @@
 
   // 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.
Index: llvm/lib/Target/X86/X86CallingConv.cpp
===================================================================
--- llvm/lib/Target/X86/X86CallingConv.cpp
+++ llvm/lib/Target/X86/X86CallingConv.cpp
@@ -64,6 +64,15 @@
   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,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107965.365999.patch
Type: text/x-patch
Size: 2757 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210812/ec1f6dab/attachment.bin>