Skip to content

Commit 4118ff6

Browse files
authored
Rollup merge of #69837 - jonas-schievink:gen-discr-opt, r=tmandry
Use smaller discriminants for generators Closes #69815 I'm not yet sure about the runtime performance impact of this, so I'll try running this on some benchmarks (if I can find any). (Update: No impact on the benchmarks I've measured on) * [x] Add test with a generator that has exactly 256 total states * [x] Add test with a generator that has more than 256 states so that it needs to use a u16 discriminant * [x] Add tests for the size of `Option<[generator]>` * [x] Add tests for the `discriminant_value` intrinsic in all cases
2 parents 3f583fc + 49aabd8 commit 4118ff6

7 files changed

+167
-30
lines changed

src/librustc/ty/layout.rs

+9-6
Original file line numberDiff line numberDiff line change
@@ -1409,12 +1409,15 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
14091409
// locals as part of the prefix. We compute the layout of all of
14101410
// these fields at once to get optimal packing.
14111411
let discr_index = substs.as_generator().prefix_tys(def_id, tcx).count();
1412-
// FIXME(eddyb) set the correct vaidity range for the discriminant.
1413-
let discr_layout = self.layout_of(substs.as_generator().discr_ty(tcx))?;
1414-
let discr = match &discr_layout.abi {
1415-
Abi::Scalar(s) => s.clone(),
1416-
_ => bug!(),
1417-
};
1412+
1413+
// `info.variant_fields` already accounts for the reserved variants, so no need to add them.
1414+
let max_discr = (info.variant_fields.len() - 1) as u128;
1415+
let discr_int = Integer::fit_unsigned(max_discr);
1416+
let discr_int_ty = discr_int.to_ty(tcx, false);
1417+
let discr = Scalar { value: Primitive::Int(discr_int, false), valid_range: 0..=max_discr };
1418+
let discr_layout = self.tcx.intern_layout(LayoutDetails::scalar(self, discr.clone()));
1419+
let discr_layout = TyLayout { ty: discr_int_ty, details: discr_layout };
1420+
14181421
let promoted_layouts = ineligible_locals
14191422
.iter()
14201423
.map(|local| subst_field(info.field_tys[local]))

src/test/ui/async-await/async-fn-size-moved-locals.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,9 @@ async fn mixed_sizes() {
110110
}
111111

112112
fn main() {
113-
assert_eq!(1028, std::mem::size_of_val(&single()));
114-
assert_eq!(1032, std::mem::size_of_val(&single_with_noop()));
115-
assert_eq!(3084, std::mem::size_of_val(&joined()));
116-
assert_eq!(3084, std::mem::size_of_val(&joined_with_noop()));
117-
assert_eq!(7188, std::mem::size_of_val(&mixed_sizes()));
113+
assert_eq!(1025, std::mem::size_of_val(&single()));
114+
assert_eq!(1026, std::mem::size_of_val(&single_with_noop()));
115+
assert_eq!(3078, std::mem::size_of_val(&joined()));
116+
assert_eq!(3079, std::mem::size_of_val(&joined_with_noop()));
117+
assert_eq!(7181, std::mem::size_of_val(&mixed_sizes()));
118118
}

src/test/ui/async-await/async-fn-size-uninit-locals.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,9 @@ async fn join_retval() -> Joiner {
9595
}
9696

9797
fn main() {
98-
assert_eq!(8, std::mem::size_of_val(&single()));
99-
assert_eq!(12, std::mem::size_of_val(&single_with_noop()));
100-
assert_eq!(3084, std::mem::size_of_val(&joined()));
101-
assert_eq!(3084, std::mem::size_of_val(&joined_with_noop()));
102-
assert_eq!(3080, std::mem::size_of_val(&join_retval()));
98+
assert_eq!(2, std::mem::size_of_val(&single()));
99+
assert_eq!(3, std::mem::size_of_val(&single_with_noop()));
100+
assert_eq!(3078, std::mem::size_of_val(&joined()));
101+
assert_eq!(3078, std::mem::size_of_val(&joined_with_noop()));
102+
assert_eq!(3074, std::mem::size_of_val(&join_retval()));
103103
}

