-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Miscellneous refactorings of trans #41287
Miscellneous refactorings of trans #41287
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo some nits that can be addressed later.
@@ -52,7 +52,6 @@ pub use self::NativeLibraryKind::*; | |||
|
|||
#[derive(Clone, Debug)] | |||
pub struct LinkMeta { | |||
pub crate_name: Symbol, | |||
pub crate_hash: Svh, | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one of the weirder structures in the whole compiler and it'd be nice if it'd just die already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah I meant to kill it altogether; it's just a newtype'd svh at this point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do that in a follow-up though
} | ||
return metadata; | ||
return (metadata_llcx, metadata_llmod, metadata); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am surprised we don't have a wrapper for the llcx
+ llmod
combo.
let (metadata_llcx, metadata_llmod, metadata) = | ||
time(tcx.sess.time_passes(), "write metadata", || { | ||
write_metadata(tcx, &link_meta, shared_ccx.exported_symbols()) | ||
}); | ||
|
||
let metadata_module = ModuleTranslation { | ||
name: link::METADATA_MODULE_NAME.to_string(), | ||
symbol_name_hash: 0, // we always rebuild metadata, at least for now | ||
source: ModuleSource::Translated(ModuleLlvm { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ModuleLlvm
is what I mean I guess?
ModuleTranslation { | ||
name: cgu_name, | ||
symbol_name_hash, | ||
source: ModuleSource::Translated(ModuleLlvm { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, it'd be nice if ccx
actually kept a ModuleLlvm
.
pub fn extend(&mut self, stats: Stats) { | ||
self.n_glues_created.set(self.n_glues_created.get() + stats.n_glues_created.get()); | ||
self.n_null_glues.set(self.n_null_glues.get() + stats.n_null_glues.get()); | ||
self.n_real_glues.set(self.n_real_glues.get() + stats.n_real_glues.get()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a bunch of stats that are irrelevant anymore AFAIK.
No good reason for them to be in there.
These do some low-level munging on the LLVM data structures. Unclear that they need to operate as a "second pass" but leave it for now.
A number of things were using `crate_hash` that really ought to be using `crate_disambiguator` (e.g., to create the plugin symbol names). They have been updated. It is important to remove `LinkMeta` from `SharedCrateContext` since it contains a hash of the entire crate, and hence it will change whenever **anything** changes (which would then require rebuilding **everything**).
shared mutable state is bad
d41ce38
to
07fb93e
Compare
@bors r+ |
📌 Commit 07fb93e has been approved by |
…ns, r=eddyb Miscellneous refactorings of trans This doesn't achieve any particular goal yet, but it's a collection of refactorings with the common goal of turning `SharedCrateContext` etc into stuff that we can use with on-demand and actually expect to hash in a stable fashion for incremental. Not there yet, clearly. r? @eddyb cc @michaelwoerister
This doesn't achieve any particular goal yet, but it's a collection of refactorings with the common goal of turning
SharedCrateContext
etc into stuff that we can use with on-demand and actually expect to hash in a stable fashion for incremental. Not there yet, clearly.r? @eddyb
cc @michaelwoerister