Skip to content
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

Several platforms still have incorrectly aligned u128/i128 #128950

Open
beetrees opened this issue Aug 11, 2024 · 10 comments
Open

Several platforms still have incorrectly aligned u128/i128 #128950

beetrees opened this issue Aug 11, 2024 · 10 comments
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-FFI Area: Foreign function interface (FFI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. llvm-fixed-upstream Issue expected to be fixed by the next major LLVM upgrade, or backported fixes O-MIPS Target: MIPS processors O-PowerPC Target: PowerPC processors O-SPARC Target: SPARC processors S-waiting-on-LLVM Status: the compiler-dragon is eepy, can someone get it some tea? T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@beetrees
Copy link
Contributor

beetrees commented Aug 11, 2024

I tried this code:

fn main() {
    dbg!(std::mem::align_of::<u128>());
}

I expected to see this happen: The printed alignment to match the alignment used for __int128_t by GCC and Clang.

Instead, this happened: On 64-bit PowerPC, 64-bit SPARC and 64-bit MIPS, Rust thinks the alignment is 8 whereas GCC and Clang think the alignment is 16. The PowerPC 64-bit ABI specifications (both ELFv1 and ELFv2) agree with GCC and Clang (I'm not aware of any specification for 128-bit integers on SPARC64 or MIPS64, but GCC/Clang's behaviour seems to be the de-facto standard). This is because the LLVM data layout for the affected platforms doesn't correctly specify the alignment. This is the same as #54341 but on different architectures (cc rust-lang/lang-team#255). I initially discovered this when running abi-cafe on PowerPC64 to test #128643. I've filed an LLVM bug at llvm/llvm-project#102783.

Meta

rustc --version --verbose:

rustc 1.82.0-nightly (ca5d25e2c 2024-08-09)
binary: rustc
commit-hash: ca5d25e2c41f5a6b4ce65c681bf2f94c7ead1f14
commit-date: 2024-08-09
host: x86_64-unknown-linux-gnu
release: 1.82.0-nightly
LLVM version: 19.1.0
@beetrees beetrees added the C-bug Category: This is a bug. label Aug 11, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 11, 2024
@beetrees
Copy link
Contributor Author

@rustbot label +A-abi +A-ffi +A-llvm +T-compiler

@rustbot rustbot added A-ABI Area: Concerning the application binary interface (ABI) A-FFI Area: Foreign function interface (FFI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 11, 2024
@jieyouxu jieyouxu added O-MIPS Target: MIPS processors O-PowerPC Target: PowerPC processors O-SPARC Target: SPARC processors S-waiting-on-LLVM Status: the compiler-dragon is eepy, can someone get it some tea? and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 13, 2024
@tgross35
Copy link
Contributor

For reference: https://godbolt.org/z/jvrxv7jP7

koachan added a commit to llvm/llvm-project that referenced this issue Sep 30, 2024
Align i128s to 16 bytes, following the example at
https://reviews.llvm.org/D86310.

clang already does this implicitly, but do it in backend code too for
the benefit of other frontends (see e.g
#102783 &
rust-lang/rust#128950).
puja2196 pushed a commit to puja2196/LLVM-tutorial that referenced this issue Sep 30, 2024
Align i128s to 16 bytes, following the example at
https://reviews.llvm.org/D86310.

clang already does this implicitly, but do it in backend code too for
the benefit of other frontends (see e.g
llvm/llvm-project#102783 &
rust-lang/rust#128950).
puja2196 pushed a commit to puja2196/LLVM-tutorial that referenced this issue Oct 2, 2024
Align i128s to 16 bytes, following the example at
https://reviews.llvm.org/D86310.

clang already does this implicitly, but do it in backend code too for
the benefit of other frontends (see e.g
llvm/llvm-project#102783 &
rust-lang/rust#128950).
xgupta pushed a commit to xgupta/llvm-project that referenced this issue Oct 4, 2024
Align i128s to 16 bytes, following the example at
https://reviews.llvm.org/D86310.

clang already does this implicitly, but do it in backend code too for
the benefit of other frontends (see e.g
llvm#102783 &
rust-lang/rust#128950).
@beetrees
Copy link
Contributor Author

Fixed in LLVM 20.

@rustbot label +llvm-fixed-upstream

@rustbot rustbot added the llvm-fixed-upstream Issue expected to be fixed by the next major LLVM upgrade, or backported fixes label Dec 10, 2024
@bjorn3
Copy link
Member

bjorn3 commented Dec 10, 2024

We still need to change the data-layout on the rustc side.

@beetrees
Copy link
Contributor Author

The data layouts have now all been updated in rustc (SPARC, MIPS, PowerPC).

@tgross35
Copy link
Contributor

What else is left to do for this issue?

@tgross35
Copy link
Contributor

Windows has an issue with its i128 ABI, I don't think it's tracked anywhere (outside of an issue in the lang team repo) so maybe this issue could be repurposed.

@beetrees
Copy link
Contributor Author

This issue is just waiting on LLVM 20 now. Regarding the Windows issue: I'm guessing your referring to rust-lang/lang-team#255 (comment)? It's not really the same issue as this (alignment mismatch vs return ABI mismatch), so it seems better to open a separate rust-lang/rust issue about it so it can be tracked separately. Both x86_64-pc-windows-gnu and x86_64-pc-windows-msvc targets on Clang and MinGW GCC seem to agree that i128 is returned in xmm0, and compiler-builtins even has a special hack to use make sure u128/i128 are returned in xmm0 so that the builtins ABI is what LLVM expects.

@tgross35
Copy link
Contributor

Right, I forgot we are waiting on a LLVM upgrade. Regarding Windows, Wesley thinks that Clang is in the wrong here on the MSVC targets https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/MSVC.20.60f16.60.20and.20.60f128.60.20ABI/near/480542167.

@tgross35
Copy link
Contributor

Opened #134288 for the Windows issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-FFI Area: Foreign function interface (FFI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. llvm-fixed-upstream Issue expected to be fixed by the next major LLVM upgrade, or backported fixes O-MIPS Target: MIPS processors O-PowerPC Target: PowerPC processors O-SPARC Target: SPARC processors S-waiting-on-LLVM Status: the compiler-dragon is eepy, can someone get it some tea? T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants