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

Replace ZST operands and debuginfo by constants. #107270

Merged
merged 4 commits into from
Mar 16, 2023

Conversation

cjgillot
Copy link
Contributor

This is work that ConstProp will not have to do.
Split from #107267

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2023

r? @Nilstrieb

(rustbot has picked a reviewer for you, use r? to override)

@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 Jan 24, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Copy link
Member

@Noratrieb Noratrieb left a comment

Choose a reason for hiding this comment

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

The change looks good, but I'll give it over to someone more familiar with MIR opts for a second look

r? mir-opt

compiler/rustc_mir_transform/src/remove_zsts.rs Outdated Show resolved Hide resolved
@rustbot rustbot assigned nagisa and oli-obk and unassigned Noratrieb Jan 24, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jan 25, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jan 25, 2023

📌 Commit f922c5f223e560470ce1dab4218f91acd5a99f67 has been approved by oli-obk

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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 25, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jan 26, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jan 26, 2023

📌 Commit 49f4399 has been approved by oli-obk

It is now in the queue for this repository.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2023
Replace ZST operands and debuginfo by constants.

This is work that ConstProp will not have to do.
Split from rust-lang#107267
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2023
Replace ZST operands and debuginfo by constants.

This is work that ConstProp will not have to do.
Split from rust-lang#107267
@matthiaskrgr
Copy link
Member

I think this failed in a rollup? #107327
@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 26, 2023
@cjgillot
Copy link
Contributor Author

Rebased and blessed the modified test.
@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Jan 27, 2023

📌 Commit 67270568b5874935958142351802bb0c6e64aeec has been approved by oli-obk

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 27, 2023
@cjgillot cjgillot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 25, 2023
@bors
Copy link
Contributor

bors commented Mar 13, 2023

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

@oli-obk
Copy link
Contributor

oli-obk commented Mar 16, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Mar 16, 2023

📌 Commit e8afb08 has been approved by oli-obk

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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 16, 2023
@bors
Copy link
Contributor

bors commented Mar 16, 2023

⌛ Testing commit e8afb08 with merge e386217...

@bors
Copy link
Contributor

bors commented Mar 16, 2023

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing e386217 to master...

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e386217): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.2% [0.4%, 1.9%] 4
Regressions ❌
(secondary)
0.7% [0.7%, 0.7%] 1
Improvements ✅
(primary)
-0.5% [-0.6%, -0.3%] 3
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.5% [-0.6%, 1.9%] 7

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
3.6% [0.0%, 9.3%] 8
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.2% [-2.4%, -0.5%] 3
Improvements ✅
(secondary)
-4.4% [-4.4%, -4.4%] 1
All ❌✅ (primary) 2.3% [-2.4%, 9.3%] 11

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.1% [4.0%, 4.2%] 2
Improvements ✅
(primary)
-0.5% [-0.5%, -0.4%] 3
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.5% [-0.5%, -0.4%] 3

@rustbot rustbot added the perf-regression Performance regression. label Mar 17, 2023
@cjgillot cjgillot deleted the remove-zst branch March 18, 2023 10:02
@cjgillot
Copy link
Contributor Author

Perf report:

  • on instruction count: 3 regressions >1% on optimized builds, to balance with many small improvements. On average, +0.14% regression and -0.22% improvements;
  • improvement on crate metadata size, on average -0.15% up to 2% ;
  • likewise on binary size (-0.22% up to 1%).

@nnethercote
Copy link
Contributor

Yes, it's a mix of results, but overall the wins exceed the losses.

@rustbot label: +perf-regression-triaged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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.

None yet

10 participants