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

Simplify MIR opt tests #100827

Merged
merged 1 commit into from
Sep 2, 2022
Merged

Simplify MIR opt tests #100827

merged 1 commit into from
Sep 2, 2022

Conversation

JakobDegen
Copy link
Contributor

This commit removes many cases of MIR opt tests emitting .diffs for more than one pass. These tests cannot be unit-tests, and so they are easy to break, and they also provide little value due to having excessively strong opinions over how a piece of code should be optimized.

Where reasonable, we instead add separate test files that only emit the PreCodegen.after MIR for code where we want to track what the end to end effect of the optimization pipeline on the example code is.

r? @wesleywiser

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

bors commented Aug 22, 2022

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

@wesleywiser
Copy link
Member

Thanks @JakobDegen! I think creating specific end-to-end tests was a good idea and I'm glad to see the mix of unit tests and e2e tests.

r=me when rebased

@rust-log-analyzer

This comment has been minimized.

@JakobDegen
Copy link
Contributor Author

@wesleywiser should be ready if CI passes

@wesleywiser
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 31, 2022

📌 Commit 4a33bf3 has been approved by wesleywiser

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 Aug 31, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 31, 2022
…iser

Simplify MIR opt tests

This commit removes many cases of MIR opt tests emitting `.diff`s for more than one pass. These tests cannot be `unit-test`s, and so they are easy to break, and they also provide little value due to having excessively strong opinions over *how* a piece of code should be optimized.

Where reasonable, we instead add separate test files that only emit the `PreCodegen.after` MIR for code where we want to track what the end to end effect of the optimization pipeline on the example code is.

r? `@wesleywiser`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 31, 2022
…iser

Simplify MIR opt tests

This commit removes many cases of MIR opt tests emitting `.diff`s for more than one pass. These tests cannot be `unit-test`s, and so they are easy to break, and they also provide little value due to having excessively strong opinions over *how* a piece of code should be optimized.

Where reasonable, we instead add separate test files that only emit the `PreCodegen.after` MIR for code where we want to track what the end to end effect of the optimization pipeline on the example code is.

r? ``@wesleywiser``
@matthiaskrgr
Copy link
Member

I think this failed in a rollup but I'm not 100% sure since I could not find the offending file changed in the rollup, maybe it was added in the meantime? Could you please have a look? Thanks! ❤️
#101246 (comment)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 1, 2022
…iser

Simplify MIR opt tests

This commit removes many cases of MIR opt tests emitting `.diff`s for more than one pass. These tests cannot be `unit-test`s, and so they are easy to break, and they also provide little value due to having excessively strong opinions over *how* a piece of code should be optimized.

Where reasonable, we instead add separate test files that only emit the `PreCodegen.after` MIR for code where we want to track what the end to end effect of the optimization pipeline on the example code is.

r? `@wesleywiser`
@Dylan-DPC
Copy link
Member

Dylan-DPC commented Sep 1, 2022

failed in rollup

@bors r-

(oops just checked the previous comment :P )

@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 Sep 1, 2022
This commit removes many cases of MIR opt tests emitting `.diff`s for more than one pass. These
tests cannot be `unit-test`s, and so they are easy to break, and they also provide little value due
to having excessively strong opinions over *how* a piece of code should be optimized.

Where reasonable, we instead add separate test files that only emit the `PreCodegen.after` MIR for
code where we want to track what the result of the net result of the optimization pipeline's output
is.
@JakobDegen
Copy link
Contributor Author

Added a // ignore-wasm32. The relevant difference in behavior was in the AbortUnwindingCalls pass

@Dylan-DPC
Copy link
Member

@bors r=wesleywiser

@bors
Copy link
Contributor

bors commented Sep 2, 2022

📌 Commit e5d60af has been approved by wesleywiser

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 Sep 2, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…iser

Simplify MIR opt tests

