Skip to content

Option<bool>::eq optimizes poorly #139078

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.

Sign up for GitHub

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

Jump to bottom
Closed
scottmcm opened this issue Mar 28, 2025 · 0 comments
Closed

Option<bool>::eq optimizes poorly #139078

scottmcm opened this issue Mar 28, 2025 · 0 comments
Labels
I-heavy Issue: Problems and improvements with respect to binary size of generated code. llvm-fixed-upstream Issue expected to be fixed by the next major LLVM upgrade, or backported fixes S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

Option<bool>::eq is ~20 lines of assembly as of rustc 1.87.0-nightly (a2e63569f 2025-03-26).

Repro: https://rust.godbolt.org/z/hPn8TvvYP

This is here to track this FIXME:

rust/tests/codegen/option-niche-eq.rs

Lines 65 to 73 in 2a06022

// FIXME: This should work too
// // FIXME-CHECK-LABEL: @bool_eq
// #[no_mangle]
// pub fn bool_eq(l: Option<bool>, r: Option<bool>) -> bool {
// // FIXME-CHECK: start:
// // FIXME-CHECK-NEXT: icmp eq i8
// // FIXME-CHECK-NEXT: ret i1
// l == r
// }

With llvm/llvm-project#133344 fixed upstream (thanks @veera-sivarajan for jumping on it so quickly!) after the LLVM 21 version upgrade we should revisit this and see whether we can enable that test.
@scottmcm scottmcm added I-heavy Issue: Problems and improvements with respect to binary size of generated code. llvm-fixed-upstream Issue expected to be fixed by the next major LLVM upgrade, or backported fixes labels Mar 28, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 28, 2025
@scottmcm scottmcm mentioned this issue Mar 28, 2025
Open
@scottmcm scottmcm added S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
I-heavy Issue: Problems and improvements with respect to binary size of generated code. llvm-fixed-upstream Issue expected to be fixed by the next major LLVM upgrade, or backported fixes S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@scottmcm @rustbot