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

Add a few MIR pre-codegen tests #111553

Merged
merged 10 commits into from
Jun 2, 2023
Merged

Add a few MIR pre-codegen tests #111553

merged 10 commits into from
Jun 2, 2023

Conversation

cjgillot
Copy link
Contributor

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 14, 2023
& (a.storage == b.storage)
}

// Optimizes good and have same semantics as PartialEq
Copy link
Member

Choose a reason for hiding this comment

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

Curiosity: Does this optimize better in LLVM too, or is this talking about MIR?

}
}

pub fn vec_iter(mut v: Vec<impl Sized>) {
Copy link
Member

Choose a reason for hiding this comment

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

This means that the dropping is included; is that intentional? It seems like a distraction from the "loops" part that the filename suggests this cares about.

Also, vec's iter and iter_mut are the same as slice's, so I would suggest removing this part since we already have https://github.com/rust-lang/rust/blob/master/tests/mir-opt/pre-codegen/slice_iter.rs#L27.

}
}

pub fn vec_iter_enumerate(mut v: Vec<impl Sized>) {
Copy link
Member

Choose a reason for hiding this comment

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

Similarly here, how much of the intent is Vec relevant here? The canonical way to write this would be as &mut [impl Sized], so the Vec seems like a distraction.

}
}

pub fn vec_move(mut v: Vec<impl Sized>) {
Copy link
Member

Choose a reason for hiding this comment

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

...since this one is the one where the Vec-ness really does matter.

@scottmcm
Copy link
Member

r=me, with or without the Vec-related tweaks I mentioned in review comments.

@scottmcm scottmcm 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 May 14, 2023
@scottmcm
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 16, 2023

📌 Commit e31c4a41c1f6265690e46733c722034ce6fce8d4 has been approved by scottmcm

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 May 16, 2023
@bors
Copy link
Contributor

bors commented May 17, 2023

⌛ Testing commit e31c4a41c1f6265690e46733c722034ce6fce8d4 with merge 25932a52ef1e0e3e81e9dfcfaa114a1f2ccf9237...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented May 17, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 17, 2023
@scottmcm
Copy link
Member

scottmcm commented May 17, 2023

Looks like a legit failure:
@bors r-

(r=me once blessed or whatever filters needed are added)

@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-review Status: Awaiting review from the assignee but also interested parties. labels May 17, 2023
@cjgillot
Copy link
Contributor Author

@bors r=scottmcm

@bors
Copy link
Contributor

bors commented May 18, 2023

📌 Commit 23e22ef6e600bc1ee5780df9648b27f8a053dddc has been approved by scottmcm

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 May 18, 2023
@bors
Copy link
Contributor

bors commented May 19, 2023

⌛ Testing commit 23e22ef6e600bc1ee5780df9648b27f8a053dddc with merge 0b28caf83947a44e6fcf23a1a40d1cd733506d41...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jun 1, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 1, 2023
@scottmcm
Copy link
Member

scottmcm commented Jun 1, 2023

@bors r+ rollup=iffy (since this keeps running into conflicts with other things, let's try to get it in sooner)

@bors
Copy link
Contributor

bors commented Jun 1, 2023

📌 Commit d796c60 has been approved by scottmcm

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 Jun 1, 2023
@bors
Copy link
Contributor

bors commented Jun 2, 2023

⌛ Testing commit d796c60 with merge 774a3d1...

@bors
Copy link
Contributor

bors commented Jun 2, 2023

☀️ Test successful - checks-actions
Approved by: scottmcm
Pushing 774a3d1 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 2, 2023
@bors bors merged commit 774a3d1 into rust-lang:master Jun 2, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 2, 2023
@cjgillot cjgillot deleted the mir-e2e branch June 2, 2023 06:09
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (774a3d1): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

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)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.2% [-3.2%, -3.1%] 2
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 643.58s -> 644.915s (0.21%)

// compile-flags: -O -Zmir-opt-level=2 -Cdebuginfo=2
// needs-unwind
// ignore-debug
// only-x86_64
Copy link
Member

Choose a reason for hiding this comment

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

Why is this only running on x86, not, say, only-64bit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants