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

--bless all mir-opt tests. #70721

Merged
merged 6 commits into from
Apr 9, 2020
Merged

--bless all mir-opt tests. #70721

merged 6 commits into from
Apr 9, 2020

Conversation

anyska
Copy link
Contributor

@anyska anyska commented Apr 2, 2020

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2020
eddyb
eddyb reviewed Apr 2, 2020
View reviewed changes
src/test/mir-opt/byte_slice/rustc.main.SimplifyCfg-elaborate-drops.after.mir
Comment on lines +44 to +46
alloc0 (size: 3, align: 1) {
66 6f 6f │ foo
}
Copy link
Member

Choose a reason for hiding this comment

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

Interesting outcome: the ty::Const uses this, but it also gets pretty-printed, so the alloc0 isn't strictly necessary.
But at least the super-verbose comments do mention Scalar(alloc0+0), so at least for now it doesn't hurt.

eddyb
eddyb reviewed Apr 2, 2020
View reviewed changes
src/test/mir-opt/graphviz/rustc.main.mir_map.0.dot
Comment on lines +1 to +10
digraph Mir_0_3 {
graph [fontname="monospace"];
node [fontname="monospace"];
edge [fontname="monospace"];
label=<fn main() -&gt; ()<br align="left"/>>;
bb0__0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">0</td></tr><tr><td align="left" balign="left">_0 = ()<br/></td></tr><tr><td align="left">goto</td></tr></table>>];
bb1__0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">1</td></tr><tr><td align="left">resume</td></tr></table>>];
bb2__0_3 [shape="none", label=<<table border="0" cellborder="1" cellspacing="0"><tr><td bgcolor="gray" align="center" colspan="1">2</td></tr><tr><td align="left">return</td></tr></table>>];
bb0__0_3 -> bb2__0_3 [label=""];
}
Copy link
Member

Choose a reason for hiding this comment

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

Whoa! I didn't even know mir-opt tests could test for this.
I now wish GitHub could render this as a graph 😞.

@rust-highfive

This comment has been minimized.

Sign in to view
@anyska anyska force-pushed the bless-all-mir-opt branch 2 times, most recently from 8633383 to 4abacd3 Compare April 2, 2020 23:12
@rust-highfive

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
oli-obk
oli-obk reviewed Apr 3, 2020
View reviewed changes
src/test/mir-opt/retag.rs Outdated Show resolved Hide resolved
oli-obk
oli-obk reviewed Apr 3, 2020
View reviewed changes
src/test/mir-opt/uninhabited_enum_branching.rs
@@ -14,6 +14,8 @@ enum Test2 {
E = 5,
}

// EMIT_MIR rustc.main.UninhabitedEnumBranching.diff
// EMIT_MIR rustc.main.SimplifyCfg-after-uninhabited-enum-branching.after.mir
Copy link
Contributor

Choose a reason for hiding this comment

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

I've seen this pattern a few times. I wonder if we should have a diff mode for "before Optimization X to after optimization Y"

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should always use .diff.

Copy link
Contributor

Choose a reason for hiding this comment

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

What I mean is to diff across multiple optimizations instead of just a single one.

@oli-obk oli-obk 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 Apr 3, 2020
@eddyb

This comment has been minimized.

Sign in to view
@oli-obk

This comment has been minimized.

Sign in to view
@eddyb eddyb mentioned this pull request Apr 4, 2020
Merged
@eddyb

This comment has been minimized.

Sign in to view
@oli-obk

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@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 Apr 4, 2020
@bors

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor

oli-obk commented Apr 8, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Apr 8, 2020

📌 Commit d0e0f40 has been approved by oli-obk

@bors
Copy link
Contributor

bors commented Apr 8, 2020

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@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 Apr 8, 2020
@pietroalbini
Copy link
Member

@bors p=1001

@bors
Copy link
Contributor

bors commented Apr 8, 2020

⌛ Testing commit d0e0f40 with merge cd6b5e409513f9076baea053dabb9b5e69b0e18c...

@bors
Copy link
Contributor

bors commented Apr 8, 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 Apr 8, 2020
@eddyb
Copy link
Member

eddyb commented Apr 8, 2020 •
edited
Loading

Failures:

    [mir-opt] mir-opt/issue-41888.rs
    [mir-opt] mir-opt/match-arm-scopes.rs

I can't easily tell if non-determinism/sensitivity to test conditions is the problem, or a rebase is needed.

EDIT: oh I was missing this (emphasis mine):

Check compiletest suite=mir-opt mode=mir-opt (x86_64-unknown-linux-gnu -> wasm32-unknown-unknown)

EDIT2: likely unwinding-related, we should probably just add this to the two tests:

// ignore-wasm32-bare compiled with panic=abort by default

@eddyb
Copy link
Member

eddyb commented Apr 8, 2020

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Apr 8, 2020

📌 Commit 82d6e07 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 Apr 8, 2020
@Dylan-DPC-zz
Copy link

deprioritising @bors p=1

@bors
Copy link
Contributor

bors commented Apr 8, 2020

⌛ Testing commit 82d6e07 with merge 1edcfc8...

@bors
Copy link
Contributor

bors commented Apr 9, 2020

☀️ Test successful - checks-azure
Approved by: oli-obk
Pushing 1edcfc8 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 9, 2020
@bors bors merged commit 1edcfc8 into rust-lang:master Apr 9, 2020
@bors bors mentioned this pull request Apr 9, 2020
Merged
@eddyb eddyb mentioned this pull request Apr 10, 2020
Merged
@SNCPlay42 SNCPlay42 mentioned this pull request Apr 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wesleywiser wesleywiser wesleywiser left review comments

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

@nikomatsakis nikomatsakis nikomatsakis left review comments

@eddyb eddyb eddyb approved these changes

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
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@anyska @rust-highfive @bors @eddyb @oli-obk @RalfJung @pietroalbini @Dylan-DPC-zz @nikomatsakis @wesleywiser