Skip to content

Removes SimplifyComparisonIntegral pass - #150940

Draft
dianqk wants to merge 2 commits into
rust-lang:mainfrom
dianqk:rm-simplify_comparison_integral
Draft

Removes SimplifyComparisonIntegral pass#150940
dianqk wants to merge 2 commits into
rust-lang:mainfrom
dianqk:rm-simplify_comparison_integral

Conversation

@dianqk

Copy link
Copy Markdown
Member

r? ghost

@rustbotrustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 10, 2026
@dianqk

Copy link
Copy Markdown
MemberAuthor

@bors try parent=f57eac1bf98cb5d578e3364b64365ec398c137df @rust-timer queue

@rust-timer

This comment has been minimized.

rust-borsBot added a commit that referenced this pull request Jan 10, 2026
@rust-bors

This comment has been minimized.

@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 10, 2026
@rust-log-analyzer

This comment has been minimized.

@rust-bors

rust-borsBot commented Jan 11, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 19560ab (19560aba9e5ccbe2ecbbf3fe8a9be7590948867f, parent: f57eac1bf98cb5d578e3364b64365ec398c137df)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (19560ab): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

meanrangecount
Regressions ❌
(primary)
0.7%[0.2%, 2.7%]16
Regressions ❌
(secondary)
0.5%[0.2%, 1.6%]12
Improvements ✅
(primary)
-0.4%[-0.7%, -0.2%]3
Improvements ✅
(secondary)
-0.4%[-0.5%, -0.2%]4
All ❌✅ (primary)0.5%[-0.7%, 2.7%]19

Max RSS (memory usage)

Results (primary 0.3%, secondary -2.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
4.6%[2.6%, 7.6%]3
Regressions ❌
(secondary)
--0
Improvements ✅
(primary)
-4.1%[-7.4%, -1.3%]3
Improvements ✅
(secondary)
-2.2%[-2.2%, -2.2%]1
All ❌✅ (primary)0.3%[-7.4%, 7.6%]6

Cycles

Results (primary 2.9%, secondary 2.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
2.9%[2.9%, 2.9%]1
Regressions ❌
(secondary)
2.5%[2.3%, 3.0%]3
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
--0
All ❌✅ (primary)2.9%[2.9%, 2.9%]1

Binary size

Results (primary 0.1%, secondary 0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
0.2%[0.0%, 0.8%]38
Regressions ❌
(secondary)
0.6%[0.0%, 5.0%]17
Improvements ✅
(primary)
-0.3%[-0.7%, -0.0%]6
Improvements ✅
(secondary)
-0.7%[-0.7%, -0.7%]2
All ❌✅ (primary)0.1%[-0.7%, 0.8%]44

Bootstrap: 473.812s -> 473.164s (-0.14%)
Artifact size: 391.34 MiB -> 391.20 MiB (-0.04%)

@rustbotrustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 11, 2026
@saethlin

Copy link
Copy Markdown
Member

That is very strange. The breakdown for the biggest check regression just has a bunch of improvements in it.

@rust-bors

This comment has been minimized.

@mati865

Copy link
Copy Markdown
Member

That is very strange. The breakdown for the biggest check regression just has a bunch of improvements in it.

IIRC, the breakdown and overall stats are two distinct invocations (with and without self-profile). So, for noisy benchmarks you can find situations like this.

@saethlin

Copy link
Copy Markdown
Member

Yes, that's why I commented on the benchmark with a significance factor of 22.

@dianqk
dianqkforce-pushed the rm-simplify_comparison_integral branch from b2d9dc5 to 3d407ddCompareJanuary 17, 2026 11:37
@dianqk
dianqkforce-pushed the rm-simplify_comparison_integral branch from 3d407dd to ae3a7dbCompareJanuary 17, 2026 11:40
@dianqk

Copy link
Copy Markdown
MemberAuthor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-borsBot pushed a commit that referenced this pull request Jan 17, 2026
@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 17, 2026
@rust-bors

rust-borsBot commented Jan 17, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: d51a4e0 (d51a4e0e80aab2cfdb4d4f594f53fb3166c1e68a, parent: 844f13103a64a898897174b19811b3bfb093dc53)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (d51a4e0): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

meanrangecount
Regressions ❌
(primary)
0.6%[0.2%, 2.3%]13
Regressions ❌
(secondary)
0.4%[0.1%, 1.3%]11
Improvements ✅
(primary)
-0.3%[-0.5%, -0.2%]7
Improvements ✅
(secondary)
-0.3%[-0.5%, -0.0%]11
All ❌✅ (primary)0.3%[-0.5%, 2.3%]20

Max RSS (memory usage)

Results (primary 1.9%, secondary 1.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
5.8%[2.0%, 13.2%]4
Regressions ❌
(secondary)
3.5%[1.4%, 5.0%]8
Improvements ✅
(primary)
-3.3%[-4.0%, -2.9%]3
Improvements ✅
(secondary)
-2.3%[-3.6%, -1.4%]4
All ❌✅ (primary)1.9%[-4.0%, 13.2%]7

Cycles

Results (primary 2.9%, secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
2.9%[2.9%, 2.9%]1
Regressions ❌
(secondary)
3.5%[2.0%, 4.4%]6
Improvements ✅
(primary)
--0
Improvements ✅
(secondary)
-5.5%[-9.2%, -2.3%]4
All ❌✅ (primary)2.9%[2.9%, 2.9%]1

Binary size

Results (primary 0.1%, secondary 0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

meanrangecount
Regressions ❌
(primary)
0.2%[0.0%, 0.6%]73
Regressions ❌
(secondary)
0.4%[0.0%, 5.0%]77
Improvements ✅
(primary)
-0.2%[-0.7%, -0.0%]12
Improvements ✅
(secondary)
-0.5%[-0.7%, -0.0%]3
All ❌✅ (primary)0.1%[-0.7%, 0.6%]85

Bootstrap: 472.032s -> 472.955s (0.20%)
Artifact size: 383.50 MiB -> 383.42 MiB (-0.02%)

@rustbotrustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 17, 2026
@rust-bors

rust-borsBot commented Jan 20, 2026

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #151381) made this pull request unmergeable. Please resolve the merge conflicts.

@dianqk

Copy link
Copy Markdown
MemberAuthor

AFAIK, rustc_codegen_ssa has emitted the canonical form for LLVM:

if switch_ty == bx.tcx().types.bool{
// Don't generate trivial icmps when switching on bool.
match test_value {
0 => {
let expect = expect.map(|e| !e);
bx.cond_br_with_expect(discr_value, llotherwise, lltarget, expect);
}
1 => {
bx.cond_br_with_expect(discr_value, lltarget, llotherwise, expect);
}
_ => bug!(),
}
}else{
let switch_llty = bx.immediate_backend_type(bx.layout_of(switch_ty));
let llval = bx.const_uint_big(switch_llty, test_value);
let cmp = bx.icmp(IntPredicate::IntEQ, discr_value, llval);
bx.cond_br_with_expect(cmp, lltarget, llotherwise, expect);
}
.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-regressionPerformance regression.S-waiting-on-authorStatus: This is awaiting some action (such as code changes or more information) from the author.T-compilerRelevant 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.

6 participants

@dianqk@rust-timer@rust-log-analyzer@saethlin@mati865@rustbot