Add an (perma-)unstable option to disable vtable vptr #114974
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
This flag is intended for evaluation of trait upcasting space cost for embedded use cases.
|
r? @b-naber (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
| @@ -152,7 +152,7 @@ fn prepare_vtable_segments_inner<'tcx, T>( | |||
| while let Some((inner_most_trait_ref, emit_vptr, mut siblings)) = stack.pop() { | |||
| segment_visitor(VtblSegment::TraitOwnEntries { | |||
| trait_ref: inner_most_trait_ref, | |||
| emit_vptr, | |||
| emit_vptr: emit_vptr && !tcx.sess.opts.unstable_opts.no_trait_vptr, | |||
| })?; | |||
|
|
|||
| // If we've emitted (fed to `segment_visitor`) a trait that has methods present in the vtable, | |||
There was a problem hiding this comment.
Maybe move the && down here? And it could use a comment.
It'll be UB if you upcast to any supertrait that's not the first. |
b-naber
approved these changes
Aug 27, 2023
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 27, 2023
…llaumeGomez Rollup of 3 pull requests Successful merges: - rust-lang#114974 (Add an (perma-)unstable option to disable vtable vptr) - rust-lang#115261 (replace outdated github username 'ozkanonur') - rust-lang#115266 (Unify CSS color formats a bit more) r? `@ghost` `@rustbot` modify labels: rollup
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.
This flag is intended for evaluation of trait upcasting space cost for embedded use cases.
Compared to the approach in #112355, this option provides a way to evaluate end-to-end cost of trait upcasting. Rationale: #112355 (comment)
How this flag should be used (after merge)
Build your project with and without
-Zno-trait-vptrflag. If you are using cargo, setRUSTFLAGS="-Zno-trait-vptr"in the environment variable. You probably also want to use-Zbuild-stdor the binary built may be broken. Save both binaries somewhere.Evaluate the space cost
The option has a direct and indirect impact on vtable space usage. Directly, it gets rid of the trait vptr entry needed to store a pointer to a vtable of a supertrait. (IMO) this is a small saving usually. The larger saving usually comes with the indirect saving by eliminating the vtable of the supertrait (and its parent).
Both impacts only affects vtables (notably the number of functions monomorphized should not change, however where vtable reside can depend on your relocation model. If the relocation model is static, then vtable is rodata (usually stored in Flash/ROM together with text in embedded scenario). If the binary is relocatable, however, the vtable will live in
.data(more specifically,.data.rel.ro), and this will need to reside in RAM (which may be a more scarce resource in some cases), together with dynamic relocation info living in readonly segment.For evaluation, you should run
sizeon both binaries, with and without the flag.sizewould output three columns,text,data,bssand the sumdec(and it's hex version). As explained above, bothtextanddatamay change.bssshouldn't usually change. It'll be useful to see: