polymorphization: shims and predicates #89514
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
Foreign items do not have bodies and so cannot be polymorphized. Signed-off-by: David Wood <david.wood@huawei.com>
This commit removes all logic which marks parameters as used based on their presence in predicates - given rust-lang#75675, this will enable more polymorphization and avoid the symbol clashes that predicate logic previously sidestepped. Signed-off-by: David Wood <david@davidtw.co>
rustc adds notes to errors which happen post-monomorphization to provide the user with helpful context (as these errors may rely on the specific instantiations). To prevent this note being added where it is not appropriate, the node is checked to originate outwith the current crate. However, when polymorphization is enabled, this can result in some errors (produced by `optimized_mir`) to occur earlier in compilation than they normally would, during the collection of shims. Some shims have ids that originate in the standard library, but these should not receive the PME note, so instances for compiler-generated functions no longer receive this note. Signed-off-by: David Wood <david.wood@huawei.com>
This commit removes the restriction of `InstanceDef::Item` on polymorphization, so that shims can now be polymorphized. Signed-off-by: David Wood <david.wood@huawei.com>
`characteristic_def_id_of_type` was being invoked during partitioning for self types of impl blocks which had projections that depended on the value of unused generic parameters of a function, resulting in an ICE in the 'generic-names' debuginfo test. If partitioning is enabled and the instance needs substitution then this is now skipped. Signed-off-by: David Wood <david.wood@huawei.com>
|
Some changes occured to the CTFE / Miri engine cc @rust-lang/miri |
rust-highfive
added
the
S-waiting-on-review
|
❤️ @bors r+ |
|
📌 Commit b39e915 has been approved by |
bors
added
S-waiting-on-bors
apiraino
added
the
T-compiler
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
Oct 16, 2021
…edicates, r=lcnr polymorphization: shims and predicates Supersedes rust-lang#75737 and rust-lang#75414. This pull request includes up some changes to polymorphization which hadn't landed previously and gets stage2 bootstrapping and the test suite passing when polymorphization is enabled. There are still issues with `type_id` and polymorphization to investigate but this should get polymorphization in a reasonable state to work on. - rust-lang#75737 and rust-lang#75414 both worked but were blocked on having the rest of the test suite pass (with polymorphization enabled) with and without the PRs. It makes more sense to just land these so that the changes are in. - rust-lang#75737's changes remove the restriction of `InstanceDef::Item` on polymorphization, so that shims can now be polymorphized. This won't have much of an effect until polymorphization's analysis is more advanced, but it doesn't hurt. - rust-lang#75414's changes remove all logic which marks parameters as used based on their presence in predicates - given rust-lang#75675, this will enable more polymorphization and avoid the symbol clashes that predicate logic previously sidestepped. - Polymorphization now explicitly checks (and skips) foreign items, this is necessary for stage2 bootstrapping to work when polymorphization is enabled. - The conditional determining the emission of a note adding context to a post-monomorphization error has been modified. Polymorphization results in `optimized_mir` running for shims during collection where that wouldn't happen previously, some errors are emitted during `optimized_mir` and these were considered post-monomorphization errors with the existing logic (more errors and shims have a `DefId` coming from the std crate, not the local crate), adding a note that resulted in tests failing. It isn't particularly feasible to change where polymorphization runs or prevent it from using `optimized_mir`, so it seemed more reasonable to not change the conditional. - `characteristic_def_id_of_type` was being invoked during partitioning for self types of impl blocks which had projections that depended on the value of unused generic parameters of a function - this caused a ICE in a debuginfo test. If partitioning is enabled and the instance needs substitution then this is skipped. That test still fails for me locally, but not with an ICE, but it fails in a fresh checkout too, so 🤷♂️. r? `@lcnr`
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (6f53ddf): comparison url. Summary: This change led to moderate relevant improvements 🎉 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
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.
Supersedes #75737 and #75414. This pull request includes up some changes to polymorphization which hadn't landed previously and gets stage2 bootstrapping and the test suite passing when polymorphization is enabled. There are still issues with
type_idand polymorphization to investigate but this should get polymorphization in a reasonable state to work on.InstanceDef::Itemon polymorphization, so that shims can now be polymorphized. This won't have much of an effect until polymorphization's analysis is more advanced, but it doesn't hurt.optimized_mirrunning for shims during collection where that wouldn't happen previously, some errors are emitted duringoptimized_mirand these were considered post-monomorphization errors with the existing logic (more errors and shims have aDefIdcoming from the std crate, not the local crate), adding a note that resulted in tests failing. It isn't particularly feasible to change where polymorphization runs or prevent it from usingoptimized_mir, so it seemed more reasonable to not change the conditional.characteristic_def_id_of_typewas being invoked during partitioning for self types of impl blocks which had projections that depended on the value of unused generic parameters of a function - this caused a ICE in a debuginfo test. If partitioning is enabled and the instance needs substitution then this is skipped. That test still fails for me locally, but not with an ICE, but it fails in a fresh checkout too, so 🤷♂️.r? @lcnr