Description
Context
i128 and u128 have long had the incorrect alignment on x86_64. Correct alignment is 128 bits; however, Rust currently aligns these types to 32 bits due to a bug in LLVM. Clang has gotten around this LLVM bug by manually setting the alignment. This means that Rust cannot safely share an i128 with C, which is the reason for it raising the improper_ctypes_definitions lint.
There is also a second issue in LLVM where i128s are passed incorrectly at function calls when they spill out of registers. Clang did not have a workaround for this, meaning that code compiled by Clang and code compiled by GCC are not compatible when 128-bit integers need to be passed in memory. See rust-lang/rust#54341 for additional context on these problems.
Just recently, the alignment issue was fixed in LLVM (https://reviews.llvm.org/D86310) as was the function call issue (https://reviews.llvm.org/D158169). These fixes should be released in LLVM 18, likely in 2024-Q1.
Proposal
Manually set the alignment of i128 to the correct 16 bytes with LLVM < 18 now, to match the workaround that Clang currently has, and pull D158196 into Rust's LLVM tree. This only needs to be done for x86 targets, essentially a hardcoded version of the layout string changes in https://reviews.llvm.org/D86310#change-9bvPKJH10jZC.
This means the following:
- Users will immediately get i128 compatibility with:
- All versions of GCC
- Clang >= 18
- Compatibility with Clang < 18 for layout, but not for in-memory passing (since D158169 will fix us but old Clang is broken).
i128/u128build files will not be compatible across therustcversion change where this lands- rustc built against LLVM < 18 that isn't from Rust's tree will mostly work with mainline rustc, with the exception of in-memory passing
We can drop this workaround when support for LLVM 17 ends
Alternatives
Just wait for LLVM 18
This means that the same version of rustc would produce incompatible code when built with different LLVM versions.
Don't pull D158196
Adding D158196 means that rustc+LLVM17 will be 100% compatible with rustc+LLVM18, GCC, and Clang18. Not adding it would mean that we have a two step ABI change and a mix of compatibility.
Mentors or Reviewers
@nikic has been helping with the LLVM side
@maurer has a WIP change at rust-lang/rust#116672
Process
The main points of the Major Change Process are as follows:
- File an issue describing the proposal.
- A compiler team member or contributor who is knowledgeable in the area can second by writing
@rustbot second.- Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a
-C flag, then full team check-off is required. - Compiler team members can initiate a check-off via
@rfcbot fcp mergeon either the MCP or the PR.
- Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a
- Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.
You can read more about Major Change Proposals on forge.
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.