Uh oh!
There was an error while loading. Please reload this page.
no more transmute<&List<Ty<'tcx>>, &List<GenericArg<'tcx>>> - #94841
Conversation
rust-highfive
commented
Mar 11, 2022
r? @davidtwco (rust-highfive has picked a reviewer for you, use r? to override) |
transmute<List<Ty<'tcx>>, List<GenericArg<'tcx>>>transmute<&List<Ty<'tcx>>, &List<GenericArg<'tcx>>>lcnr
commented
Mar 11, 2022
@bors try @rust-timer queue |
rust-timer
commented
Mar 11, 2022
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
bors
commented
Mar 11, 2022
⌛ Trying commit 0ed0fce with merge 4b4ec5434ef1443b760a9a294271b40d3f67560b... |
bors
commented
Mar 11, 2022
☀️ Try build successful - checks-actions |
rust-timer
commented
Mar 11, 2022
Queued 4b4ec5434ef1443b760a9a294271b40d3f67560b with parent f58d51b, future comparison URL. |
rust-timer
commented
Mar 11, 2022
Finished benchmarking commit (4b4ec5434ef1443b760a9a294271b40d3f67560b): comparison url. Summary: This benchmark run did not return any relevant results. 36 results were found to be statistically significant but too small to be relevant. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking 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 led to changes in compiler perf. @bors rollup=never |
jackh726
commented
Mar 12, 2022
It is a very small regression, but I think it's worth it for the reduced complexity. |
lcnr
commented
Mar 14, 2022
i don't necessarily think so 🤔 I want to keep using Going to keep looking for the actual cause of that regression for now and then figure out where to go from there |
davidtwco
commented
Mar 18, 2022
It doesn't sound like you're intending to land this, so I'll mark it as |
…errors Don't transmute `&List<GenericArg>` <-> `&List<Ty>` In rust-lang#93505 we allowed safely transmuting between `&List<GenericArg<'_>>` and `&List<Ty<'_>>`. This was possible because `GenericArg` is a tagged pointer and the tag for types is `0b00`, such that a `GenericArg` with a type inside has the same layout as `Ty`. While this was meant as an optimization, it doesn't look like it was actually any perf or max-rss win (see rust-lang#94799 (comment), rust-lang#94841, rust-lang#110496 (comment)). Additionally the way it was done is quite fragile — `unsafe` code was not properly documented or contained in a module, types were not marked as `repr(C)` (making the transmutes possibly unsound). All of this makes the code maintenance harder and blocks other possible optimizations (as an example I've found out about these `transmutes` when my change caused them to sigsegv compiler). Thus, I think we can safely (pun intended) remove those transmutes, making maintenance easier, optimizations possible, code less cursed, etc. r? `@compiler-errors`
Don't transmute `&List<GenericArg>` <-> `&List<Ty>` In #93505 we allowed safely transmuting between `&List<GenericArg<'_>>` and `&List<Ty<'_>>`. This was possible because `GenericArg` is a tagged pointer and the tag for types is `0b00`, such that a `GenericArg` with a type inside has the same layout as `Ty`. While this was meant as an optimization, it doesn't look like it was actually any perf or max-rss win (see rust-lang/rust#94799 (comment), rust-lang/rust#94841, rust-lang/rust#110496 (comment)). Additionally the way it was done is quite fragile — `unsafe` code was not properly documented or contained in a module, types were not marked as `repr(C)` (making the transmutes possibly unsound). All of this makes the code maintenance harder and blocks other possible optimizations (as an example I've found out about these `transmutes` when my change caused them to sigsegv compiler). Thus, I think we can safely (pun intended) remove those transmutes, making maintenance easier, optimizations possible, code less cursed, etc. r? `@compiler-errors`
cc #94799 (comment)
reverts the first commit of #93505.
let's see what's the perf and max-rss effect of this change.