src/test/ui/async-await/async-fn-size.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,13 @@ async fn await3_level5() -> u8 {
8686

8787
fn main() {
8888
assert_eq!(2, std::mem::size_of_val(&base()));
89-
assert_eq!(8, std::mem::size_of_val(&await1_level1()));
90-
assert_eq!(12, std::mem::size_of_val(&await2_level1()));
91-
assert_eq!(12, std::mem::size_of_val(&await3_level1()));
92-
assert_eq!(24, std::mem::size_of_val(&await3_level2()));
93-
assert_eq!(36, std::mem::size_of_val(&await3_level3()));
94-
assert_eq!(48, std::mem::size_of_val(&await3_level4()));
95-
assert_eq!(60, std::mem::size_of_val(&await3_level5()));
89+
assert_eq!(3, std::mem::size_of_val(&await1_level1()));
90+
assert_eq!(4, std::mem::size_of_val(&await2_level1()));
91+
assert_eq!(5, std::mem::size_of_val(&await3_level1()));
92+
assert_eq!(8, std::mem::size_of_val(&await3_level2()));
93+
assert_eq!(11, std::mem::size_of_val(&await3_level3()));
94+
assert_eq!(14, std::mem::size_of_val(&await3_level4()));
95+
assert_eq!(17, std::mem::size_of_val(&await3_level5()));
9696

9797
assert_eq!(1, wait(base()));
9898
assert_eq!(1, wait(await1_level1()));

src/test/ui/generator/discriminant.rs

+134
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
//! Tests that generator discriminant sizes and ranges are chosen optimally and that they are
2+
//! reflected in the output of `mem::discriminant`.
3+
4+
// run-pass
5+
6+
#![feature(generators, generator_trait, core_intrinsics)]
7+
8+
use std::intrinsics::discriminant_value;
9+
use std::marker::Unpin;
10+
use std::mem::size_of_val;
11+
use std::{cmp, ops::*};
12+
13+
macro_rules! yield25 {
14+
($e:expr) => {
15+
yield $e;
16+
yield $e;
17+
yield $e;
18+
yield $e;
19+
yield $e;
20+
21+
yield $e;
22+
yield $e;
23+
yield $e;
24+
yield $e;
25+
yield $e;
26+
27+
yield $e;
28+
yield $e;
29+
yield $e;
30+
yield $e;
31+
yield $e;
32+
33+
yield $e;
34+
yield $e;
35+
yield $e;
36+
yield $e;
37+
yield $e;
38+
39+
yield $e;
40+
yield $e;
41+
yield $e;
42+
yield $e;
43+
yield $e;
44+
};
45+
}
46+
47+
/// Yields 250 times.
48+
macro_rules! yield250 {
49+
() => {
50+
yield250!(())
51+
};
52+
53+
($e:expr) => {
54+
yield25!($e);
55+
yield25!($e);
56+
yield25!($e);
57+
yield25!($e);
58+
yield25!($e);
59+
60+
yield25!($e);
61+
yield25!($e);
62+
yield25!($e);
63+
yield25!($e);
64+
yield25!($e);
65+
};
66+
}
67+
68+
fn cycle(gen: impl Generator<()> + Unpin, expected_max_discr: u64) {
69+
let mut gen = Box::pin(gen);
70+
let mut max_discr = 0;
71+
loop {
72+
max_discr = cmp::max(max_discr, discriminant_value(gen.as_mut().get_mut()));
73+
match gen.as_mut().resume(()) {
74+
GeneratorState::Yielded(_) => {}
75+
GeneratorState::Complete(_) => {
76+
assert_eq!(max_discr, expected_max_discr);
77+
return;
78+
}
79+
}
80+
}
81+
}
82+
83+
fn main() {
84+
// Has only one invalid discr. value.
85+
let gen_u8_tiny_niche = || {
86+
|| {
87+
// 3 reserved variants
88+
89+
yield250!(); // 253 variants
90+
91+
yield; // 254
92+
yield; // 255
93+
}
94+
};
95+
96+
// Uses all values in the u8 discriminant.
97+
let gen_u8_full = || {
98+
|| {
99+
// 3 reserved variants
100+
101+
yield250!(); // 253 variants
102+
103+
yield; // 254
104+
yield; // 255
105+
yield; // 256
106+
}
107+
};
108+
109+
// Barely needs a u16 discriminant.
110+
let gen_u16 = || {
111+
|| {
112+
// 3 reserved variants
113+
114+
yield250!(); // 253 variants
115+
116+
yield; // 254
117+
yield; // 255
118+
yield; // 256
119+
yield; // 257
120+
}
121+
};
122+
123+
assert_eq!(size_of_val(&gen_u8_tiny_niche()), 1);
124+
assert_eq!(size_of_val(&Some(gen_u8_tiny_niche())), 1); // uses niche
125+
assert_eq!(size_of_val(&Some(Some(gen_u8_tiny_niche()))), 2); // cannot use niche anymore
126+
assert_eq!(size_of_val(&gen_u8_full()), 1);
127+
assert_eq!(size_of_val(&Some(gen_u8_full())), 2); // cannot use niche
128+
assert_eq!(size_of_val(&gen_u16()), 2);
129+
assert_eq!(size_of_val(&Some(gen_u16())), 2); // uses niche
130+
131+
cycle(gen_u8_tiny_niche(), 254);
132+
cycle(gen_u8_full(), 255);
133+
cycle(gen_u16(), 256);
134+
}

src/test/ui/generator/resume-arg-size.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ fn main() {
2323

2424
// Neither of these generators have the resume arg live across the `yield`, so they should be
2525
// 4 Bytes in size (only storing the discriminant)
26-
assert_eq!(size_of_val(&gen_copy), 4);
27-
assert_eq!(size_of_val(&gen_move), 4);
26+
assert_eq!(size_of_val(&gen_copy), 1);
27+
assert_eq!(size_of_val(&gen_move), 1);
2828
}

src/test/ui/generator/size-moved-locals.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ fn overlap_move_points() -> impl Generator<Yield = (), Return = ()> {
5858
}
5959
}
6060

61-
fn overlap_x_and_y() -> impl Generator<Yield = (), Return = ()>{
61+
fn overlap_x_and_y() -> impl Generator<Yield = (), Return = ()> {
6262
static || {
6363
let x = Foo([0; FOO_SIZE]);
6464
yield;
@@ -70,8 +70,8 @@ fn overlap_x_and_y() -> impl Generator<Yield = (), Return = ()>{
7070
}
7171

7272
fn main() {
73-
assert_eq!(1028, std::mem::size_of_val(&move_before_yield()));
74-
assert_eq!(1032, std::mem::size_of_val(&move_before_yield_with_noop()));
75-
assert_eq!(2056, std::mem::size_of_val(&overlap_move_points()));
76-
assert_eq!(1032, std::mem::size_of_val(&overlap_x_and_y()));
73+
assert_eq!(1025, std::mem::size_of_val(&move_before_yield()));
74+
assert_eq!(1026, std::mem::size_of_val(&move_before_yield_with_noop()));
75+
assert_eq!(2051, std::mem::size_of_val(&overlap_move_points()));
76+
assert_eq!(1026, std::mem::size_of_val(&overlap_x_and_y()));
7777
}

0 commit comments

Comments
 (0)