This commit removes many cases of MIR opt tests emitting `.diff`s for more than one pass. These tests cannot be `unit-test`s, and so they are easy to break, and they also provide little value due to having excessively strong opinions over *how* a piece of code should be optimized.

Where reasonable, we instead add separate test files that only emit the `PreCodegen.after` MIR for code where we want to track what the end to end effect of the optimization pipeline on the example code is.

r? `@wesleywiser`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…iser

Simplify MIR opt tests

This commit removes many cases of MIR opt tests emitting `.diff`s for more than one pass. These tests cannot be `unit-test`s, and so they are easy to break, and they also provide little value due to having excessively strong opinions over *how* a piece of code should be optimized.

Where reasonable, we instead add separate test files that only emit the `PreCodegen.after` MIR for code where we want to track what the end to end effect of the optimization pipeline on the example code is.

r? ``@wesleywiser``
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…iser

Simplify MIR opt tests

This commit removes many cases of MIR opt tests emitting `.diff`s for more than one pass. These tests cannot be `unit-test`s, and so they are easy to break, and they also provide little value due to having excessively strong opinions over *how* a piece of code should be optimized.

Where reasonable, we instead add separate test files that only emit the `PreCodegen.after` MIR for code where we want to track what the end to end effect of the optimization pipeline on the example code is.

r? ```@wesleywiser```
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 2, 2022
…iser

Simplify MIR opt tests

This commit removes many cases of MIR opt tests emitting `.diff`s for more than one pass. These tests cannot be `unit-test`s, and so they are easy to break, and they also provide little value due to having excessively strong opinions over *how* a piece of code should be optimized.

Where reasonable, we instead add separate test files that only emit the `PreCodegen.after` MIR for code where we want to track what the end to end effect of the optimization pipeline on the example code is.

r? ````@wesleywiser````
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 2, 2022
…llaumeGomez

Rollup of 9 pull requests

Successful merges:

 - rust-lang#97739 (Uplift the `let_underscore` lints from clippy into rustc.)
 - rust-lang#99583 (Add additional methods to the Demand type)
 - rust-lang#100147 (optimization of access level table construction)
 - rust-lang#100552 (rustc_target: Add a compatibility layer to separate internal and user-facing linker flavors)
 - rust-lang#100827 (Simplify MIR opt tests)
 - rust-lang#101166 (Generate error index with mdbook instead of raw HTML pages)
 - rust-lang#101294 (Fix rust-lang#100844 rebase accident)
 - rust-lang#101298 (rustdoc: remove unused CSS `#main-content > .since`)
 - rust-lang#101304 (Add autolabels for `A-query-system`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 203526e into rust-lang:master Sep 2, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 2, 2022
@JakobDegen JakobDegen deleted the better-tests branch September 5, 2022 00:41
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 7, 2022
…ywiser

Remove unnecessary `EMIT_MIR_FOR_EACH_BITWIDTH`

This commit removes the annotation only for those tests where the 32 bit and 64 bit files were exactly identical. I didn't touch anything in the `mir-opt/const` directory, since having this annotation there seems more principled, even if it doesn't make a difference.

This also removes four additional files related to the `separate_const_switch.rs` test. The associated annotations were removed in rust-lang#100827 , but I forgot to remove the files as well. (rust-lang#97564 is the issue tracking an automated check here)

r? ``@wesleywiser``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 7, 2022
…ywiser

Remove unnecessary `EMIT_MIR_FOR_EACH_BITWIDTH`

This commit removes the annotation only for those tests where the 32 bit and 64 bit files were exactly identical. I didn't touch anything in the `mir-opt/const` directory, since having this annotation there seems more principled, even if it doesn't make a difference.

This also removes four additional files related to the `separate_const_switch.rs` test. The associated annotations were removed in rust-lang#100827 , but I forgot to remove the files as well. (rust-lang#97564 is the issue tracking an automated check here)

r? ```@wesleywiser```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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