Remove i128 and u128 from improper_ctypes_definitions

[tgross35]

Rust's 128-bit integers have historically been incompatible with C 1. However, there have been a number of changes in Rust and LLVM that mean this is no longer the case:

• Incorrect alignment of i128 on x86 1: adjusting Rust's alignment proposed at Set alignment of i128 to 128 bits for x86 compiler-team#683, implemented at LLVM 18 x86 data layout update #116672.
• LLVM version of the above: resolved in LLVM, including ABI fix. Present in LLVM18 (our minimum supported version).
• Incorrect alignment of i128 on 64-bit PowerPC, SPARC, and MIPS 2: Rust's data layouts adjusted at llvm: Match new LLVM 128-bit integer alignment on sparc #132422, Update mips64 data layout to match LLVM 20 change #132741, rustc_target: ppc64 target string fixes for LLVM 20 #134115.
• LLVM version of the above: done in LLVM 20 LLVM data layout for i128 doesn't match Clang's alignment of __int128_t on 64-bit PowerPC, 64-bit SPARC and 64-bit MIPS llvm/llvm-project#102783.
• Incorrect return convention of i128 on Windows: adjusted to match GCC and Clang at Windows x86: Change i128 to return via the vector ABI #134290.

At rust-lang/lang-team#255 (comment), the lang team considered it acceptable to remove i128 from improper_ctypes_definitions if the LLVM version is known to be compatible. Time has elapsed since then and we have dropped support for LLVM versions that do not have the x86 fixes, meaning a per-llvm-version lint should no longer be necessary. The PowerPC, SPARC, and MIPS changes only came in LLVM 20 but since Rust's datalayouts have also been updated to match, we will be using the correct alignment regardless of LLVM version.

repr(i128) was added to this lint in #138282, but is also removed here.

Part of the decision is that i128 should match __int128 in C on platforms that provide it, which documentation is updated to indicate. We will not guarantee that i128 matches _BitInt(128) since that can be different from __int128. Some platforms (usually 32-bit) do not provide __int128; if any ABIs are extended in the future to define it, we will need to make sure that our ABI matches.

Closes: #134288

[rustbot]

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[tgross35]

I am not aware of any other incompatibilities at this time, @beetrees is there anything outstanding?

I believe this needs a lang FCP @rust-lang/lang.

@rustbot label +T-lang, +I-lang-nominated, +I-lang-easy-decision

[RalfJung]

Cc @workingjubilee

[programmerjake]

I may be misunderstanding, but it seems like rustc is removing the fixes from the data layout string when it detects LLVM < 20, so unless that's changed to not remove the 128-bit layout from the layout string, rustc would still be broken on LLVM < 20:

rust/compiler/rustc_codegen_llvm/src/context.rs

