Clean up MonoItem::instantiation_mode #136394
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
saethlin
force-pushed
the
clean-up-instantiation-mode
branch
from
7ae2e33 to
eea88f5
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 1, 2025
…, r=<try> Clean up MonoItem::instantiation_mode More progress on cleaning up and documenting instantiation mode selection. Tests pass locally, but I want this to pass perf first. r? ghost
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (ab75159): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -3.9%, secondary 1.4%)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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 777.851s -> 777.154s (-0.09%) |
rustbot
removed
the
S-waiting-on-perf
|
r? compiler |
compiler-errors
approved these changes
Feb 1, 2025
|
@bors r+ |
bors
added
S-waiting-on-bors
|
There's a lot of nevers in the queue and this is supposed to no semantic changes and has passed a perf run with no changes. @bors rollup=maybe |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 1, 2025
… r=<try> Remove InstanceKind::generates_cgu_internal_copy This is on top of rust-lang#136394 for now. `InstanceKind::generates_cgu_internal_copy` was called in two places: 1. By `reachable_non_generics`, but in that case the caller was guaranteed to not be generic and therefore we always fall through to just calling `cross_crate_inlinable`. 2. By `MonoItem::instantiation_mode`, but in that case the *only* thing that it does is some very complicated logic for selecting the instantiation mode for drop glue, and only very recently do we have a single codegen test for this. So I've touched up the logic in `cross_crate_inlinable` so that we don't try to claim that `drop_in_place` isn't cross-crate-inlinable, because it clearly is. Now off to perf, and if we get regressions I'll start reintroducing the logic about drop glue but, but this time into `MonoItem::instantiation_mode` where it always should have been.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Feb 2, 2025
…de, r=compiler-errors Clean up MonoItem::instantiation_mode More progress on cleaning up and documenting instantiation mode selection. This should have no behavior changes at all, it just rearranges the code inside `MonoItem::instantiation_mode` to a more logical flow and I've tried to explain every choice the implementation is making.
This was referenced Feb 2, 2025
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 2, 2025
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#134272 (Remove rustc_encodable_decodable feature) - rust-lang#136283 (Update encode_utf16 to mention it is native endian) - rust-lang#136394 (Clean up MonoItem::instantiation_mode) - rust-lang#136402 (diagnostics: fix borrowck suggestions for if/while let conditionals) - rust-lang#136415 (Highlight clarifying information in "expected/found" error) - rust-lang#136422 (Convert two `rustc_middle::lint` functions to `Span` methods.) - rust-lang#136434 (rustc_allowed_through_unstable_modules: require deprecation message) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 2, 2025
Rollup merge of rust-lang#136394 - saethlin:clean-up-instantiation-mode, r=compiler-errors Clean up MonoItem::instantiation_mode More progress on cleaning up and documenting instantiation mode selection. This should have no behavior changes at all, it just rearranges the code inside `MonoItem::instantiation_mode` to a more logical flow and I've tried to explain every choice the implementation is making.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 8, 2025
… r=<try> Remove InstanceKind::generates_cgu_internal_copy This is on top of rust-lang#136394 for now. `InstanceKind::generates_cgu_internal_copy` was called in two places: 1. By `reachable_non_generics`, but in that case the caller was guaranteed to not be generic and therefore we always fall through to just calling `cross_crate_inlinable`. 2. By `MonoItem::instantiation_mode`, but in that case the *only* thing that it does is some very complicated logic for selecting the instantiation mode for drop glue, and only very recently do we have a single codegen test for this. So I've touched up the logic in `cross_crate_inlinable` so that we don't try to claim that `drop_in_place` isn't cross-crate-inlinable, because it clearly is. Now off to perf, and if we get regressions I'll start reintroducing the logic about drop glue but, but this time into `MonoItem::instantiation_mode` where it always should have been.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
More progress on cleaning up and documenting instantiation mode selection.
This should have no behavior changes at all, it just rearranges the code inside
MonoItem::instantiation_modeto a more logical flow and I've tried to explain every choice the implementation is making.