Skip to content

__int128 stack calling conventions are incorrect on x86-64 #41784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
stephenhines opened this issue Jun 28, 2019 · 5 comments
Closed

__int128 stack calling conventions are incorrect on x86-64 #41784

stephenhines opened this issue Jun 28, 2019 · 5 comments
Assignees
@nikic
Labels
ABI Application Binary Interface backend:X86 bugzilla Issues migrated from bugzilla

Comments

@stephenhines
Copy link
Collaborator

stephenhines commented Jun 28, 2019 •
edited by nikic
Loading

Bugzilla Link 42439
Version trunk
OS Linux
CC @topperc,@gregbedwell,@RKSimon,@nickdesaulniers,@pogo59,@rotateright,@tstellar,@wjristow

Extended Description

This is a bug report that was posted internally at Google, since the user can't register here on bugs.llvm.org:

Clang violates x86-64 calling convention in the obscure case when __int128 is passed on stack.

Conside the following two functions:

__int128 foo(__int128 x, __int128 y, __int128 z, uint64_t a, __int128 c) {
   return x + y + z + a + c;
}

__int128 foo(__int128 x, __int128 y, __int128 z, uint64_t a, uint64_t b, __int128 c) {
   return x + y + z + a + c;
}

Gcc generates identical code for these because __int128 have to be stored aligned on stack (psABI says:

Arguments of type __int128 offer the same operations as INTEGERs, yet they do not fit into one general purpose register but require two registers.

For classification purposes __int128 is treated as if it were implemented as:
typedef struct {
long low, high;
} __int128;
with the exception that arguments of type __int128 that are stored in memory must be aligned on a 16-byte boundary.

Clang generates different code for these two functions:
https://godbolt.org/z/ZKjb4Z

Note that both GCC and Clang show the alignment of __int128 properly as 16 bytes, but this is only getting ignored when the variable is passed on the stack.
@topperc
Copy link
Collaborator

topperc commented Dec 13, 2019

Do we also put half in a register and half in memory if we've already used 5 GPRs before we get to the i128?

@tstellar
Copy link
Collaborator

These bugs are related:

#20283
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92904

@topperc
Copy link
Collaborator

topperc commented Dec 13, 2019 •
edited by nikic
Loading

This patch might work. I based this of some similar code in AArch64

diff --git a/llvm/lib/Target/X86/X86CallingConv.td b/llvm/lib/Target/X86/X86CallingConv.td
index 30d05c6..0b07033 100644
--- a/llvm/lib/Target/X86/X86CallingConv.td
+++ b/llvm/lib/Target/X86/X86CallingConv.td
@@ -516,6 +516,14 @@ def CC_X86_64_C : CallingConv<[

   // The first 6 integer arguments are passed in integer registers.
   CCIfType<[i32], CCAssignToReg<[EDI, ESI, EDX, ECX, R8D, R9D]>>,
+
+  // i128 is split to two i64s, we can't fit half to register R9.
+  CCIfType<[i64],
+           CCIfSplit<CCAssignToReg<[EDI, ESI, EDX, ECX, R8D]>>>,
+
+  // i128 is split to two i64s, and its stack alignment is 16 bytes.
+  CCIfType<[i64], CCIfSplit<CCAssignToStackWithShadow<8, 16, [R9]>>>,
+
   CCIfType<[i64], CCAssignToReg<[RDI, RSI, RDX, RCX, R8 , R9 ]>>,

   // The first 8 MMX vector arguments are passed in XMM registers on Darwin.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@workingjubilee workingjubilee mentioned this issue Feb 23, 2023
Open
@tgross35 tgross35 mentioned this issue Jul 20, 2023
Open
This was referenced Aug 11, 2023
Closed
Open
@nikic
Copy link
Contributor

nikic commented Aug 17, 2023

I was wondering why Clang's ABI handling doesn't end up passing this explicitly on the stack, and it seems like this is the reason:

llvm-project/clang/lib/CodeGen/Targets/X86.cpp

Lines 2155 to 2171 in 91af0d0

// If this is a scalar LLVM value then assume LLVM will pass it in the right
// place naturally.
//
// This assumption is optimistic, as there could be free registers available
// when we need to pass this argument in memory, and LLVM could try to pass
// the argument in the free register. This does not seem to happen currently,
// but this code would be much safer if we could mark the argument with
// 'onstack'. See PR12193.
if (!isAggregateTypeForABI(Ty) && !IsIllegalVectorType(Ty) &&
!Ty->isBitIntType()) {
// Treat an enum type as its underlying type.
if (const EnumType *EnumTy = Ty->getAs<EnumType>())
Ty = EnumTy->getDecl()->getIntegerType();
return (isPromotableIntegerTypeForABI(Ty) ? ABIArgInfo::getExtend(Ty)
: ABIArgInfo::getDirect());
}

As the comment indicates the assumption is, indeed, optimistic...

@nikic
Copy link
Contributor

nikic commented Aug 17, 2023

Candidate patch: https://reviews.llvm.org/D158169

@nikic nikic self-assigned this Aug 17, 2023
@nikic nikic closed this as completed in fa1b6e6 Aug 21, 2023
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 2, 2023
@nikic @razmser
[X86] Fix i128 argument passing under SysV ABI
6e7f4f7 
The x86_64 SysV ABI specifies that __int128 is passed either in
two registers (if available) or in a 16 byte aligned stack slot.
GCC implements this behavior. However, if only one free register
is available, LLVM will instead pass one half of the i128 in a
register, and the other on the stack.

Make sure that either both are passed in registers or both on the
stack.

Fixes llvm#41784.
The patch is basically what craig.topper proposed to do there.

Differential Revision: https://reviews.llvm.org/D158169
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 2, 2023
@nikic @razmser
[X86] Fix i128 argument passing under SysV ABI
2f5f020 
The x86_64 SysV ABI specifies that __int128 is passed either in
two registers (if available) or in a 16 byte aligned stack slot.
GCC implements this behavior. However, if only one free register
is available, LLVM will instead pass one half of the i128 in a
register, and the other on the stack.

Make sure that either both are passed in registers or both on the
stack.

Fixes llvm#41784.
The patch is basically what craig.topper proposed to do there.

Differential Revision: https://reviews.llvm.org/D158169
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 2, 2023
@nikic @razmser
[X86] Fix i128 argument passing under SysV ABI
4e0582a 
The x86_64 SysV ABI specifies that __int128 is passed either in
two registers (if available) or in a 16 byte aligned stack slot.
GCC implements this behavior. However, if only one free register
is available, LLVM will instead pass one half of the i128 in a
register, and the other on the stack.

Make sure that either both are passed in registers or both on the
stack.

Fixes llvm#41784.
The patch is basically what craig.topper proposed to do there.

Differential Revision: https://reviews.llvm.org/D158169
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 3, 2023
@nikic @razmser
[X86] Fix i128 argument passing under SysV ABI
2f49e37 
The x86_64 SysV ABI specifies that __int128 is passed either in
two registers (if available) or in a 16 byte aligned stack slot.
GCC implements this behavior. However, if only one free register
is available, LLVM will instead pass one half of the i128 in a
register, and the other on the stack.

Make sure that either both are passed in registers or both on the
stack.

Fixes llvm#41784.
The patch is basically what craig.topper proposed to do there.

Differential Revision: https://reviews.llvm.org/D158169
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 3, 2023
@nikic @razmser
[X86] Fix i128 argument passing under SysV ABI
f97e772 
The x86_64 SysV ABI specifies that __int128 is passed either in
two registers (if available) or in a 16 byte aligned stack slot.
GCC implements this behavior. However, if only one free register
is available, LLVM will instead pass one half of the i128 in a
register, and the other on the stack.

Make sure that either both are passed in registers or both on the
stack.

Fixes llvm#41784.
The patch is basically what craig.topper proposed to do there.

Differential Revision: https://reviews.llvm.org/D158169
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 6, 2023
@nikic @razmser
[X86] Fix i128 argument passing under SysV ABI
2ea27a1 
The x86_64 SysV ABI specifies that __int128 is passed either in
two registers (if available) or in a 16 byte aligned stack slot.
GCC implements this behavior. However, if only one free register
is available, LLVM will instead pass one half of the i128 in a
register, and the other on the stack.

Make sure that either both are passed in registers or both on the
stack.

Fixes llvm#41784.
The patch is basically what craig.topper proposed to do there.

Differential Revision: https://reviews.llvm.org/D158169
razmser pushed a commit to SuduIDE/llvm-project that referenced this issue Oct 11, 2023
@nikic @razmser
[X86] Fix i128 argument passing under SysV ABI
519b705 
The x86_64 SysV ABI specifies that __int128 is passed either in
two registers (if available) or in a 16 byte aligned stack slot.
GCC implements this behavior. However, if only one free register
is available, LLVM will instead pass one half of the i128 in a
register, and the other on the stack.

Make sure that either both are passed in registers or both on the
stack.

Fixes llvm#41784.
The patch is basically what craig.topper proposed to do there.

Differential Revision: https://reviews.llvm.org/D158169
tgross35 pushed a commit to tgross35/llvm-project that referenced this issue Dec 15, 2023
@nikic @tgross35
[X86] Fix i128 argument passing under SysV ABI
5417cc6 
The x86_64 SysV ABI specifies that __int128 is passed either in
two registers (if available) or in a 16 byte aligned stack slot.
GCC implements this behavior. However, if only one free register
is available, LLVM will instead pass one half of the i128 in a
register, and the other on the stack.

Make sure that either both are passed in registers or both on the
stack.

Fixes llvm#41784.
The patch is basically what craig.topper proposed to do there.

Differential Revision: https://reviews.llvm.org/D158169

(cherry picked from commit fa1b6e6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nikic nikic

Labels
ABI Application Binary Interface backend:X86 bugzilla Issues migrated from bugzilla
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@tstellar @nikic @stephenhines @topperc