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

Rust's i128 return ABI does not agree with Clang and GCC on Windows targets #134288

Open
tgross35 opened this issue Dec 13, 2024 · 7 comments · May be fixed by #137306
Open

Rust's i128 return ABI does not agree with Clang and GCC on Windows targets #134288

tgross35 opened this issue Dec 13, 2024 · 7 comments · May be fixed by #137306
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tgross35
Copy link
Contributor

Originally reported at rust-lang/lang-team#255 (comment), Clang and GCC-MinGW return __int128 in xmm0. Rust currently returns it on the stack.

@wesleywiser has a proposed patch to make Clang return on the stack for -msvc targets, while keeping the behavior for MinGW wesleywiser/llvm-project@b2f8b83.

Rustc could probably just change the Windows ABI to always return in xmm0 for now, and then if the Clang patch lands we can change this to apply only to MinGW.

Demo: https://godbolt.org/z/o9h8We69o

@tgross35 tgross35 added A-ABI Area: Concerning the application binary interface (ABI) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 13, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 13, 2024
@tgross35 tgross35 removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 13, 2024
tgross35 added a commit to tgross35/rust that referenced this issue Dec 14, 2024
Clang and GCC both return `i128` in xmm0 on Windows, both for MSVC and
MinGW targets. Currently, Rust returns `i128` indirectly. Add a fixup
for return ABI so we also return scalar `i128`s, which should make our
`i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix)
tgross35 added a commit to tgross35/rust that referenced this issue Dec 14, 2024
Clang and GCC both return `i128` in xmm0 on Windows, both for MSVC and
MinGW targets. Currently, Rust returns `i128` indirectly. Add a fixup
for return ABI so we also return scalar `i128`s, which should make our
`i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix)
tgross35 added a commit to tgross35/rust that referenced this issue Dec 14, 2024
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix)
tgross35 added a commit to tgross35/rust that referenced this issue Dec 14, 2024
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix)
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 14, 2024
Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix)

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
tgross35 added a commit to tgross35/rust that referenced this issue Dec 15, 2024
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288
tgross35 added a commit to tgross35/rust that referenced this issue Dec 15, 2024
Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu
but Rust returns the type on the stack. The LLVM backend was changed to
use the same convention as the C compilers; change Cranelift to do the
same.

Link: rust-lang#134288
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 15, 2024
Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix)

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
tgross35 added a commit to tgross35/rust that referenced this issue Dec 17, 2024
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288
tgross35 added a commit to tgross35/rust that referenced this issue Dec 17, 2024
Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu
but Rust returns the type on the stack. The LLVM backend was changed to
use the same convention as the C compilers; change Cranelift libcalls to
do the same.

Link: rust-lang#134288
tgross35 added a commit to tgross35/rust that referenced this issue Dec 17, 2024
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288
tgross35 added a commit to tgross35/rust that referenced this issue Dec 17, 2024
Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu
but Rust returns the type on the stack. The LLVM backend was changed to
use the same convention as the C compilers; change Cranelift libcalls to
do the same.

Link: rust-lang#134288
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 17, 2024
Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix)

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
tgross35 added a commit to tgross35/rust that referenced this issue Dec 22, 2024
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288
tgross35 added a commit to tgross35/rust that referenced this issue Dec 22, 2024
Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu
but Rust returns the type on the stack. The LLVM backend was changed to
use the same convention as the C compilers; change Cranelift libcalls to
do the same.

Link: rust-lang#134288
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 22, 2024
Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix) [1]

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
tgross35 added a commit to tgross35/rust that referenced this issue Jan 15, 2025
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 15, 2025
Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix) [1]

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
tgross35 added a commit to tgross35/rust that referenced this issue Jan 16, 2025
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 17, 2025
…bjorn3,wesleywiser

Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix) [1]

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
tgross35 added a commit to tgross35/rust that referenced this issue Jan 27, 2025
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang#134288
Zalathar added a commit to Zalathar/rust that referenced this issue Jan 28, 2025
…bjorn3,wesleywiser

Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix) [1]

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
Zalathar added a commit to Zalathar/rust that referenced this issue Jan 28, 2025
…bjorn3,wesleywiser

Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix) [1]

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 28, 2025
…orn3,wesleywiser

Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang#134288 (does not fix) [1]

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
@joshtriplett
Copy link
Member

What are the chances that we can get mingw to fix their alignment to match MSVC? The two are supposed to be interoperable.

