Skip to content

Conversation

borsakv
Copy link
Member

@borsakv borsakv commented Aug 20, 2025

https://rust.godbolt.org/z/EjxqE8WcT

Fixes #139093

Add a regression test to ensure that comparing Option<Ordering> to
Some(Ordering::Equal) does not trigger unnecessary const promotion
in MIR.

Previously, inlined constants like Some(Ordering::Equal) would get
promoted, leading to more complex MIR and redundant LLVM IR checks.
This test verifies that both the direct form and the let-binding form
now generate equivalent, simplified MIR.

r? cjgillot

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 20, 2025
@rustbot

This comment has been minimized.

Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we are testing here is not really the absence of promoted constants. What we want to test is that both versions produce the same optimized MIR.

@cjgillot cjgillot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 10, 2025

⚠️ Warning ⚠️

@cjgillot
Copy link
Contributor

r=me once CI passes

@cjgillot
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 10, 2025

📌 Commit c86474c has been approved by cjgillot

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 10, 2025
bors added a commit that referenced this pull request Oct 10, 2025
Rollup of 12 pull requests

Successful merges:

 - #145651 (Regression test for const promotion with Option<Ordering>)
 - #145722 (implement Extend<{Group, Literal, Punct, Ident}> for TokenStream)
 - #146520 (Promote armv8r-none-eabihf target to Tier 2)
 - #146522 (Promote armv7a-none-eabihf to Tier 2)
 - #147289 (Mitigate `thread_local!` shadowing issues)
 - #147515 (Update rustc-perf submodule)
 - #147522 (compiletest: Use the same directive lines for EarlyProps and ignore/only/needs)
 - #147525 (Replace locals in debuginfo records during ref_prop and dest_prop)
 - #147544 (Remove StatementKind::Deinit.)
 - #147551 (remove `#[rustc_inherit_overflow_checks]` from `is_multiple_of`)
 - #147553 (Move `wasm32-wasip3`  to the tier 3 table)
 - #147562 (Stabilize `NonZero<u*>::div_ceil`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Oct 11, 2025
Rollup of 12 pull requests

Successful merges:

 - #145651 (Regression test for const promotion with Option<Ordering>)
 - #145722 (implement Extend<{Group, Literal, Punct, Ident}> for TokenStream)
 - #146520 (Promote armv8r-none-eabihf target to Tier 2)
 - #146522 (Promote armv7a-none-eabihf to Tier 2)
 - #147289 (Mitigate `thread_local!` shadowing issues)
 - #147515 (Update rustc-perf submodule)
 - #147522 (compiletest: Use the same directive lines for EarlyProps and ignore/only/needs)
 - #147525 (Replace locals in debuginfo records during ref_prop and dest_prop)
 - #147544 (Remove StatementKind::Deinit.)
 - #147551 (remove `#[rustc_inherit_overflow_checks]` from `is_multiple_of`)
 - #147553 (Move `wasm32-wasip3`  to the tier 3 table)
 - #147562 (Stabilize `NonZero<u*>::div_ceil`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 486fae1 into rust-lang:master Oct 11, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Oct 11, 2025
rust-timer added a commit that referenced this pull request Oct 11, 2025
Rollup merge of #145651 - borsakv:139093, r=cjgillot

Regression test for const promotion with Option<Ordering>

https://rust.godbolt.org/z/EjxqE8WcT

Fixes #139093

Add a regression test to ensure that comparing `Option<Ordering>` to
`Some(Ordering::Equal)` does not trigger unnecessary const promotion
in MIR.

Previously, inlined constants like `Some(Ordering::Equal)` would get
promoted, leading to more complex MIR and redundant LLVM IR checks.
This test verifies that both the direct form and the `let`-binding form
now generate equivalent, simplified MIR.

r? cjgillot
rust-cloud-vms bot pushed a commit to makai410/rustc_public that referenced this pull request Oct 12, 2025
Rollup of 12 pull requests

Successful merges:

 - rust-lang/rust#145651 (Regression test for const promotion with Option<Ordering>)
 - rust-lang/rust#145722 (implement Extend<{Group, Literal, Punct, Ident}> for TokenStream)
 - rust-lang/rust#146520 (Promote armv8r-none-eabihf target to Tier 2)
 - rust-lang/rust#146522 (Promote armv7a-none-eabihf to Tier 2)
 - rust-lang/rust#147289 (Mitigate `thread_local!` shadowing issues)
 - rust-lang/rust#147515 (Update rustc-perf submodule)
 - rust-lang/rust#147522 (compiletest: Use the same directive lines for EarlyProps and ignore/only/needs)
 - rust-lang/rust#147525 (Replace locals in debuginfo records during ref_prop and dest_prop)
 - rust-lang/rust#147544 (Remove StatementKind::Deinit.)
 - rust-lang/rust#147551 (remove `#[rustc_inherit_overflow_checks]` from `is_multiple_of`)
 - rust-lang/rust#147553 (Move `wasm32-wasip3`  to the tier 3 table)
 - rust-lang/rust#147562 (Stabilize `NonZero<u*>::div_ceil`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Promoted constants lead to poor MIR from comparisons

4 participants