Skip to content

Stop SRoA'ing DynMetadata in MIR #125508

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 26, 2024
Merged

Stop SRoA'ing DynMetadata in MIR #125508

merged 1 commit into from
May 26, 2024

Conversation

scottmcm
Copy link
Member

Fixes #125506

@rustbot
Copy link
Collaborator

rustbot commented May 24, 2024

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 24, 2024
@rustbot
Copy link
Collaborator

rustbot commented May 24, 2024

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@matthiaskrgr
Copy link
Member

test? :3

@@ -70,6 +70,11 @@ fn escaping_locals<'tcx>(
// Exclude #[repr(simd)] types so that they are not de-optimized into an array
return true;
}
if Some(def.did()) == tcx.lang_items().dyn_metadata() {
// codegen wants to see the `DynMetadata<T>`,
Copy link
Member

Choose a reason for hiding this comment

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

i actually think it's about time to investigate remove the layout special casing from dynmetadata and seeing what happens if you just let it be repr(transparent) :3

Copy link
Member Author

@scottmcm scottmcm May 25, 2024

Choose a reason for hiding this comment

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

Tried it out; #125532 (comment)

Looks like the hardest part might be dealing with whether the reference actually has AM memory behind it, and I have no idea how to solve that, so I'll stick with this change for now.

@rust-log-analyzer

This comment has been minimized.

@Noratrieb
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented May 25, 2024

📌 Commit d7248d7 has been approved by Nilstrieb

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 May 25, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 25, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125307 (tidy: stop special-casing tests/ui entry limit)
 - rust-lang#125375 (Create a triagebot ping group for Rust for Linux)
 - rust-lang#125413 (drop region constraints for ambiguous goals)
 - rust-lang#125433 (A small diagnostic improvement for dropping_copy_types)
 - rust-lang#125508 (Stop SRoA'ing `DynMetadata` in MIR)
 - rust-lang#125530 (cleanup dependence of `ExtCtxt` in transcribe when macro expansion)
 - rust-lang#125544 (Also mention my-self for other check-cfg docs changes)
 - rust-lang#125546 (Try to not reinstall tools in mingw CI)

r? `@ghost`
`@rustbot` modify labels: rollup
fmease added a commit to fmease/rust that referenced this pull request May 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#125070 (Panic if `PathBuf::set_extension` would add a path separator)
 - rust-lang#125307 (tidy: stop special-casing tests/ui entry limit)
 - rust-lang#125375 (Create a triagebot ping group for Rust for Linux)
 - rust-lang#125413 (drop region constraints for ambiguous goals)
 - rust-lang#125433 (A small diagnostic improvement for dropping_copy_types)
 - rust-lang#125508 (Stop SRoA'ing `DynMetadata` in MIR)
 - rust-lang#125530 (cleanup dependence of `ExtCtxt` in transcribe when macro expansion)
 - rust-lang#125544 (Also mention my-self for other check-cfg docs changes)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#125070 (Panic if `PathBuf::set_extension` would add a path separator)
 - rust-lang#125307 (tidy: stop special-casing tests/ui entry limit)
 - rust-lang#125375 (Create a triagebot ping group for Rust for Linux)
 - rust-lang#125413 (drop region constraints for ambiguous goals)
 - rust-lang#125433 (A small diagnostic improvement for dropping_copy_types)
 - rust-lang#125508 (Stop SRoA'ing `DynMetadata` in MIR)
 - rust-lang#125530 (cleanup dependence of `ExtCtxt` in transcribe when macro expansion)
 - rust-lang#125544 (Also mention my-self for other check-cfg docs changes)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request May 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#125307 (tidy: stop special-casing tests/ui entry limit)
 - rust-lang#125375 (Create a triagebot ping group for Rust for Linux)
 - rust-lang#125433 (A small diagnostic improvement for dropping_copy_types)
 - rust-lang#125508 (Stop SRoA'ing `DynMetadata` in MIR)
 - rust-lang#125530 (cleanup dependence of `ExtCtxt` in transcribe when macro expansion)
 - rust-lang#125544 (Also mention my-self for other check-cfg docs changes)
 - rust-lang#125546 (Try to not reinstall tools in mingw CI)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125307 (tidy: stop special-casing tests/ui entry limit)
 - rust-lang#125375 (Create a triagebot ping group for Rust for Linux)
 - rust-lang#125413 (drop region constraints for ambiguous goals)
 - rust-lang#125433 (A small diagnostic improvement for dropping_copy_types)
 - rust-lang#125508 (Stop SRoA'ing `DynMetadata` in MIR)
 - rust-lang#125530 (cleanup dependence of `ExtCtxt` in transcribe when macro expansion)
 - rust-lang#125544 (Also mention my-self for other check-cfg docs changes)
 - rust-lang#125559 (Simplify the `unchecked_sh[lr]` ub-checks a bit)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#125307 (tidy: stop special-casing tests/ui entry limit)
 - rust-lang#125375 (Create a triagebot ping group for Rust for Linux)
 - rust-lang#125433 (A small diagnostic improvement for dropping_copy_types)
 - rust-lang#125508 (Stop SRoA'ing `DynMetadata` in MIR)
 - rust-lang#125530 (cleanup dependence of `ExtCtxt` in transcribe when macro expansion)
 - rust-lang#125544 (Also mention my-self for other check-cfg docs changes)
 - rust-lang#125559 (Simplify the `unchecked_sh[lr]` ub-checks a bit)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#125307 (tidy: stop special-casing tests/ui entry limit)
 - rust-lang#125375 (Create a triagebot ping group for Rust for Linux)
 - rust-lang#125473 (fix(opt-dist): respect existing config.toml)
 - rust-lang#125508 (Stop SRoA'ing `DynMetadata` in MIR)
 - rust-lang#125530 (cleanup dependence of `ExtCtxt` in transcribe when macro expansion)
 - rust-lang#125544 (Also mention my-self for other check-cfg docs changes)
 - rust-lang#125559 (Simplify the `unchecked_sh[lr]` ub-checks a bit)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
@bors
Copy link
Collaborator

bors commented May 26, 2024

⌛ Testing commit d7248d7 with merge 8768a6a...

@bors
Copy link
Collaborator

bors commented May 26, 2024

💔 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 26, 2024
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Total Installed Size:  166.93 MiB

:: Proceed with installation? [Y/n] 
:: Retrieving packages...
error: failed retrieving file 'mingw-w64-i686-openssl-3.2.0-1-any.pkg.tar.zst.sig' from mirror.umd.edu : Operation too slow. Less than 1 bytes/sec transferred the last 10 seconds
 mingw-w64-i686-cmake-3.28.1-1-any downloading...
error: failed retrieving file 'mingw-w64-i686-curl-8.5.0-1-any.pkg.tar.zst.sig' from mirror.umd.edu : Operation too slow. Less than 1 bytes/sec transferred the last 10 seconds
error: failed retrieving file 'mingw-w64-i686-gettext-0.22.4-3-any.pkg.tar.zst' from mirror.msys2.org : Operation too slow. Less than 1 bytes/sec transferred the last 10 seconds
warning: failed to retrieve some files
error: failed to commit transaction (unexpected error)
 mingw-w64-i686-openssl-3.2.0-1-any downloading...

@Noratrieb
Copy link
Member

skill issue
@bors retry

@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 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#125307 (tidy: stop special-casing tests/ui entry limit)
 - rust-lang#125375 (Create a triagebot ping group for Rust for Linux)
 - rust-lang#125473 (fix(opt-dist): respect existing config.toml)
 - rust-lang#125508 (Stop SRoA'ing `DynMetadata` in MIR)
 - rust-lang#125561 (Stabilize `slice_flatten`)
 - rust-lang#125571 (f32: use constants instead of reassigning a dummy value as PI)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5fef6c5 into rust-lang:master May 26, 2024
6 of 7 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 26, 2024
Rollup merge of rust-lang#125508 - scottmcm:fix-125506, r=Nilstrieb

Stop SRoA'ing `DynMetadata` in MIR

Fixes rust-lang#125506
@scottmcm scottmcm deleted the fix-125506 branch May 26, 2024 19:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE: You can't project to field 0 of DynMetadata because layout is weird and thinks it doesn't have fields.
7 participants