Skip to content

Concerns about x86-64 _BitInt(65-128) alignment #60925

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.

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

Open
workingjubilee opened this issue Feb 22, 2023 · 4 comments
Open

Concerns about x86-64 _BitInt(65-128) alignment #60925

workingjubilee opened this issue Feb 22, 2023 · 4 comments
Labels
ABI Application Binary Interface backend:X86

Comments

@workingjubilee
Copy link
Contributor

It is my understanding that currently, for the System V AMD64 ABI specification of _BitInt(65-128), it has an alignment of only 8, whereas __int128 has an alignment of 16. If I remember correctly, since both are specified to be "as if a struct of 8bytes", this has no consequence for the raw by-value case for functions with a few parameters. However, in the presence of many arguments, this means that it will hit the stack, and that it will be passed differently than __int128 on the stack. It also means that if an allocation attempts to align itself to _BitInt(128), it is incorrectly aligned for __int128, whereas __int128 is correctly (over)aligned for _BitInt(128) allocations.

I have a concern about cross-language, and even, to some extent, assembly-code interoperability. A lack of 16-byte alignment means the type is incorrectly aligned using as a target of CMPXCHG16B's all-important m128 operand which must be 16-byte aligned (an indicator of a 128-bit value at a memory address, not to be confused with __m128, which only prefers it for VEX encodings, except for VMOVAPS, but also the legacy SSE encodings still exist). It also means that any other programming language must double-up on types in order to do C FFI for such 16-byte types, whereas in some cases they may already have such types (and may want to already be able to use it for __int128). Obviously, the C Standard doesn't guarantee other languages won't need them anyways, but it still seems preferable if more implementations choose the form which has the largest aligned integer type match _BitInt(N), even if merely for principle-of-least-astonishment reasons, and to help other compiler implementers down the road.

Obviously, we still need a stopping point somewhere. I only suggest it here as it's already a specified large integer type. The stack is 16-byte-aligned anyways, too.

I am aware that this decision is ultimately also something that requires the attention of the people who specified the psABI, and I have raised this as an issue with them. I am mentioning it to you, here, because I wish to make sure this concern is factored in to the release of LLVM 16, and because Clang is the pioneer of this implementation.

@EugeneZelenko EugeneZelenko added backend:X86 ABI Application Binary Interface and removed new issue labels Feb 22, 2023
@llvmbot
Copy link
Member

llvmbot commented Feb 22, 2023

@llvm/issue-subscribers-backend-x86

@phoebewang
Copy link
Contributor

cc @FreddyLeaf
Add an example which may demonstrate it: https://godbolt.org/z/c16sqjsjs
It's unfortunate __int128 isn't aligned to 16bytes in Clang, which makes it happen to behave the same as _BitInt(128).

@workingjubilee
Copy link
Contributor Author

Indeed, Clang correctly believes __int128 is aligned to 16 bytes and then simply fails to pass it with that alignment during parameter handling. https://godbolt.org/z/eM7Yab41W

Also see:

@hvdijk
Copy link
Contributor

hvdijk commented Nov 2, 2023

Related: in D86310, an issue is raised that results from _BitInt(129) having size 24, alignment 8, whereas LLVM's i129 has, in i128:128 layouts, size 32, alignment 16. This means _BitInt(129) cannot be implemented in terms of LLVM's i129, and the fact that Clang does so results in various codegen issues on targets with those layouts (currently X86 is the only one where this is a non-experimental type). Presumably if the ABI alignment of _BitInt(65-128) would be changed, _BitInt(129) would also change to size 32, alignment 16, and it becomes possible again to use i129.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI Application Binary Interface backend:X86
Projects
None yet
Development

No branches or pull requests

5 participants