[MIR] Store MIR in crate metadata #30301
Conversation
michaelwoerister
added
the
A-MIR
|
(rust_highfive has picked a reviewer for you, use r? to override) |
michaelwoerister
force-pushed
the
mir-to-metadata2
branch
2 times, most recently
from
bcff7ba to
7786e1e
Compare
|
cc @alexcrichton @brson -- thoughts about binary size? this would only be temporary until MIR trans lands, of course, but that could be a little while. UPDATE: To be clear, I mean that eventually we would not have to include the HIR as well. |
|
r+ modulo the two comments -- no tests, but that's ok because this is going to be used very heavily and is already tested in mw's local branches (also, I don't believe there are any tests for metadata encoding-decoding at all, so no existing infrastructure) |
|
There've been some nice improvements to metadata size recently so this may just get us back to where we used to be at, so in that sense it's not that much worse. One possibility perhaps could be to only enable this on the nightly channel, but if MIR is enabled anywhere this seems ok to me as HIR is on its way out. |
|
So many bloat. We need to try and get it under control. |
| @@ -632,7 +652,7 @@ pub struct DebruijnIndex { | |||
| /// | |||
| /// [1] http://smallcultfollowing.com/babysteps/blog/2013/10/29/intermingled-parameter-lists/ | |||
| /// [2] http://smallcultfollowing.com/babysteps/blog/2013/11/04/intermingled-parameter-lists/ | |||
| #[derive(Clone, PartialEq, Eq, Hash, Copy)] | |||
| #[derive(Clone, PartialEq, Eq, Hash, Copy, RustcEncodable, RustcDecodable)] | |||
There was a problem hiding this comment.
What are you doing? Are we encoding non-erased regions?
There was a problem hiding this comment.
Region shows up in mir::repr::Rvalue so we need serialization implementations for it. At the encoding time it should always have been erased though.
michaelwoerister
force-pushed
the
mir-to-metadata2
branch
from
7786e1e to
5addc31
Compare
At some point we definitely should, I agree. |
|
@bors r=nikomatsakis |
|
📌 Commit 5addc31 has been approved by |
This PR makes `Mir` `RustcEncodable` and `RustcDecodable` and stores it in crate metadata for inlinable items. Some other things in here: - `mir::visit::Visitor` is extended to also visit `Literals`, `Spans` and `DefIds`. - It also adds `mir::visit::MutVisitor` which allows to mutate the visited `Mir` structure in place. - Some numbers on how big MIR is in metadata (total metadata size in bytes): | | w/ MIR | w/o MIR | Rel. Size | |----------------|-----------:|------------:|:---------:| | libcore | 17,695,473 | 14,263,377 | 124% | | liblibc | 411,440 | 404,382 | 102% | | libcollections | 4,537,975 | 3,526,933 | 129% | | libserialize | 2,574,769 | 2,060,798 | 125% | | libsyntax | 15,262,894 | 12,075,574 | 126% | | librustc | 16,984,537 | 13,692,168 | 124% | So, adding MIR to metadata makes it about 25% bigger. It could be worse, considering that it still uses the inefficient RBML encoding. Still, the question is whether we should put MIR emission behind a `-Z` flag.
1 similar comment
This PR makes
MirRustcEncodableandRustcDecodableand stores it in crate metadata for inlinable items.Some other things in here:
mir::visit::Visitoris extended to also visitLiterals,SpansandDefIds.mir::visit::MutVisitorwhich allows to mutate the visitedMirstructure in place.So, adding MIR to metadata makes it about 25% bigger. It could be worse, considering that it still uses the inefficient RBML encoding. Still, the question is whether we should put MIR emission behind a
-Zflag.