Skip to content

Update CmpResult to use a pointer-sized return type #920

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

Merged
merged 1 commit into from
May 28, 2025

Conversation

tgross35
Copy link
Contributor

As seen at 1, LLVM uses long long on LLP64 (to get a 64-bit integer matching pointer size) and long on everything else, with exceptions for AArch64 and AVR. Our current logic always uses an i32. This happens to work because LLVM uses 32-bit instructions to check the output on x86-64, but the GCC checks the full 64-bit register so garbage in the upper half leads to incorrect results.

Update our return type to be isize, with exceptions for AArch64 and AVR.

Fixes: #919

@tgross35 tgross35 force-pushed the cmp-return-type branch 6 times, most recently from e051061 to cb22631 Compare May 28, 2025 04:52
@tgross35 tgross35 enabled auto-merge (rebase) May 28, 2025 04:53
@tgross35 tgross35 force-pushed the cmp-return-type branch 4 times, most recently from 0079f13 to 18d2dbe Compare May 28, 2025 06:21
As seen at [1], LLVM uses `long long` on LLP64 (to get a 64-bit integer
matching pointer size) and `long` on everything else, with exceptions
for AArch64 and AVR. Our current logic always uses an `i32`. This
happens to work because LLVM uses 32-bit instructions to check the
output on x86-64, but the GCC checks the full 64-bit register so garbage
in the upper half leads to incorrect results.

Update our return type to be `isize`, with exceptions for AArch64 and
AVR.

Fixes: rust-lang#919

[1]: https://github.com/llvm/llvm-project/blob/0cf3c437c18ed27d9663d87804a9a15ff6874af2/compiler-rt/lib/builtins/fp_compare_impl.inc#L11-L27
@tgross35 tgross35 merged commit 7365ea4 into rust-lang:master May 28, 2025
35 checks passed
@tgross35 tgross35 deleted the cmp-return-type branch May 28, 2025 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Float CmpResult seems to be of the wrong type
1 participant