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 FileCheck annotations to MIR-opt inlining tests #117029

Merged
merged 29 commits into from
Nov 1, 2023

Conversation

rmehri01
Copy link
Contributor

Part of #116971, adds FileCheck annotations to MIR-opt tests in tests/mir-opt/inline.

I left out a few (such as inline_cycle) where it mentioned that the particular outcome of inlining isn't important, just that the inliner doesn't get stuck in an infinite loop.

r? cjgillot

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 21, 2023
Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

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

Thanks @rmehri01! Looking good.

Copy link
Contributor

Choose a reason for hiding this comment

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

This test verifies that we give the correct unwind block to the inline ASM. Added by #102778
It's only relevant for ASM that unwinds.
Could you:

  • add a // needs-unwind directive at the top;
  • add a CHECK that the asm! terminator has an unwind block, and that unwind block is marked (cleanup)?

tests/mir-opt/inline/cycle.rs Show resolved Hide resolved
tests/mir-opt/inline/cycle.rs Show resolved Hide resolved
tests/mir-opt/inline/dont_ice_on_generic_rust_call.rs Outdated Show resolved Hide resolved
@@ -9,6 +8,8 @@ fn main() {

// EMIT_MIR inline_retag.bar.Inline.after.mir
fn bar() -> bool {
// CHECK-LABEL: fn bar(
// CHECK: (inlined foo)
Copy link
Contributor

Choose a reason for hiding this comment

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

The point of this test is to verify that we put the Retag statements in the correct place. Could you add them here?

tests/mir-opt/inline/inline_retag.rs Show resolved Hide resolved
tests/mir-opt/inline/inline_trait_method.rs Show resolved Hide resolved
tests/mir-opt/inline/inline_trait_method.rs Outdated Show resolved Hide resolved
@rmehri01 rmehri01 force-pushed the mir_opt_filecheck_inline_tests branch from 840e2fd to 1f35769 Compare October 22, 2023 16:32
@cjgillot
Copy link
Contributor

Thanks!
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 23, 2023

📌 Commit 1f35769 has been approved by cjgillot

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 Oct 23, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 24, 2023
…tests, r=cjgillot

Add FileCheck annotations to MIR-opt inlining tests

Part of rust-lang#116971, adds FileCheck annotations to MIR-opt tests in `tests/mir-opt/inline`.

I left out a few (such as `inline_cycle`) where it mentioned that the particular outcome of inlining isn't important, just that the inliner doesn't get stuck in an infinite loop.

r? cjgillot
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 24, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#116094 (Introduce `-C instrument-coverage=branch` to gate branch coverage)
 - rust-lang#116396 (Migrate diagnostics in `rustc_hir_analysis/src/coherence/orphan.rs`)
 - rust-lang#116792 (Avoid unnecessary renumbering during borrowck)
 - rust-lang#116943 (Add target features for LoongArch)
 - rust-lang#117010 (Add method to convert internal to stable constructs)
 - rust-lang#117029 (Add FileCheck annotations to MIR-opt inlining tests )

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-
#117125 (comment)

@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 Oct 24, 2023
@bors
Copy link
Contributor

bors commented Oct 25, 2023

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

@workingjubilee
Copy link
Member

@bors delegate+

@bors
Copy link
Contributor

bors commented Oct 31, 2023

✌️ @rmehri01, you can now approve this pull request!

If @workingjubilee told you to "r=me" after making some further change, please make that change, then do @bors r=@workingjubilee

@rmehri01
Copy link
Contributor Author

@bors r=cjgillot rollup

@bors
Copy link
Contributor

bors commented Oct 31, 2023

📌 Commit 2fcb4d9 has been approved by cjgillot

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 Oct 31, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 1, 2023
…tests, r=cjgillot

Add FileCheck annotations to MIR-opt inlining tests

Part of rust-lang#116971, adds FileCheck annotations to MIR-opt tests in `tests/mir-opt/inline`.

I left out a few (such as `inline_cycle`) where it mentioned that the particular outcome of inlining isn't important, just that the inliner doesn't get stuck in an infinite loop.

r? cjgillot
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 1, 2023
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#115626 (Clean up unchecked_math, separate out unchecked_shifts)
 - rust-lang#117029 (Add FileCheck annotations to MIR-opt inlining tests )
 - rust-lang#117397 (Don't emit delayed good-path bugs on panic)
 - rust-lang#117401 (Refactor: move suggestion functions from demand to suggestions)
 - rust-lang#117475 (Inline and remove `create_session`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r- rollup=iffy
#117481 (comment)

@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 Nov 1, 2023
@cjgillot
Copy link
Contributor

cjgillot commented Nov 1, 2023

You need to update the spans in tests/mir-opt/inline/inline_coroutine.main.Inline.panic-abort.diff

@rmehri01
Copy link
Contributor Author

rmehri01 commented Nov 1, 2023

Oh sorry, I think that's because adding the comments messed up the spans when I did --bless? I just moved them to the bottom of the file instead

@rmehri01
Copy link
Contributor Author

rmehri01 commented Nov 1, 2023

Oh wait nevermind I didn't realize that the unwind file was changing but not the abort one 🤦

@rmehri01 rmehri01 force-pushed the mir_opt_filecheck_inline_tests branch from 6a4bb1f to 5f75326 Compare November 1, 2023 14:59
@cjgillot
Copy link
Contributor

cjgillot commented Nov 1, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Nov 1, 2023

📌 Commit 5f75326 has been approved by cjgillot

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

bors commented Nov 1, 2023

⌛ Testing commit 5f75326 with merge 75b064d...

@bors
Copy link
Contributor

bors commented Nov 1, 2023

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 75b064d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 1, 2023
@bors bors merged commit 75b064d into rust-lang:master Nov 1, 2023
12 checks passed
@rustbot rustbot added this to the 1.75.0 milestone Nov 1, 2023
@rmehri01 rmehri01 deleted the mir_opt_filecheck_inline_tests branch November 1, 2023 22:42
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (75b064d): 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)
2.4% [2.4%, 2.4%] 1
Improvements ✅
(primary)
-4.0% [-4.0%, -4.0%] 1
Improvements ✅
(secondary)
-3.0% [-3.4%, -2.2%] 3
All ❌✅ (primary) -4.0% [-4.0%, -4.0%] 1

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: 638.942s -> 637.908s (-0.16%)
Artifact size: 304.49 MiB -> 304.52 MiB (0.01%)

bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Nov 2, 2023
80: Automated pull from upstream `master` r=tshepang a=github-actions[bot]


This PR pulls the following changes from the upstream repository:

* rust-lang/rust#117498
  * rust-lang/rust#117488
  * rust-lang/rust#117441
  * rust-lang/rust#117373
  * rust-lang/rust#117298
* rust-lang/rust#117029
* rust-lang/rust#117289
* rust-lang/rust#117307
* rust-lang/rust#114208
* rust-lang/rust#117482
  * rust-lang/rust#117475
  * rust-lang/rust#117401
  * rust-lang/rust#117397
  * rust-lang/rust#115626
* rust-lang/rust#117436
* rust-lang/rust#115356
* rust-lang/rust#117422
* rust-lang/rust#116692



Co-authored-by: David CARLIER <devnexen@gmail.com>
Co-authored-by: Taiki Endo <te316e89@gmail.com>
Co-authored-by: ltdk <usr@ltdk.xyz>
Co-authored-by: Ryan Mehri <ryan.mehri1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

7 participants