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.

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

update Miri #97546

Merged
merged 3 commits into from
May 30, 2022
Merged

update Miri #97546

merged 3 commits into from
May 30, 2022

Conversation

RalfJung
Copy link
Member

First update with the new ui test suite, let's hope this all works. :)
r? @oli-obk

Fixes #97486

@rust-highfive
Copy link
Collaborator

Some changes occured to the Miri submodule

cc @rust-lang/miri

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 30, 2022
@oli-obk
Copy link
Contributor

oli-obk commented May 30, 2022

@bors r+ p=1

@bors
Copy link
Contributor

bors commented May 30, 2022

📌 Commit a272c45 has been approved by oli-obk

@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 May 30, 2022
@bors
Copy link
Contributor

bors commented May 30, 2022

⌛ Testing commit a272c45 with merge 42639118b9f34eed46e9befed92b8138fb7aee53...

@bors
Copy link
Contributor

bors commented May 30, 2022

💔 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 30, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor

oli-obk commented May 30, 2022

Interesting. It looks like a couple of compile-fail tests are not failing miri anymore?

@RalfJung
Copy link
Member Author

RalfJung commented May 30, 2022 via email

@oli-obk
Copy link
Contributor

oli-obk commented May 30, 2022

Maybe they are incorrectly being optimized?

just wondered that, too, and found "-Zmir-opt-level=4" in the logs...

@oli-obk
Copy link
Contributor

oli-obk commented May 30, 2022

Hmm, we only ever ran it with mir opt level 4 apparently, not with zero like our test suite does?

rust/src/bootstrap/test.rs

Line 576 in 855fc02

cargo.env("MIRIFLAGS", "-O -Zmir-opt-level=4");

@oli-obk
Copy link
Contributor

oli-obk commented May 30, 2022

@bors r+

@bors
Copy link
Contributor

bors commented May 30, 2022

📌 Commit 3c66939 has been approved by oli-obk

@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 May 30, 2022
@RalfJung
Copy link
Member Author

Hmm, we only ever ran it with mir opt level 4 apparently, not with zero like our test suite does?

We run it the regular way here:

rust/src/bootstrap/test.rs

Line 571 in 855fc02

if !try_run(builder, &mut cargo) {

RalfJung
RalfJung commented May 30, 2022
View reviewed changes
src/bootstrap/test.rs Outdated
@@ -572,8 +572,6 @@ impl Step for Miri {
return;
}

// # Run `cargo test` with `-Zmir-opt-level=4`.
cargo.env("MIRIFLAGS", "-O -Zmir-opt-level=4");
if !try_run(builder, &mut cargo) {
return;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Now we are just running it twice. :)

@RalfJung
Copy link
Member Author

For now, let's just only run the unoptimized tests here, and leave the optimized tests for Miri CI.

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented May 30, 2022

📌 Commit 962d54e has been approved by oli-obk

@bors
Copy link
Contributor

bors commented May 30, 2022

⌛ Testing commit 962d54e with merge 946a88a...

@bors
Copy link
Contributor

bors commented May 30, 2022

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 30, 2022
@bors bors merged commit 946a88a into rust-lang:master May 30, 2022
@rustbot rustbot added this to the 1.63.0 milestone May 30, 2022
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #97546!

Tested on commit 946a88a.
Direct link to PR: #97546

🎉 miri on windows: build-fail → test-pass (cc @eddyb @oli-obk @RalfJung).
🎉 miri on linux: build-fail → test-pass (cc @eddyb @oli-obk @RalfJung).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request May 30, 2022
@rust-highfive
📣 Toolstate changed by rust-lang/rust#97546!
c6864b1 
Tested on commit rust-lang/rust@946a88a.
Direct link to PR: <rust-lang/rust#97546>

🎉 miri on windows: build-fail → test-pass (cc @eddyb @oli-obk @RalfJung).
🎉 miri on linux: build-fail → test-pass (cc @eddyb @oli-obk @RalfJung).
@bors bors mentioned this pull request May 30, 2022
Merged
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (946a88a): comparison url.

Instruction count

  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)		 0.2% 0.2% 1
Regressions 😿
(secondary)		 1.1% 1.1% 3
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) 0.2% 0.2% 1

Max RSS (memory usage)

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 3.5% 5.8% 4
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 -3.3% -3.5% 2
All 😿🎉 (primary) N/A N/A 0

Cycles

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)		 2.9% 2.9% 1
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -2.1% -2.1% 1
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) 0.4% 2.9% 2

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2 ↩3

  2. number of relevant changes ↩ ↩2 ↩3

@RalfJung RalfJung deleted the miri branch May 30, 2022 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
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
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.

miri no longer builds after rust-lang/rust#97158
7 participants
@RalfJung @rust-highfive @oli-obk @bors @rust-log-analyzer @rust-timer @rustbot