@tgross35
Copy link
Contributor Author

I think that the Windows story might actually be all set since #134290 merged; the alignment is the same (looking at https://godbolt.org/z/WeEsYTqfP), and MinGW, Clang, and (as of a few minutes ago) Rust all pass i128 on the stack but return in xmm0.

I left this open because @wesleywiser may still plan to submit Clang patch, which is linked in the top post. But maybe that is no longer needed? Unless Microsoft eventually specifies an __int128 ABI, at which point all implementations will need to update. (Or they can just bless passing on the stack and returning in XMM, in which case nobody has to change anything :) )

BoxyUwU pushed a commit to BoxyUwU/rustc-dev-guide that referenced this issue Jan 28, 2025
…eywiser

Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang/rust#134288 (does not fix) [1]

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jan 29, 2025
…eywiser

Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang/rust#134288 (does not fix) [1]

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
@mati865
Copy link
Contributor

mati865 commented Jan 31, 2025

Correct, until MS implements i128 the only way to be compatible with them is to remove i128 on Windows.

@RalfJung
Copy link
Member

We can do whatever we want on our own ABI, but we'd have to remove it from extern "C" functions.

bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this issue Feb 1, 2025
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang/rust#134288
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this issue Feb 1, 2025
…eywiser

Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang/rust#134288 (does not fix) [1]

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
@tgross35
Copy link
Contributor Author

tgross35 commented Feb 4, 2025

I think this could probably be closed, if Clang does wind up changing things then the best we can do is update our end at the same time. @RalfJung are you suggesting that the current situation is not sufficient to remove improper_ctypes for i128 on Windows?

@RalfJung
Copy link
Member

RalfJung commented Feb 4, 2025

I was just replying to @mati865. No strong opinion from my side; I just think there should be an explicit decision by a team (compiler and/or lang) on our stance of whether we can promise to be ABI-compatible with the native C calling convention for a type that the "official" C compiler for that target does not support. The alternative of emitting a warning and eventually a hard error saying "this type cannot be passed by-value via this ABI" also seems quite reasonable to me.

@mati865
Copy link
Contributor

mati865 commented Feb 4, 2025

My intention was only to show that compatibility with MVSC is impossible (I should have quoted Josh's comment there) because their support for 128-bit integers is nonexistent. Therefore, not supporting them (as a C ABI) is the only way to achieve parity. However, that would be a significant downgrade, and the current handling is better.

Also, I think it would be unwise to commit to a specific representation, but this is addressable in the documentation by mentioning the problem with MVSC's lack of decision.

BGluth pushed a commit to BGluth/rustc_codegen_cranelift that referenced this issue Feb 4, 2025
Clang and GCC both return `i128` in xmm0 on windows-msvc and
windows-gnu. Currently, Rust returns the type on the stack. Add a
calling convention adjustment so we also return scalar `i128`s using the
vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its
`-msvc` targets (more at [1]). If this happens, the change here will
need to be adjusted to only affect MinGW.

Link: rust-lang/rust#134288
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Feb 10, 2025
…eywiser

Windows x86: Change i128 to return via the vector ABI

Clang and GCC both return `i128` in xmm0 on windows-msvc and windows-gnu. Currently, Rust returns the type on the stack. Add a calling convention adjustment so we also return scalar `i128`s using the vector ABI, which makes our `i128` compatible with C.

In the future, Clang may change to return `i128` on the stack for its `-msvc` targets (more at [1]). If this happens, the change here will need to be adjusted to only affect MinGW.

Link: rust-lang/rust#134288 (does not fix) [1]

try-job: x86_64-msvc
try-job: x86_64-msvc-ext1
try-job: x86_64-mingw-1
try-job: x86_64-mingw-2
tgross35 added a commit to tgross35/rust that referenced this issue Feb 20, 2025
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 rust-lang/compiler-team#683,
  implemented at rust-lang#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
  rust-lang#132422,
  rust-lang#132741,
  rust-lang#134115.
* LLVM version of the above: done in LLVM 20
  llvm/llvm-project#102783.
* Incorrect return convention of `i128` on Windows: adjusted to match
  GCC and Clang at rust-lang#134290.

At [3], 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.

Closes: rust-lang#134288
Closes: rust-lang#128950

[1]: rust-lang#54341
[2]: rust-lang#128950
[3]: rust-lang/lang-team#255 (comment)
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-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants