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

Fix #70767 #74837

Merged
merged 2 commits into from
Jul 29, 2020
Merged

Fix #70767 #74837

merged 2 commits into from
Jul 29, 2020

Conversation

xldenis
Copy link
Contributor

@xldenis xldenis commented Jul 27, 2020 •
edited
Loading

This PR changes the format of MIR dump filenames to include the crate name rather than rustc at the start.

As a result, we can now place mir-opt tests in the same directory as the source files, like with UI tests. I had to make sure that compiletest added a bit_width suffix to the expected files when appropriate but otherwise the change is only moving the files to the correct location and ensuring that the EMIT_MIR lines are correct.

Fixes #70767
cc @oli-obk

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 27, 2020
@bors
Copy link
Contributor

bors commented Jul 28, 2020

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

@xldenis xldenis force-pushed the mir-dump-crate-file branch 2 times, most recently from f7ceca8 to 3bc09e5 Compare July 28, 2020 11:23
@xldenis xldenis marked this pull request as ready for review July 28, 2020 11:26
@xldenis
Copy link
Contributor Author

xldenis commented Jul 28, 2020

ah seems like i didn't generate the 32bit wide expectations. I'll have to run bless with a 32bit target i guess

@oli-obk
Copy link
Contributor

oli-obk commented Jul 28, 2020

r? @oli-obk

@xldenis xldenis force-pushed the mir-dump-crate-file branch 4 times, most recently from 0fde18d to 86dd7bd Compare July 28, 2020 13:26
@xldenis
Copy link
Contributor Author

xldenis commented Jul 28, 2020

I wasn't able to run 32 bit tests (mac) but I moved the test expectations manually to the top level so it should be good. We'll see if CI agrees.

@xldenis xldenis force-pushed the mir-dump-crate-file branch 3 times, most recently from 4290676 to 18966f0 Compare July 28, 2020 15:12
@xldenis xldenis force-pushed the mir-dump-crate-file branch 4 times, most recently from 5f684af to 3e968fe Compare July 28, 2020 17:52
@oli-obk
Copy link
Contributor

oli-obk commented Jul 29, 2020

Maybe due to the changed byte offsets in the file due to your comment changes? Did you rebase and re-bless?

@xldenis
Copy link
Contributor Author

xldenis commented Jul 29, 2020

Yea, locally it seems to ignore the test when I bless because I don’t have a profiler set up, I’m still confused why branch CI passed if the test is wrong.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 29, 2020

Oh it's that test -.-. Yea, PR CI doesn't run a profiler either, only bors does that, and only on a single builder. Are you able to set up a profiler or should I try?

@xldenis
Copy link
Contributor Author

xldenis commented Jul 29, 2020 •
edited
Loading

I'm building with profiler support right now :), hopefully that's all i need to do

@xldenis
Copy link
Contributor Author

xldenis commented Jul 29, 2020

Rebuilt and blessed with profiling!

@oli-obk
Copy link
Contributor

oli-obk commented Jul 29, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jul 29, 2020

📌 Commit f07607f 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 Jul 29, 2020
@bors
Copy link
Contributor

bors commented Jul 29, 2020

⌛ Testing commit f07607f with merge af37ac1f38010f0dd2439d552bf56ec22c709bae...

@bors
Copy link
Contributor

bors commented Jul 29, 2020

💔 Test failed - checks-azure

@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 Jul 29, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Jul 29, 2020

@bors retry network timeout

@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 Jul 29, 2020
@bors
Copy link
Contributor

bors commented Jul 29, 2020

⌛ Testing commit f07607f with merge 8611e52...

@bors
Copy link
Contributor

bors commented Jul 29, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: oli-obk
Pushing 8611e52 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 29, 2020
@bors bors merged commit 8611e52 into rust-lang:master Jul 29, 2020
This was referenced Jul 29, 2020
Merged
Merged
Merged
Closed
Merged
@xldenis xldenis deleted the mir-dump-crate-file branch July 29, 2020 23:14
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk left review comments

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.47.0
Development

Successfully merging this pull request may close these issues.

MIR dumps should use "crate_name." as a prefix instead of "rustc.".
6 participants
@xldenis @rust-highfive @bors @oli-obk @cuviper @nikomatsakis