Lines 168 to 190 in 6d3c050

	if llvm_version < (20, 0, 0) {
	if sess.target.arch == "aarch64" || sess.target.arch.starts_with("arm64") {
	// LLVM 20 defines three additional address spaces for alternate
	// pointer kinds used in Windows.
	// See https://github.com/llvm/llvm-project/pull/111879
	target_data_layout =
	target_data_layout.replace("-p270:32:32-p271:32:32-p272:64:64", "");
	}
	if sess.target.arch.starts_with("sparc") {
	// LLVM 20 updates the sparc layout to correctly align 128 bit integers to 128 bit.
	// See https://github.com/llvm/llvm-project/pull/106951
	target_data_layout = target_data_layout.replace("-i128:128", "");
	}
	if sess.target.arch.starts_with("mips64") {
	// LLVM 20 updates the mips64 layout to correctly align 128 bit integers to 128 bit.
	// See https://github.com/llvm/llvm-project/pull/112084
	target_data_layout = target_data_layout.replace("-i128:128", "");
	}
	if sess.target.arch.starts_with("powerpc64") {
	// LLVM 20 updates the powerpc64 layout to correctly align 128 bit integers to 128 bit.
	// See https://github.com/llvm/llvm-project/pull/118004
	target_data_layout = target_data_layout.replace("-i128:128", "");
	}

[beetrees]

All the sizes and alignments of i128 match __int128 on 64-bit targets; I haven't checked the ABIs in depth but I'm not aware of any incompatibilities with __int128 (it would probably be good to use something like abi-cafe to check the ABIs). _BitInt(128) currently has a different alignment than __int128 on x86-64: it would be good to document somewhere which one Rust's i128 is compatible with.

While 32-bit platforms don't support __int128, they do support _BitInt(128), so if the lint is going to be removed on all platforms (not just 64-bit platforms) then I'd expect i128 and _BitInt(128) to be ABI compatible on 32-bit platforms. Spot checking a few targets now, I've discovered:

• On x86-32, Clang and GCC give _BitInt(128) an alignment of 4 whereas Rust gives i128 an alignment of 16. Additionally, Rust and GCC pass i128/_BitInt(128) directly, whereas Clang passes _BitInt(128) indirectly.
• On 32-bit PowerPC, Rust passes i128 directly in registers whereas Clang passes _BitInt(128) indirectly.

Additionally, the Clang documentation states:

Note: the ABI for _BitInt(N) is still in the process of being stabilized, so this type should not yet be used in interfaces that require ABI stability.

[programmerjake]

_BitInt(128) currently has a different alignment than __int128 on x86-64: it would be good to document somewhere which one Rust's i128 is compatible with.

ideally x86-64 would fix its broken ABIs so __int128 and _BitInt(128) have the same ABI.

see https://gitlab.com/x86-psABIs/x86-64-ABI/-/issues/11

[beetrees]

I may be misunderstanding, but it seems like rustc is removing the fixes from the data layout string when it detects LLVM < 20, so unless that's changed to not remove the 128-bit layout from the layout string, rustc would still be broken on LLVM < 20:

The alignment is only removed when passing the layout to LLVM, so this will only matter if LLVM uses the alignment itself for e.g. low-level function call ABI details. I'm not aware of any cases that this happens for the affected targets, but I haven't looked into it in depth so it's possible there are cases where it matters.

[bjorn3]

I would expect i128 arguments that are passed on the stack to be aligned to 16 bytes with the fix and to be aligned to 4 or 8 bytes without the fix. This changes the offset where the argument is passed and thus affects the ABI.

[nikic]

While 32-bit platforms don't support __int128, they do support _BitInt(128), so if the lint is going to be removed on all platforms (not just 64-bit platforms) then I'd expect i128 and _BitInt(128) to be ABI compatible on 32-bit platforms. Spot checking a few targets now, I've discovered:

Given the fact that i128 maps to __int128, but not _BitInt(128) on x86_64, I don't think that mapping it to _BitInt(128) on other platforms is a good idea.

I think it would be preferable to treat i128 as not a proper FFI type on platforms that do not actually specify an ABI for __int128.

[petrochenkov]

r? @workingjubilee

[rustbot]

Could not assign reviewer from: workingjubilee.
User(s) workingjubilee are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

[petrochenkov]

r? @RalfJung

[RalfJung]

Sorry, I don't have the low level ABI / LLVM knowledge to review this. r? @nikic

[workingjubilee]

I don't actually think this will be an easy decision.

[beetrees]

I would expect i128 arguments that are passed on the stack to be aligned to 16 bytes with the fix and to be aligned to 4 or 8 bytes without the fix. This changes the offset where the argument is passed and thus affects the ABI.

I've just checked; 64-bit PowerPC, 64-bit MIPS and 64-bit SPARC all appear to only align i128 to 8 bytes when it is passed as an argument on the stack, even with LLVM 20 (this is consistent with the PowerPC64 ELFv2 ABI specification).

[workingjubilee]

This FIXME does not seem to be addressed for 64-bit Power:

rust/compiler/rustc_target/src/callconv/powerpc64.rs

Lines 1 to 3 in a18bd8a

	// FIXME:
	// Alignment of 128 bit types is not currently handled, this will
	// need to be fixed when PowerPC vector support is added.

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[tgross35]

To be clear; this PR removes the lint on all targets and does not add a check against a list of known good/problematic targets. Is that what is desired, or is there still interest in the target-specific lint?

I think it would be a good idea to update the i128 docs to say our i128 is meant to be the same as __int128 on platforms where that exists, but not necessarily with _BitInt(128), which seems to be consensus here. If that sounds reasonable, I'll make that change as well.

[traviscross]

Having looked back over the minutes from our call, my read and recollection is that we were OK with doing this without the target-specific check, so this should be OK to go forward just with the rebase and with the documentation. The theme of our conversation was that it's not worth linting on this any longer if it's working well enough on the platforms with which people are actually likely to use this. We were OK if the lint had some false negatives.

@tgross35 and I talked this over just now and double checked some other details. One thing we discussed is that if x86-32 later added a __int128 for C, we'd probably want to adjust things such that extern "C" fn f(_: i128) {} would match it, and that might affect anyone who was expecting stability across Rust versions for that signature on that platform, but that seems niche enough that simply documenting it should be OK.

I think it would be a good idea to update the i128 docs to say our i128 is meant to be the same as __int128 on platforms where that exists, but not necessarily with _BitInt(128), which seems to be consensus here. If that sounds reasonable, I'll make that change as well.

Good idea.

[tgross35]

Rebased. I also removed the lint against repr(i128) enums that fell into this group and was added in #138282.

Also added a note about ABI compatibility to the primitive docs.

@rustbot ready

[bors]

☔ The latest upstream changes (presumably #141717) made this pull request unmergeable. Please resolve the merge conflicts.

[tgross35]

Rebased. @workingjubilee I think this mostly just needs an ack from you regarding the new documentation; the rest is pretty straightforward.

[workingjubilee]

Thank you, apologies for the latency. This looks good to ship.

@bors r+ rollup

[bors]

📌 Commit 0cba7fb has been approved by workingjubilee

It is now in the queue for this repository.

[workingjubilee]

@bors r=traviscross,workingjubilee

[bors]

💡 This pull request was already approved, no need to approve it again.

• There's another pull request that is currently being tested, blocking this pull request: Merge compiler-builtins as a Josh subtree #141229

[bors]

📌 Commit 0cba7fb has been approved by traviscross,workingjubilee

It is now in the queue for this repository.